From 102d90ebf07b1f268a3551daf5457131ae063d4a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 30 Jan 2025 11:27:24 +0100 Subject: [PATCH 1/2] Fix duplicate setPathDisplay() Fixes messages like 'copying /tmp/repo/tmp/repo to the store'. The PosixSourceAccessor already sets the prefix. Setting the prefix twice shouldn't be a problem, but GitRepoImpl::getAccessor() returns a wrapped accessor so it's not actually idempotent. --- src/libfetchers/git.cc | 2 -- tests/functional/fetchGit.sh | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index b411e112f..e8698709a 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -737,8 +737,6 @@ struct GitInputScheme : InputScheme exportIgnore, makeNotAllowedError(repoInfo.locationToArg())); - accessor->setPathDisplay(repoInfo.locationToArg()); - /* If the repo has submodules, return a mounted input accessor consisting of the accessor for the top-level repo and the accessors for the submodule workdirs. */ diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index 78925b5cd..f3eda54dc 100755 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -37,6 +37,7 @@ nix-instantiate --eval -E "builtins.readFile ((builtins.fetchGit file://$TEST_RO # Fetch a worktree. unset _NIX_FORCE_HTTP +expectStderr 0 nix eval -vvvv --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath" | grepQuiet "copying '$TEST_ROOT/worktree/' to the store" path0=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath") path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath") [[ $path0 = $path0_ ]] From 3032512425a09fc58f2d658442043894e0aab256 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 30 Jan 2025 12:41:02 +0100 Subject: [PATCH 2/2] =?UTF-8?q?GitExportIgnoreSourceAccessor:=20Don't=20sh?= =?UTF-8?q?ow=20=C2=ABunknown=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In general we should set the path display prefix on the inner accessor, so we now pass the display prefix to getAccessor(). --- src/libfetchers/git-utils.cc | 21 +++++++++++++-------- src/libfetchers/git-utils.hh | 5 ++++- src/libfetchers/git.cc | 4 +--- src/libfetchers/github.cc | 7 ++++--- src/libfetchers/tarball.cc | 12 +++++++----- 5 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 6a75daf61..a6b13fb31 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -508,7 +508,10 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this */ ref getRawAccessor(const Hash & rev); - ref getAccessor(const Hash & rev, bool exportIgnore) override; + ref getAccessor( + const Hash & rev, + bool exportIgnore, + std::string displayPrefix) override; ref getAccessor(const WorkdirInfo & wd, bool exportIgnore, MakeNotAllowedError e) override; @@ -627,7 +630,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this Hash treeHashToNarHash(const Hash & treeHash) override { - auto accessor = getAccessor(treeHash, false); + auto accessor = getAccessor(treeHash, false, ""); fetchers::Cache::Key cacheKey{"treeHashToNarHash", {{"treeHash", treeHash.gitRev()}}}; @@ -1194,16 +1197,18 @@ ref GitRepoImpl::getRawAccessor(const Hash & rev) return make_ref(self, rev); } -ref GitRepoImpl::getAccessor(const Hash & rev, bool exportIgnore) +ref GitRepoImpl::getAccessor( + const Hash & rev, + bool exportIgnore, + std::string displayPrefix) { auto self = ref(shared_from_this()); ref rawGitAccessor = getRawAccessor(rev); - if (exportIgnore) { + rawGitAccessor->setPathDisplay(std::move(displayPrefix)); + if (exportIgnore) return make_ref(self, rawGitAccessor, rev); - } - else { + else return rawGitAccessor; - } } ref GitRepoImpl::getAccessor(const WorkdirInfo & wd, bool exportIgnore, MakeNotAllowedError makeNotAllowedError) @@ -1236,7 +1241,7 @@ std::vector> GitRepoImpl::getSubmodules /* Read the .gitmodules files from this revision. */ CanonPath modulesFile(".gitmodules"); - auto accessor = getAccessor(rev, exportIgnore); + auto accessor = getAccessor(rev, exportIgnore, ""); if (!accessor->pathExists(modulesFile)) return {}; /* Parse it and get the revision of each submodule. */ diff --git a/src/libfetchers/git-utils.hh b/src/libfetchers/git-utils.hh index ff115143f..9677f5079 100644 --- a/src/libfetchers/git-utils.hh +++ b/src/libfetchers/git-utils.hh @@ -86,7 +86,10 @@ struct GitRepo virtual bool hasObject(const Hash & oid) = 0; - virtual ref getAccessor(const Hash & rev, bool exportIgnore) = 0; + virtual ref getAccessor( + const Hash & rev, + bool exportIgnore, + std::string displayPrefix) = 0; virtual ref getAccessor(const WorkdirInfo & wd, bool exportIgnore, MakeNotAllowedError makeNotAllowedError) = 0; diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index e8698709a..e40afb865 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -672,9 +672,7 @@ struct GitInputScheme : InputScheme verifyCommit(input, repo); bool exportIgnore = getExportIgnoreAttr(input); - auto accessor = repo->getAccessor(rev, exportIgnore); - - accessor->setPathDisplay("«" + input.to_string() + "»"); + auto accessor = repo->getAccessor(rev, exportIgnore, "«" + input.to_string() + "»"); /* If the repo has submodules, fetch them and return a mounted input accessor consisting of the accessor for the top-level diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 185941988..ec469df7c 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -294,9 +294,10 @@ struct GitArchiveInputScheme : InputScheme #endif input.attrs.insert_or_assign("lastModified", uint64_t(tarballInfo.lastModified)); - auto accessor = getTarballCache()->getAccessor(tarballInfo.treeHash, false); - - accessor->setPathDisplay("«" + input.to_string() + "»"); + auto accessor = getTarballCache()->getAccessor( + tarballInfo.treeHash, + false, + "«" + input.to_string() + "»"); return {accessor, input}; } diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 28574e7b1..699612e25 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -105,7 +105,8 @@ DownloadFileResult downloadFile( static DownloadTarballResult downloadTarball_( const std::string & url, - const Headers & headers) + const Headers & headers, + const std::string & displayPrefix) { Cache::Key cacheKey{"tarball", {{"url", url}}}; @@ -118,7 +119,7 @@ static DownloadTarballResult downloadTarball_( .treeHash = treeHash, .lastModified = (time_t) getIntAttr(infoAttrs, "lastModified"), .immutableUrl = maybeGetStrAttr(infoAttrs, "immutableUrl"), - .accessor = getTarballCache()->getAccessor(treeHash, false), + .accessor = getTarballCache()->getAccessor(treeHash, false, displayPrefix), }; }; @@ -371,9 +372,10 @@ struct TarballInputScheme : CurlInputScheme { auto input(_input); - auto result = downloadTarball_(getStrAttr(input.attrs, "url"), {}); - - result.accessor->setPathDisplay("«" + input.to_string() + "»"); + auto result = downloadTarball_( + getStrAttr(input.attrs, "url"), + {}, + "«" + input.to_string() + "»"); if (result.immutableUrl) { auto immutableInput = Input::fromURL(*input.settings, *result.immutableUrl);