Input: Replace markFileChanged() by putFile()

Committing a lock file using markFileChanged() required the input to
be writable by the caller in the local filesystem (using the path
returned by getSourcePath()). putFile() abstracts over this.

(cherry picked from commit 95d657c8b3ae4282e24628ba7426edb90c8f3942)
Change-Id: Ie081c5d9eb4e923b229191c5e23ece85145557ff

src/libexpr/flake/flake.cc

@@ -627,12 +627,7 @@ LockedFlake lockFlake(
 
         debug("new lock file: %s", newLockFile);
 
-        auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
         auto sourcePath = topRef.input.getSourcePath();
-        auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt;
-        if (lockFlags.outputLockFilePath) {
-            outputLockFilePath = lockFlags.outputLockFilePath;
-        }
 
         /* Check whether we need to / can write the new lock file. */
         if (newLockFile != oldLockFile || lockFlags.outputLockFilePath) {
@@ -640,7 +635,7 @@ LockedFlake lockFlake(
             auto diff = LockFile::diff(oldLockFile, newLockFile);
 
             if (lockFlags.writeLockFile) {
-                if (outputLockFilePath) {
+                if (sourcePath || lockFlags.outputLockFilePath) {
                     if (auto unlockedInput = newLockFile.isUnlocked()) {
                         if (fetchSettings.warnDirty)
                             warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput);
@@ -648,41 +643,49 @@ LockedFlake lockFlake(
                         if (!lockFlags.updateLockFile)
                             throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef);
 
-                        bool lockFileExists = pathExists(*outputLockFilePath);
+                        auto newLockFileS = fmt("%s\n", newLockFile);
 
-                        if (lockFileExists) {
+                        if (lockFlags.outputLockFilePath)
-                            auto s = chomp(diff);
+                            writeFile(*lockFlags.outputLockFilePath, newLockFileS);
-                            if (s.empty())
+                        else {
-                                warn("updating lock file '%s'", *outputLockFilePath);
+                            auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
-                            else
+                            auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt;
-                                warn("updating lock file '%s':\n%s", *outputLockFilePath, s);
-                        } else
-                            warn("creating lock file '%s'", *outputLockFilePath);
 
-                        newLockFile.write(*outputLockFilePath);
+                            bool lockFileExists = pathExists(*outputLockFilePath);
 
-                        std::optional<std::string> commitMessage = std::nullopt;
+                            if (lockFileExists) {
-                        if (lockFlags.commitLockFile) {
+                                auto s = chomp(diff);
-                            if (lockFlags.outputLockFilePath) {
+                                if (s.empty())
-                                throw Error("--commit-lock-file and --output-lock-file are currently incompatible");
+                                    warn("updating lock file '%s'", *outputLockFilePath);
-                            }
+                                else
-                            std::string cm;
+                                    warn("updating lock file '%s':\n%s", *outputLockFilePath, s);
+                            } else
+                                warn("creating lock file '%s'", *outputLockFilePath);
 
-                            cm = fetchSettings.commitLockFileSummary.get();
+                            std::optional<std::string> commitMessage = std::nullopt;
 
-                            if (cm == "") {
+                            if (lockFlags.commitLockFile) {
-                                cm = fmt("%s: %s", relPath, lockFileExists ? "Update" : "Add");
+                                if (lockFlags.outputLockFilePath) {
+                                    throw Error("--commit-lock-file and --output-lock-file are currently incompatible");
+                                }
+                                std::string cm;
+
+                                cm = fetchSettings.commitLockFileSummary.get();
+
+                                if (cm == "") {
+                                    cm = fmt("%s: %s", relPath, lockFileExists ? "Update" : "Add");
+                                }
+
+                                cm += "\n\nFlake lock file updates:\n\n";
+                                cm += filterANSIEscapes(diff, true);
+                                commitMessage = cm;
                             }
 
-                            cm += "\n\nFlake lock file updates:\n\n";
+                            topRef.input.putFile(
-                            cm += filterANSIEscapes(diff, true);
+                                CanonPath((topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock"),
-                            commitMessage = cm;
+                                newLockFileS, commitMessage);
                         }
 
-                        topRef.input.markChangedFile(
-                            (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock",
-                            commitMessage);
-
                         /* Rewriting the lockfile changed the top-level
                            repo, so we should re-read it. FIXME: we could
                            also just clear the 'rev' field... */

src/libfetchers/cache.hh

@@ -2,6 +2,7 @@
 ///@file
 
 #include "fetchers.hh"
+#include "path.hh"
 
 namespace nix::fetchers {
 

src/libfetchers/fetchers.cc

@@ -200,12 +200,13 @@ std::optional<Path> Input::getSourcePath() const
     return scheme->getSourcePath(*this);
 }
 
-void Input::markChangedFile(
+void Input::putFile(
-    std::string_view file,
+    const CanonPath & path,
+    std::string_view contents,
     std::optional<std::string> commitMsg) const
 {
     assert(scheme);
-    return scheme->markChangedFile(*this, file, commitMsg);
+    return scheme->putFile(*this, path, contents, commitMsg);
 }
 
 std::string Input::getName() const
@@ -295,14 +296,18 @@ Input InputScheme::applyOverrides(
     return input;
 }
 
-std::optional<Path> InputScheme::getSourcePath(const Input & input)
+std::optional<Path> InputScheme::getSourcePath(const Input & input) const
 {
     return {};
 }
 
-void InputScheme::markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg)
+void InputScheme::putFile(
+    const Input & input,
+    const CanonPath & path,
+    std::string_view contents,
+    std::optional<std::string> commitMsg) const
 {
-    assert(false);
+    throw Error("input '%s' does not support modifying file '%s'", input.to_string(), path);
 }
 
 void InputScheme::clone(const Input & input, const Path & destDir) const

src/libfetchers/fetchers.hh

@@ -3,13 +3,14 @@
 
 #include "types.hh"
 #include "hash.hh"
+#include "canon-path.hh"
 #include "path.hh"
 #include "attrs.hh"
 #include "url.hh"
 
 #include <memory>
 
-namespace nix { class Store; }
+namespace nix { class Store; class StorePath; }
 
 namespace nix::fetchers {
 
@@ -97,8 +98,13 @@ public:
 
     std::optional<Path> getSourcePath() const;
 
-    void markChangedFile(
+    /**
-        std::string_view file,
+     * Write a file to this input, for input types that support
+     * writing. Optionally commit the change (for e.g. Git inputs).
+     */
+    void putFile(
+        const CanonPath & path,
+        std::string_view contents,
         std::optional<std::string> commitMsg) const;
 
     std::string getName() const;
@@ -144,9 +150,13 @@ struct InputScheme
 
     virtual void clone(const Input & input, const Path & destDir) const;
 
-    virtual std::optional<Path> getSourcePath(const Input & input);
+    virtual std::optional<Path> getSourcePath(const Input & input) const;
 
-    virtual void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg);
+    virtual void putFile(
+        const Input & input,
+        const CanonPath & path,
+        std::string_view contents,
+        std::optional<std::string> commitMsg) const;
 
     virtual std::pair<StorePath, Input> fetch(ref<Store> store, const Input & input) = 0;
 };

src/libfetchers/git.cc

@@ -363,7 +363,7 @@ struct GitInputScheme : InputScheme
         runProgram("git", true, args, {}, true);
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
+    std::optional<Path> getSourcePath(const Input & input) const override
     {
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         if (url.scheme == "file" && !input.getRef() && !input.getRev())
@@ -371,22 +371,30 @@ struct GitInputScheme : InputScheme
         return {};
     }
 
-    void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
+    void putFile(
+        const Input & input,
+        const CanonPath & path,
+        std::string_view contents,
+        std::optional<std::string> commitMsg) const override
     {
-        auto sourcePath = getSourcePath(input);
+        auto root = getSourcePath(input);
-        assert(sourcePath);
+        if (!root)
+            throw Error("cannot commit '%s' to Git repository '%s' because it's not a working tree", path, input.to_string());
+
+        writeFile((CanonPath(*root) + path).abs(), contents);
+
         auto gitDir = ".git";
 
         auto result = runProgram(RunOptions {
             .program = "git",
-            .args = {"-C", *sourcePath, "--git-dir", gitDir, "check-ignore", "--quiet", std::string(file)},
+            .args = {"-C", *root, "--git-dir", gitDir, "check-ignore", "--quiet", std::string(path.rel())},
         });
         auto exitCode = WEXITSTATUS(result.first);
 
         if (exitCode != 0) {
             // The path is not `.gitignore`d, we can add the file.
             runProgram("git", true,
-                { "-C", *sourcePath, "--git-dir", gitDir, "add", "--intent-to-add", "--", std::string(file) });
+                { "-C", *root, "--git-dir", gitDir, "add", "--intent-to-add", "--", std::string(path.rel()) });
 
 
             if (commitMsg) {
@@ -394,7 +402,7 @@ struct GitInputScheme : InputScheme
                 logger->pause();
                 Finally restoreLogger([]() { logger->resume(); });
                 runProgram("git", true,
-                    { "-C", *sourcePath, "--git-dir", gitDir, "commit", std::string(file), "-m", *commitMsg });
+                    { "-C", *root, "--git-dir", gitDir, "commit", std::string(path.rel()), "-m", *commitMsg });
             }
         }
     }

src/libfetchers/indirect.cc

@@ -1,5 +1,6 @@
 #include "fetchers.hh"
 #include "url-parts.hh"
+#include "path.hh"
 
 namespace nix::fetchers {
 

src/libfetchers/mercurial.cc

@@ -116,7 +116,7 @@ struct MercurialInputScheme : InputScheme
         return res;
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
+    std::optional<Path> getSourcePath(const Input & input) const override
     {
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         if (url.scheme == "file" && !input.getRef() && !input.getRev())
@@ -124,18 +124,27 @@ struct MercurialInputScheme : InputScheme
         return {};
     }
 
-    void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
+    void putFile(
+        const Input & input,
+        const CanonPath & path,
+        std::string_view contents,
+        std::optional<std::string> commitMsg) const override
     {
-        auto sourcePath = getSourcePath(input);
+        auto [isLocal, repoPath] = getActualUrl(input);
-        assert(sourcePath);
+        if (!isLocal)
+            throw Error("cannot commit '%s' to Mercurial repository '%s' because it's not a working tree", path, input.to_string());
+
+        auto absPath = CanonPath(repoPath) + path;
+
+        writeFile(absPath.abs(), contents);
 
         // FIXME: shut up if file is already tracked.
         runHg(
-            { "add", *sourcePath + "/" + std::string(file) });
+            { "add", absPath.abs() });
 
         if (commitMsg)
             runHg(
-                { "commit", *sourcePath + "/" + std::string(file), "-m", *commitMsg });
+                { "commit", absPath.abs(), "-m", *commitMsg });
     }
 
     std::pair<bool, std::string> getActualUrl(const Input & input) const

src/libfetchers/path.cc

@@ -71,14 +71,28 @@ struct PathInputScheme : InputScheme
         return true;
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
+    std::optional<Path> getSourcePath(const Input & input) const override
     {
         return getStrAttr(input.attrs, "path");
     }
 
-    void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
+    void putFile(
+        const Input & input,
+        const CanonPath & path,
+        std::string_view contents,
+        std::optional<std::string> commitMsg) const override
     {
-        // nothing to do
+        writeFile((CanonPath(getAbsPath(input)) + path).abs(), contents);
+    }
+
+    CanonPath getAbsPath(const Input & input) const
+    {
+        auto path = getStrAttr(input.attrs, "path");
+
+        if (path[0] == '/')
+            return CanonPath(path);
+
+        throw Error("cannot fetch input '%s' because it uses a relative path", input.to_string());
     }
 
     std::pair<StorePath, Input> fetch(ref<Store> store, const Input & _input) override