Merge pull request #9239 from edolstra/putFile

Input: Replace markFileChanged() by putFile()

src/libexpr/flake/flake.cc

@@ -623,12 +623,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) {
@@ -636,7 +631,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);
@@ -644,41 +639,48 @@ 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);
+                            if (lockFlags.commitLockFile)
-                            if (s.empty())
+                                throw Error("'--commit-lock-file' and '--output-lock-file' are incompatible");
-                                warn("updating lock file '%s'", *outputLockFilePath);
+                            writeFile(*lockFlags.outputLockFilePath, newLockFileS);
-                            else
+                        } else {
-                                warn("updating lock file '%s':\n%s", *outputLockFilePath, s);
+                            auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
-                        } else
+                            auto outputLockFilePath = *sourcePath + "/" + relPath;
-                            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");
+                                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/libexpr/flake/lockfile.cc

@@ -214,12 +214,6 @@ std::ostream & operator <<(std::ostream & stream, const LockFile & lockFile)
     return stream;
 }
 
-void LockFile::write(const Path & path) const
-{
-    createDirs(dirOf(path));
-    writeFile(path, fmt("%s\n", *this));
-}
-
 std::optional<FlakeRef> LockFile::isUnlocked() const
 {
     std::set<ref<const Node>> nodes;

src/libexpr/flake/lockfile.hh

@@ -65,8 +65,6 @@ struct LockFile
 
     static LockFile read(const Path & path);
 
-    void write(const Path & path) const;
-
     /**
      * Check whether this lock file has any unlocked inputs.
      */

src/libfetchers/cache.hh

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

src/libfetchers/fetchers.cc

@@ -196,12 +196,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
@@ -292,14 +293,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,13 @@
 
 #include "types.hh"
 #include "hash.hh"
-#include "path.hh"
+#include "canon-path.hh"
 #include "attrs.hh"
 #include "url.hh"
 
 #include <memory>
 
-namespace nix { class Store; }
+namespace nix { class Store; class StorePath; }
 
 namespace nix::fetchers {
 
@@ -90,8 +90,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;
@@ -135,9 +140,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

@@ -354,7 +354,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())
@@ -362,18 +362,26 @@ 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";
 
         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)
             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 });
     }
 
     std::pair<bool, std::string> getActualUrl(const Input & input) const

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

@@ -109,7 +109,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())
@@ -117,18 +117,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

@@ -66,14 +66,28 @@ struct PathInputScheme : InputScheme
         };
     }
 
-    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