From 48083028ac075f251442bab34013c92394903aee Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 11 Jul 2025 15:49:27 -0700 Subject: [PATCH 1/2] Add a test case for failing git cache builtins.fetchGit is not using the cached Git directory if packed-references are used. This is because the ref file for the fetchGit `refs/heads/master` is used to check the mtime for whether to cache or not. Let's at least codify this failure in a test case. --- tests/functional/git/meson.build | 6 ++ tests/functional/git/packed-refs-no-cache.sh | 70 ++++++++++++++++++++ tests/functional/meson.build | 1 + 3 files changed, 77 insertions(+) create mode 100644 tests/functional/git/meson.build create mode 100644 tests/functional/git/packed-refs-no-cache.sh diff --git a/tests/functional/git/meson.build b/tests/functional/git/meson.build new file mode 100644 index 000000000..af6882698 --- /dev/null +++ b/tests/functional/git/meson.build @@ -0,0 +1,6 @@ +suites += { + 'name' : 'git', + 'deps' : [], + 'tests' : [ 'packed-refs-no-cache.sh' ], + 'workdir' : meson.current_source_dir(), +} diff --git a/tests/functional/git/packed-refs-no-cache.sh b/tests/functional/git/packed-refs-no-cache.sh new file mode 100644 index 000000000..0f39da775 --- /dev/null +++ b/tests/functional/git/packed-refs-no-cache.sh @@ -0,0 +1,70 @@ +#!/usr/bin/env bash + +source ../common.sh + +requireGit + +clearStoreIfPossible + +# Intentionally not in a canonical form +# See https://github.com/NixOS/nix/issues/6195 +repo=$TEST_ROOT/./git + +export _NIX_FORCE_HTTP=1 + +rm -rf "$repo" "${repo}-tmp" "$TEST_HOME/.cache/nix" + +git init --initial-branch="master" "$repo" +git -C "$repo" config user.email "nix-tests@example.com" +git -C "$repo" config user.name "Nix Tests" + +echo "hello world" > "$repo/hello_world" +git -C "$repo" add hello_world +git -C "$repo" commit -m 'My first commit.' + +# We now do an eval +nix eval --impure --raw --expr "builtins.fetchGit { url = file://$repo; }" + +# test that our eval even worked by checking for the presence of the file +[[ $(nix eval --impure --raw --expr "builtins.readFile ((builtins.fetchGit { url = file://$repo; }) + \"/hello_world\")") = 'hello world' ]] + +# Validate that refs/heads/master exists +shopt -s nullglob +matches=("$TEST_HOME/.cache/nix/gitv3/*/refs/heads/master") +shopt -u nullglob + +if [[ ${#matches[@]} -eq 0 ]]; then + echo "refs/heads/master does not exist." + exit 1 +fi +# pack refs +git -C "$TEST_HOME"/.cache/nix/gitv3/*/ pack-refs --all + +shopt -s nullglob +matches=("$TEST_HOME"/.cache/nix/gitv3/*/refs/heads/master) +shopt -u nullglob + +# ensure refs/heads/master is now gone +if [[ ${#matches[@]} -ne 0 ]]; then + echo "refs/heads/master still exists after pack-refs" + exit 1 +fi + +# create a new commit +echo "hello again" > "$repo/hello_again" +git -C "$repo" add hello_again +git -C "$repo" commit -m 'Second commit.' + +# re-eval — this should return the path to the cached version +store_path=$(nix eval --tarball-ttl 3600 --impure --raw --expr "(builtins.fetchGit { url = file://$repo; }).outPath") +echo "Fetched store path: $store_path" + +# Validate that the new file is *not* there +# FIXME: This is a broken test case and we should swap the assertion here. +if [[ -e "$store_path/hello_again" ]]; then + echo "ERROR: Cached fetchGit should not include the new commit." + exit 0 +else + echo "PASS: New commit was not fetched due to caching (as expected)." + exit 1 +fi \ No newline at end of file diff --git a/tests/functional/meson.build b/tests/functional/meson.build index b03507c91..0e2004219 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -213,6 +213,7 @@ endif subdir('ca') subdir('dyn-drv') subdir('flakes') +subdir('git') subdir('git-hashing') subdir('local-overlay-store') From 0c32b0c8c32b558f6b2c3782172181ba2f2c3558 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 11 Jul 2025 15:57:27 -0700 Subject: [PATCH 2/2] Added comment to test case --- tests/functional/git/packed-refs-no-cache.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/functional/git/packed-refs-no-cache.sh b/tests/functional/git/packed-refs-no-cache.sh index 0f39da775..54e0ab901 100644 --- a/tests/functional/git/packed-refs-no-cache.sh +++ b/tests/functional/git/packed-refs-no-cache.sh @@ -1,5 +1,16 @@ #!/usr/bin/env bash +# Please see https://github.com/NixOS/nix/issues/13457 +# for a higher description of the purpose of the test. +# tl;dr;fetchGit will utilize the git cache and avoid refetching when possible. +# It relies on the presence of either the commit when rev is provided +# or checks if the ref refs/heads/ if ref is provided. +# +# Unfortunately, git can occasionally "pack references" which moves the references +# from individual files to a single unifies file. +# When this occurs, nix can no longer check for the presence of the ref to check +# for the mtime and will refetch unnecessarily. + source ../common.sh requireGit @@ -67,4 +78,4 @@ if [[ -e "$store_path/hello_again" ]]; then else echo "PASS: New commit was not fetched due to caching (as expected)." exit 1 -fi \ No newline at end of file +fi