From 87baf29d6aebcda00eb2c46275afca7d3ce958b5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 15:30:38 +0100 Subject: [PATCH 1/8] Git fetcher: Don't compute revCount if it's already specified We don't care if the user (or more likely the lock file) specifies an incorrect value for revCount, since it doesn't matter for security (unlikely content hashes like narHash). --- src/libfetchers/fetchers.cc | 5 ----- src/libfetchers/git.cc | 9 +++++++-- tests/nixos/tarball-flakes.nix | 1 - 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 7e091ef1071..0d8bd4514fe 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -282,11 +282,6 @@ void Input::checkLocks(Input specified, Input & result) if (result.getRev() != prevRev) throw Error("'rev' attribute mismatch in input '%s', expected %s", result.to_string(), prevRev->gitRev()); } - - if (auto prevRevCount = specified.getRevCount()) { - if (result.getRevCount() != prevRevCount) - throw Error("'revCount' attribute mismatch in input '%s', expected %d", result.to_string(), *prevRevCount); - } } std::pair, Input> Input::getAccessor(const Settings & settings, Store & store) const diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 75e3f121481..f4d492308d7 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -891,8 +891,13 @@ struct GitInputScheme : InputScheme input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); - if (!getShallowAttr(input)) - input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); + if (!getShallowAttr(input)) { + /* Skip revCount computation if it's already supplied by the caller. + We don't care if they specify an incorrect value; it doesn't + matter for security, unlike narHash. */ + if (!input.attrs.contains("revCount")) + input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); + } printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg()); diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix index 26c20cb1aef..b0402412d85 100644 --- a/tests/nixos/tarball-flakes.nix +++ b/tests/nixos/tarball-flakes.nix @@ -99,7 +99,6 @@ in # Check that fetching fails if we provide incorrect attributes. machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?rev=493300eb13ae6fb387fbd47bf54a85915acc31c0") - machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?revCount=789") machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?narHash=sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=") ''; From 8f32f28ebdaf3272d386756724d345883eec760c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 15:39:19 +0100 Subject: [PATCH 2/8] Git fetcher: Don't compute lastModified if it's already specified Same as revCount. --- src/libfetchers/fetchers.cc | 9 --------- src/libfetchers/git.cc | 10 ++++++---- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 0d8bd4514fe..48a23ad3693 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -269,15 +269,6 @@ void Input::checkLocks(Input specified, Input & result) } } - if (auto prevLastModified = specified.getLastModified()) { - if (result.getLastModified() != prevLastModified) - throw Error( - "'lastModified' attribute mismatch in input '%s', expected %d, got %d", - result.to_string(), - *prevLastModified, - result.getLastModified().value_or(-1)); - } - if (auto prevRev = specified.getRev()) { if (result.getRev() != prevRev) throw Error("'rev' attribute mismatch in input '%s', expected %s", result.to_string(), prevRev->gitRev()); diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index f4d492308d7..96008d45bd2 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -889,12 +889,14 @@ struct GitInputScheme : InputScheme auto rev = *input.getRev(); - input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); + /* Skip lastModified computation if it's already supplied by the caller. + We don't care if they specify an incorrect value; it doesn't + matter for security, unlike narHash. */ + if (!input.attrs.contains("lastModified")) + input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); if (!getShallowAttr(input)) { - /* Skip revCount computation if it's already supplied by the caller. - We don't care if they specify an incorrect value; it doesn't - matter for security, unlike narHash. */ + /* Like lastModified, skip revCount if supplied by the caller. */ if (!input.attrs.contains("revCount")) input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); } From 4ecc09c43f5f42808134ec8144ac2afdcce0fed2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 19:58:08 +0100 Subject: [PATCH 3/8] Make content-encoding test more reliable --- tests/nixos/content-encoding.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/nixos/content-encoding.nix b/tests/nixos/content-encoding.nix index debee377bdf..1e188cb060b 100644 --- a/tests/nixos/content-encoding.nix +++ b/tests/nixos/content-encoding.nix @@ -131,6 +131,7 @@ in start_all() machine.wait_for_unit("nginx.service") + machine.wait_for_open_port(80) # Original test: zstd archive with gzip content-encoding # Make sure that the file is properly compressed as the test would be meaningless otherwise From 50b013f6122077886a5f3bf8487ba10be36d7b28 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 16:37:43 +0100 Subject: [PATCH 4/8] Remove fetchTree 'shallow' hack builtins.fetchTree was setting `shallow = true` when fetching from git. That's bad because it makes it behave inconsistently from non-fetchTree fetches, e.g. when updating an input. Instead, the Git fetcher now will do a shallow fetch automatically if `revCount` is already set (e.g. when fetching a lock). Fixes https://github.com/NixOS/nix/issues/14588. --- src/libexpr/primops/fetchTree.cc | 5 ----- src/libfetchers/git.cc | 34 ++++++++++++++++++++------------ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 1614fcc595d..7d408e56a27 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -151,11 +151,6 @@ static void fetchTree( attrs.emplace("exportIgnore", Explicit{true}); } - // fetchTree should fetch git repos with shallow = true by default - if (type == "git" && !params.isFetchGit && !attrs.contains("shallow")) { - attrs.emplace("shallow", Explicit{true}); - } - if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) state.error("argument 'name' isn’t supported in call to '%s'", fetcher) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 96008d45bd2..d67eac0c9ef 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -778,6 +778,16 @@ struct GitInputScheme : InputScheme } } + /** + * Decide whether we can do a shallow clone, which is faster. This is possible if the user explicitly specified + * `shallow = true`, or if we already have a `revCount`. + */ + bool canDoShallow(const Input & input) const + { + bool shallow = getShallowAttr(input); + return shallow || input.getRevCount().has_value(); + } + std::pair, Input> getAccessorFromCommit(const Settings & settings, Store & store, RepoInfo & repoInfo, Input && input) const { @@ -786,7 +796,7 @@ struct GitInputScheme : InputScheme auto origRev = input.getRev(); auto originalRef = input.getRef(); - bool shallow = getShallowAttr(input); + bool shallow = canDoShallow(input); auto ref = originalRef ? *originalRef : getDefaultRef(repoInfo, shallow); input.attrs.insert_or_assign("ref", ref); @@ -831,7 +841,6 @@ struct GitInputScheme : InputScheme } if (doFetch) { - bool shallow = getShallowAttr(input); try { auto fetchRef = getAllRefsAttr(input) ? "refs/*:refs/*" : input.getRev() ? input.getRev()->gitRev() @@ -878,13 +887,6 @@ struct GitInputScheme : InputScheme auto repo = GitRepo::openRepo(repoDir); - auto isShallow = repo->isShallow(); - - if (isShallow && !getShallowAttr(input)) - throw Error( - "'%s' is a shallow Git repository, but shallow repositories are only allowed when `shallow = true;` is specified", - repoInfo.locationToArg()); - // FIXME: check whether rev is an ancestor of ref? auto rev = *input.getRev(); @@ -895,10 +897,16 @@ struct GitInputScheme : InputScheme if (!input.attrs.contains("lastModified")) input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev)); - if (!getShallowAttr(input)) { - /* Like lastModified, skip revCount if supplied by the caller. */ - if (!input.attrs.contains("revCount")) - input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); + /* Like lastModified, skip revCount if supplied by the caller. */ + if (!shallow && !input.attrs.contains("revCount")) { + auto isShallow = repo->isShallow(); + + if (isShallow && !shallow) + throw Error( + "'%s' is a shallow Git repository, but shallow repositories are only allowed when `shallow = true;` is specified", + repoInfo.locationToArg()); + + input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev)); } printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg()); From 5b7badd0084be58878d250fe200039df339e6a8f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 16:56:11 +0100 Subject: [PATCH 5/8] Use non-shallow cache repo if it contains the requested commit This fixes the issue where updating a Git input does a non-shallow fetch, and then a subsequent eval does a shallow refetch because the revCount is already known. Now the subsequent eval will use the repo used in the first fetch. --- src/libfetchers/git.cc | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d67eac0c9ef..ad498f854fc 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -807,11 +807,27 @@ struct GitInputScheme : InputScheme if (!input.getRev()) input.attrs.insert_or_assign("rev", GitRepo::openRepo(repoDir)->resolveRef(ref).gitRev()); } else { + auto rev = input.getRev(); auto repoUrl = std::get(repoInfo.location); std::filesystem::path cacheDir = getCachePath(repoUrl.to_string(), shallow); repoDir = cacheDir; repoInfo.gitDir = "."; + /* If shallow = false, but we have a non-shallow repo that already contains the desired rev, then use that + * repo instead. */ + std::filesystem::path cacheDirNonShallow = getCachePath(repoUrl.to_string(), false); + if (rev && shallow && pathExists(cacheDirNonShallow)) { + auto nonShallowRepo = GitRepo::openRepo(cacheDirNonShallow, true, true); + if (nonShallowRepo->hasObject(*rev)) { + debug( + "using non-shallow cached repo for '%s' since it contains rev '%s'", + repoUrl.to_string(), + rev->gitRev()); + repoDir = cacheDirNonShallow; + goto have_rev; + } + } + std::filesystem::create_directories(cacheDir.parent_path()); PathLocks cacheDirLock({cacheDir.string()}); @@ -827,7 +843,7 @@ struct GitInputScheme : InputScheme /* If a rev was specified, we need to fetch if it's not in the repo. */ - if (auto rev = input.getRev()) { + if (rev) { doFetch = !repo->hasObject(*rev); } else { if (getAllRefsAttr(input)) { @@ -868,7 +884,7 @@ struct GitInputScheme : InputScheme warn("could not update cached head '%s' for '%s'", ref, repoInfo.locationToArg()); } - if (auto rev = input.getRev()) { + if (rev) { if (!repo->hasObject(*rev)) throw Error( "Cannot find Git revision '%s' in ref '%s' of repository '%s'! " @@ -885,6 +901,7 @@ struct GitInputScheme : InputScheme // the remainder } + have_rev: auto repo = GitRepo::openRepo(repoDir); // FIXME: check whether rev is an ancestor of ref? From 8cd0b35985463ce52144403358b98fd604134ff6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 18 Nov 2025 17:45:35 +0100 Subject: [PATCH 6/8] Add test --- tests/functional/flakes/common.sh | 2 ++ tests/functional/flakes/meson.build | 1 + tests/functional/flakes/shallow.sh | 35 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 tests/functional/flakes/shallow.sh diff --git a/tests/functional/flakes/common.sh b/tests/functional/flakes/common.sh index 77bc030605f..1c7c5664da0 100644 --- a/tests/functional/flakes/common.sh +++ b/tests/functional/flakes/common.sh @@ -32,6 +32,8 @@ writeSimpleFlake() { baseName = builtins.baseNameOf ./.; root = ./.; + + number = 123; }; } EOF diff --git a/tests/functional/flakes/meson.build b/tests/functional/flakes/meson.build index de76a55804a..ebc1bb41556 100644 --- a/tests/functional/flakes/meson.build +++ b/tests/functional/flakes/meson.build @@ -34,6 +34,7 @@ suites += { 'source-paths.sh', 'old-lockfiles.sh', 'trace-ifd.sh', + 'shallow.sh', ], 'workdir' : meson.current_source_dir(), } diff --git a/tests/functional/flakes/shallow.sh b/tests/functional/flakes/shallow.sh new file mode 100644 index 00000000000..82a3789b567 --- /dev/null +++ b/tests/functional/flakes/shallow.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +export _NIX_FORCE_HTTP=1 + +source ./common.sh + +requireGit +TODO_NixOS + +createFlake1 + +repoDir="$TEST_ROOT/repo" +mkdir -p "$repoDir" +echo "# foo" >> "$flake1Dir/flake.nix" +git -C "$flake1Dir" commit -a -m bla + +cat > "$repoDir"/flake.nix < Date: Tue, 18 Nov 2025 22:46:23 +0100 Subject: [PATCH 7/8] Fix test --- tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix b/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix index f635df1f879..a204caedd57 100644 --- a/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix +++ b/tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix @@ -1,5 +1,5 @@ { - description = "fetchTree fetches git repos shallowly by default"; + description = "fetchTree fetches git repos shallowly if possible"; script = '' # purge nix git cache to make sure we start with a clean slate client.succeed("rm -rf ~/.cache/nix") @@ -28,6 +28,7 @@ type = "git"; url = "{repo.remote}"; rev = "{commit2_rev}"; + revCount = 1234; }} """ From ffc6252e361a4caa4023b579bf4fc7387fab15b0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 24 Nov 2025 19:56:05 +0100 Subject: [PATCH 8/8] Expose builtins.fetchFinalTree to the user This may be more efficient than fetchTree since it allows substitution from binary caches if `narHash` is specified in the input attributes. This is what call-flake.nix already used internally. --- src/libexpr/primops/fetchTree.cc | 7 +++++-- src/libflake/call-flake.nix | 5 +---- src/libflake/flake.cc | 5 +---- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 7d408e56a27..c81db90b946 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -362,10 +362,13 @@ void prim_fetchFinalTree(EvalState & state, const PosIdx pos, Value ** args, Val } static RegisterPrimOp primop_fetchFinalTree({ - .name = "fetchFinalTree", + .name = "__fetchFinalTree", .args = {"input"}, + .doc = R"( + Like `fetchTree`, but does not return any additional fetcher attributes (like `revCount`). + This allows inputs to be substituted if `narHash` is specified. + )", .fun = prim_fetchFinalTree, - .internal = true, }); static void fetch( diff --git a/src/libflake/call-flake.nix b/src/libflake/call-flake.nix index ed7947e0601..3fc3353cca2 100644 --- a/src/libflake/call-flake.nix +++ b/src/libflake/call-flake.nix @@ -10,9 +10,6 @@ lockFileStr: # unlocked trees. overrides: -# This is `prim_fetchFinalTree`. -fetchTreeFinal: - let inherit (builtins) mapAttrs; @@ -52,7 +49,7 @@ let else # FIXME: remove obsolete node.info. # Note: lock file entries are always final. - fetchTreeFinal (node.info or { } // removeAttrs node.locked [ "dir" ]); + builtins.fetchFinalTree (node.info or { } // removeAttrs node.locked [ "dir" ]); subdir = overrides.${key}.dir or node.locked.dir or ""; diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 9f7476bd0e2..592bbaed25f 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -968,10 +968,7 @@ void callFlake(EvalState & state, const LockedFlake & lockedFlake, Value & vRes) auto vLocks = state.allocValue(); vLocks->mkString(lockFileStr, state.mem); - auto vFetchFinalTree = get(state.internalPrimOps, "fetchFinalTree"); - assert(vFetchFinalTree); - - Value * args[] = {vLocks, &vOverrides, *vFetchFinalTree}; + Value * args[] = {vLocks, &vOverrides}; state.callFunction(*vCallFlake, args, vRes, noPos); }