From f780539406c1cca65ef3d9167419725879a26804 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 27 Jul 2022 14:26:15 +0200 Subject: [PATCH] Add abstract interface for writing files to an Input --- src/libexpr/flake/flake.cc | 90 +++++++++++++++-------------------- src/libexpr/flake/lockfile.cc | 6 --- src/libexpr/flake/lockfile.hh | 4 -- src/libfetchers/fetchers.cc | 15 ++++-- src/libfetchers/fetchers.hh | 11 +++-- src/libfetchers/git.cc | 21 ++++++-- src/libfetchers/mercurial.cc | 21 ++++++-- src/libfetchers/path.cc | 18 +++++-- 8 files changed, 102 insertions(+), 84 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 50bac8d4f..5304f6b5e 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -621,65 +621,53 @@ LockedFlake lockFlake( auto diff = LockFile::diff(oldLockFile, newLockFile); if (lockFlags.writeLockFile) { - if (auto sourcePath = topRef.input.getAccessor(state.store).first->root().getPhysicalPath()) { - 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); - } else { - if (!lockFlags.updateLockFile) - throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef); + 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); + } else { + if (!lockFlags.updateLockFile) + throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef); - CanonPath flakeDir(*sourcePath); + auto path = flake->path.parent() + "flake.lock"; - auto relPath = flakeDir + "flake.lock"; + bool lockFileExists = path.pathExists(); - auto path = flakeDir + "flake.lock"; + if (lockFileExists) { + auto s = chomp(diff); + if (s.empty()) + warn("updating lock file '%s'", path); + else + warn("updating lock file '%s':\n%s", path, s); + } else + warn("creating lock file '%s'", path); - bool lockFileExists = pathExists(path.abs()); + std::optional commitMessage = std::nullopt; + if (lockFlags.commitLockFile) { + std::string cm; - if (lockFileExists) { - auto s = chomp(diff); - if (s.empty()) - warn("updating lock file '%s'", path); - else - warn("updating lock file '%s':\n%s", path, s); - } else - warn("creating lock file '%s'", path); + cm = fetchSettings.commitLockFileSummary.get(); - newLockFile.write(path.abs()); + if (cm == "") + cm = fmt("%s: %s", path.path.rel(), lockFileExists ? "Update" : "Add"); - std::optional commitMessage = std::nullopt; - if (lockFlags.commitLockFile) { - 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; - } - - 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... */ - auto prevLockedRef = flake->lockedRef; - flake = std::make_unique(getFlake(state, topRef, useRegistries)); - - if (lockFlags.commitLockFile && - flake->lockedRef.input.getRev() && - prevLockedRef.input.getRev() != flake->lockedRef.input.getRev()) - warn("committed new revision '%s'", flake->lockedRef.input.getRev()->gitRev()); + cm += "\n\nFlake lock file updates:\n\n"; + cm += filterANSIEscapes(diff, true); + commitMessage = cm; } - } else - throw Error("cannot write modified lock file of flake '%s' (use '--no-write-lock-file' to ignore)", topRef); + + topRef.input.putFile(path.path, fmt("%s\n", newLockFile), commitMessage); + + /* Rewriting the lockfile changed the top-level + repo, so we should re-read it. FIXME: we could + also just clear the 'rev' field... */ + auto prevLockedRef = flake->lockedRef; + flake = std::make_unique(getFlake(state, topRef, useRegistries)); + + if (lockFlags.commitLockFile && + flake->lockedRef.input.getRev() && + prevLockedRef.input.getRev() != flake->lockedRef.input.getRev()) + warn("committed new revision '%s'", flake->lockedRef.input.getRev()->gitRev()); + } } else { warn("not writing modified lock file of flake '%s':\n%s", topRef, chomp(diff)); flake->forceDirty = true; diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index 8bfb9b2d2..166abe243 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -190,12 +190,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 LockFile::isUnlocked() const { std::unordered_set> nodes; diff --git a/src/libexpr/flake/lockfile.hh b/src/libexpr/flake/lockfile.hh index 8f8ff2793..9edd2ef01 100644 --- a/src/libexpr/flake/lockfile.hh +++ b/src/libexpr/flake/lockfile.hh @@ -59,10 +59,6 @@ struct LockFile std::string to_string() const; - static LockFile read(const Path & path); - - void write(const Path & path) const; - /* Check whether this lock file has any unlocked inputs. If so, return one. */ std::optional isUnlocked() const; diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 28ccfc0f9..66063a324 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -187,12 +187,13 @@ void Input::clone(const Path & destDir) const scheme->clone(*this, destDir); } -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 @@ -278,9 +279,13 @@ Input InputScheme::applyOverrides( return input; } -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) diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 6c14a92ec..97bc4c20f 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -79,8 +79,9 @@ public: void clone(const Path & destDir) const; - void markChangedFile( - std::string_view file, + void putFile( + const CanonPath & path, + std::string_view contents, std::optional commitMsg) const; std::string getName() const; @@ -130,7 +131,11 @@ struct InputScheme virtual void clone(const Input & input, const Path & destDir); - 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; /* Note: the default implementations of fetchToStore() and getAccessor() are defined using the other, so implementations diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 0df8c2976..f1cbbdae3 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -245,17 +245,28 @@ struct GitInputScheme : InputScheme runProgram("git", true, args); } - 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 { auto repoInfo = getRepoInfo(input); - assert(repoInfo.isLocal); + if (!repoInfo.isLocal) + throw Error("cannot commit '%s' to Git repository '%s' because it's not a working tree", path, input.to_string()); + + auto absPath = CanonPath(repoInfo.url) + path; + + // FIXME: make sure that absPath is not a symlink that escapes + // the repo. + writeFile(absPath.abs(), contents); runProgram("git", true, - { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "add", "--force", "--intent-to-add", "--", std::string(file) }); + { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "add", "--force", "--intent-to-add", "--", std::string(path.rel()) }); if (commitMsg) runProgram("git", true, - { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "commit", std::string(file), "-m", *commitMsg }); + { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "commit", std::string(path.rel()), "-m", *commitMsg }); } struct RepoInfo @@ -292,7 +303,7 @@ struct GitInputScheme : InputScheme std::string gitDir = ".git"; }; - RepoInfo getRepoInfo(const Input & input) + RepoInfo getRepoInfo(const Input & input) const { auto checkHashType = [&](const std::optional & hash) { diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 13eded9b7..a10fc0a63 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -116,18 +116,29 @@ struct MercurialInputScheme : InputScheme return res; } - 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 { - auto [isLocal, path] = getActualUrl(input); - assert(isLocal); + 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; + + // FIXME: make sure that absPath is not a symlink that escapes + // the repo. + writeFile(absPath.abs(), contents); // FIXME: shut up if file is already tracked. runHg( - { "add", path + "/" + file }); + { "add", absPath.abs() }); if (commitMsg) runHg( - { "commit", path + "/" + 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 59e1b4c72..ea056861b 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -80,12 +80,20 @@ struct PathInputScheme : InputScheme return true; } - 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 { - // nothing to do + auto absPath = CanonPath(getAbsPath(input)) + path; + + // FIXME: make sure that absPath is not a symlink that escapes + // the repo. + writeFile(absPath.abs(), contents); } - CanonPath getAbsPath(ref store, const Input & input) const + CanonPath getAbsPath(const Input & input) const { auto path = getStrAttr(input.attrs, "path"); @@ -97,7 +105,7 @@ struct PathInputScheme : InputScheme std::pair, Input> getAccessor(ref store, const Input & input) override { - auto absPath = getAbsPath(store, input); + auto absPath = getAbsPath(input); auto input2(input); input2.attrs.emplace("path", (std::string) absPath.abs()); return {makeFSInputAccessor(absPath), std::move(input2)}; @@ -109,7 +117,7 @@ struct PathInputScheme : InputScheme locked, so just use the path as its fingerprint. Maybe we should restrict this to CA paths but that's not super-important. */ - auto path = getAbsPath(store, input); + auto path = getAbsPath(input); if (store->isInStore(path.abs())) return path.abs(); return std::nullopt;