From b525d0f20c4fd94b89d6d34d8acd0ca4c579a5fc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 25 Oct 2023 18:18:15 +0200 Subject: [PATCH] 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 | 67 +++++++++++++++++++----------------- src/libfetchers/cache.hh | 1 + src/libfetchers/fetchers.cc | 17 +++++---- src/libfetchers/fetchers.hh | 20 ++++++++--- src/libfetchers/git.cc | 22 ++++++++---- src/libfetchers/indirect.cc | 1 + src/libfetchers/mercurial.cc | 21 +++++++---- src/libfetchers/path.cc | 20 +++++++++-- 8 files changed, 110 insertions(+), 59 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 72e98f6bb..20d7878e9 100644 --- a/src/libexpr/flake/flake.cc +++ b/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) { - auto s = chomp(diff); - if (s.empty()) - warn("updating lock file '%s'", *outputLockFilePath); - else - warn("updating lock file '%s':\n%s", *outputLockFilePath, s); - } else - warn("creating lock file '%s'", *outputLockFilePath); + if (lockFlags.outputLockFilePath) + writeFile(*lockFlags.outputLockFilePath, newLockFileS); + else { + auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock"; + auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt; - newLockFile.write(*outputLockFilePath); + bool lockFileExists = pathExists(*outputLockFilePath); - std::optional commitMessage = std::nullopt; - if (lockFlags.commitLockFile) { - if (lockFlags.outputLockFilePath) { - throw Error("--commit-lock-file and --output-lock-file are currently incompatible"); - } - std::string cm; + if (lockFileExists) { + auto s = chomp(diff); + if (s.empty()) + warn("updating lock file '%s'", *outputLockFilePath); + else + warn("updating lock file '%s':\n%s", *outputLockFilePath, s); + } else + warn("creating lock file '%s'", *outputLockFilePath); - cm = fetchSettings.commitLockFileSummary.get(); + std::optional commitMessage = std::nullopt; - if (cm == "") { - cm = fmt("%s: %s", relPath, lockFileExists ? "Update" : "Add"); + if (lockFlags.commitLockFile) { + 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"; - cm += filterANSIEscapes(diff, true); - commitMessage = cm; + topRef.input.putFile( + CanonPath((topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock"), + 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... */ diff --git a/src/libfetchers/cache.hh b/src/libfetchers/cache.hh index ae398d040..af34e66ce 100644 --- a/src/libfetchers/cache.hh +++ b/src/libfetchers/cache.hh @@ -2,6 +2,7 @@ ///@file #include "fetchers.hh" +#include "path.hh" namespace nix::fetchers { diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 530473023..dc8df2217 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -200,12 +200,13 @@ std::optional Input::getSourcePath() const return scheme->getSourcePath(*this); } -void Input::markChangedFile( - std::string_view file, +void Input::putFile( + const CanonPath & path, + std::string_view contents, std::optional 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 InputScheme::getSourcePath(const Input & input) +std::optional InputScheme::getSourcePath(const Input & input) const { return {}; } -void InputScheme::markChangedFile(const Input & input, std::string_view file, std::optional commitMsg) +void InputScheme::putFile( + const Input & input, + const CanonPath & path, + std::string_view contents, + std::optional 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 diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 6e10e9513..e1067ce1c 100644 --- a/src/libfetchers/fetchers.hh +++ b/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 -namespace nix { class Store; } +namespace nix { class Store; class StorePath; } namespace nix::fetchers { @@ -97,8 +98,13 @@ public: std::optional 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 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 getSourcePath(const Input & input); + virtual std::optional getSourcePath(const Input & input) const; - virtual void markChangedFile(const Input & input, std::string_view file, std::optional commitMsg); + virtual void putFile( + const Input & input, + const CanonPath & path, + std::string_view contents, + std::optional commitMsg) const; virtual std::pair fetch(ref store, const Input & input) = 0; }; diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 48e1bd6e3..671779f84 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -363,7 +363,7 @@ struct GitInputScheme : InputScheme runProgram("git", true, args, {}, true); } - std::optional getSourcePath(const Input & input) override + std::optional 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 commitMsg) override + void putFile( + const Input & input, + const CanonPath & path, + std::string_view contents, + std::optional commitMsg) const override { - auto sourcePath = getSourcePath(input); - assert(sourcePath); + auto root = getSourcePath(input); + 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 }); } } } diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index 4874a43ff..c73505b31 100644 --- a/src/libfetchers/indirect.cc +++ b/src/libfetchers/indirect.cc @@ -1,5 +1,6 @@ #include "fetchers.hh" #include "url-parts.hh" +#include "path.hh" namespace nix::fetchers { diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index a70403660..87fecfca3 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -116,7 +116,7 @@ struct MercurialInputScheme : InputScheme return res; } - std::optional getSourcePath(const Input & input) override + std::optional 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 commitMsg) override + void putFile( + const Input & input, + const CanonPath & path, + std::string_view contents, + std::optional commitMsg) const override { - auto sourcePath = getSourcePath(input); - assert(sourcePath); + auto [isLocal, repoPath] = getActualUrl(input); + 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 getActualUrl(const Input & input) const diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 01f1be978..11c952dc2 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -71,14 +71,28 @@ struct PathInputScheme : InputScheme return true; } - std::optional getSourcePath(const Input & input) override + std::optional getSourcePath(const Input & input) const override { return getStrAttr(input.attrs, "path"); } - void markChangedFile(const Input & input, std::string_view file, std::optional commitMsg) override + void putFile( + const Input & input, + const CanonPath & path, + std::string_view contents, + std::optional 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 fetch(ref store, const Input & _input) override