diff options
| -rw-r--r-- | doc/manual/rl-next/fetchGit-regression.md | 23 | ||||
| -rw-r--r-- | src/libexpr/primops/fetchTree.cc | 3 | ||||
| -rw-r--r-- | src/libfetchers/git.cc | 107 | ||||
| -rw-r--r-- | tests/functional/fetchGit.sh | 45 |
4 files changed, 157 insertions, 21 deletions
diff --git a/doc/manual/rl-next/fetchGit-regression.md b/doc/manual/rl-next/fetchGit-regression.md new file mode 100644 index 000000000..f6b4fb9e5 --- /dev/null +++ b/doc/manual/rl-next/fetchGit-regression.md @@ -0,0 +1,23 @@ +--- +synopsis: restore backwards-compatibility of `builtins.fetchGit` with Nix 2.3 +issues: [5291, 5128] +credits: [ma27] +category: Fixes +--- + +Compatibility with `builtins.fetchGit` from Nix 2.3 has been restored as follows: + +* Until now, each `ref` was prefixed with `refs/heads` unless it starts with `refs/` itself. + + Now, this is not done if the `ref` looks like a commit hash. + +* Specifying `builtins.fetchGit { ref = "a-tag"; /* … */ }` was broken because `refs/heads` was appended. + + Now, the fetcher doesn't turn a ref into `refs/heads/ref`, but into `refs/*/ref`. That way, + the value in `ref` can be either a tag or a branch. + +* The ref resolution happens the same way as in git: + + * If `refs/ref` exists, it's used. + * If a tag `refs/tags/ref` exists, it's used. + * If a branch `refs/heads/ref` exists, it's used. diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index b0e14a26e..c98fe2a03 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -394,7 +394,8 @@ static RegisterPrimOp primop_fetchGit({ [Git reference]: https://git-scm.com/book/en/v2/Git-Internals-Git-References By default, the `ref` value is prefixed with `refs/heads/`. - As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/`. + As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/` or + if `ref` looks like a commit hash for backwards compatibility with CppNix 2.3. - `submodules` (default: `false`) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 7d16d3f57..da60bf331 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -1,3 +1,4 @@ +#include "error.hh" #include "fetchers.hh" #include "cache.hh" #include "globals.hh" @@ -257,6 +258,28 @@ std::pair<StorePath, Input> fetchFromWorkdir(ref<Store> store, Input & input, co } } // end namespace +static std::optional<Path> resolveRefToCachePath( + Input & input, + const Path & cacheDir, + std::vector<Path> & gitRefFileCandidates, + std::function<bool(const Path&)> condition) +{ + if (input.getRef()->starts_with("refs/")) { + Path fullpath = cacheDir + "/" + *input.getRef(); + if (condition(fullpath)) { + return fullpath; + } + } + + for (auto & candidate : gitRefFileCandidates) { + if (condition(candidate)) { + return candidate; + } + } + + return std::nullopt; +} + struct GitInputScheme : InputScheme { std::optional<Input> inputFromURL(const ParsedURL & url, bool requireTree) const override @@ -539,10 +562,13 @@ struct GitInputScheme : InputScheme runProgram("git", true, { "-c", "init.defaultBranch=" + gitInitialBranch, "init", "--bare", repoDir }); } - Path localRefFile = - input.getRef()->compare(0, 5, "refs/") == 0 - ? cacheDir + "/" + *input.getRef() - : cacheDir + "/refs/heads/" + *input.getRef(); + std::vector<Path> gitRefFileCandidates; + for (auto & infix : {"", "tags/", "heads/"}) { + Path p = cacheDir + "/refs/" + infix + *input.getRef(); + gitRefFileCandidates.push_back(p); + } + + Path localRefFile; bool doFetch; time_t now = time(0); @@ -564,29 +590,70 @@ struct GitInputScheme : InputScheme if (allRefs) { doFetch = true; } else { - /* If the local ref is older than ‘tarball-ttl’ seconds, do a - git fetch to update the local ref to the remote ref. */ - struct stat st; - doFetch = stat(localRefFile.c_str(), &st) != 0 || - !isCacheFileWithinTtl(now, st); + std::function<bool(const Path&)> condition; + condition = [&now](const Path & path) { + /* If the local ref is older than ‘tarball-ttl’ seconds, do a + git fetch to update the local ref to the remote ref. */ + struct stat st; + return stat(path.c_str(), &st) == 0 && + isCacheFileWithinTtl(now, st); + }; + if (auto result = resolveRefToCachePath( + input, + cacheDir, + gitRefFileCandidates, + condition + )) { + localRefFile = *result; + doFetch = false; + } else { + doFetch = true; + } } } + // When having to fetch, we don't know `localRefFile` yet. + // Because git needs to figure out what we're fetching + // (i.e. is it a rev? a branch? a tag?) if (doFetch) { Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching Git repository '%s'", actualUrl)); - // FIXME: git stderr messes up our progress indicator, so - // we're using --quiet for now. Should process its stderr. + auto ref = input.getRef(); + std::string fetchRef; + if (allRefs) { + fetchRef = "refs/*"; + } else if ( + ref->starts_with("refs/") + || *ref == "HEAD" + || std::regex_match(*ref, revRegex)) + { + fetchRef = *ref; + } else { + fetchRef = "refs/*/" + *ref; + } + try { - auto ref = input.getRef(); - auto fetchRef = allRefs - ? "refs/*" - : ref->compare(0, 5, "refs/") == 0 - ? *ref - : ref == "HEAD" - ? *ref - : "refs/heads/" + *ref; - runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, true); + Finally finally([&]() { + if (auto p = resolveRefToCachePath( + input, + cacheDir, + gitRefFileCandidates, + pathExists + )) { + localRefFile = *p; + } + }); + + // FIXME: git stderr messes up our progress indicator, so + // we're using --quiet for now. Should process its stderr. + runProgram("git", true, { + "-C", repoDir, + "--git-dir", gitDir, + "fetch", + "--quiet", + "--force", + "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) + }, true); } catch (Error & e) { if (!pathExists(localRefFile)) throw; warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index 2c00facc2..492c57602 100644 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -53,8 +53,17 @@ out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = \"file://$repo\" [[ $status == 1 ]] [[ $out =~ 'Cannot find Git revision' ]] +# allow revs as refs (for 2.3 compat) [[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]] +rm -rf "$TEST_ROOT/test-home" +[[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]] + +rm -rf "$TEST_ROOT/test-home" +out=$(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; ref = \"lolkek\"; } + \"/differentbranch\")" 2>&1) || status=$? +[[ $status == 1 ]] +[[ $out =~ 'Cannot find Git revision' ]] + # In pure eval mode, fetchGit without a revision should fail. [[ $(nix eval --impure --raw --expr "builtins.readFile (fetchGit \"file://$repo\" + \"/hello\")") = world ]] (! nix eval --raw --expr "builtins.readFile (fetchGit \"file://$repo\" + \"/hello\")") @@ -228,6 +237,12 @@ export _NIX_FORCE_HTTP=1 rev_tag1_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag1\"; }).rev") rev_tag1=$(git -C $repo rev-parse refs/tags/tag1) [[ $rev_tag1_nix = $rev_tag1 ]] + +# Allow fetching tags w/o specifying refs/tags +rm -rf "$TEST_ROOT/test-home" +rev_tag1_nix_alt=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"tag1\"; }).rev") +[[ $rev_tag1_nix_alt = $rev_tag1 ]] + rev_tag2_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag2\"; }).rev") rev_tag2=$(git -C $repo rev-parse refs/tags/tag2) [[ $rev_tag2_nix = $rev_tag2 ]] @@ -254,3 +269,33 @@ git -C "$repo" add hello .gitignore git -C "$repo" commit -m 'Bla1' cd "$repo" path11=$(nix eval --impure --raw --expr "(builtins.fetchGit ./.).outPath") + +# test behavior if both branch and tag with same name exist +repo="$TEST_ROOT/git" +rm -rf "$repo"/.git +git init "$repo" +git -C "$repo" config user.email "foobar@example.com" +git -C "$repo" config user.name "Foobar" + +touch "$repo"/test +echo "hello world" > "$repo"/test +git -C "$repo" checkout -b branch +git -C "$repo" add test + +git -C "$repo" commit -m "Init" + +git -C "$repo" tag branch + +echo "goodbye world" > "$repo"/test +git -C "$repo" add test +git -C "$repo" commit -m "Update test" + +path12=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"branch\"; }).outPath") +[[ "$(cat "$path12"/test)" =~ 'hello world' ]] +[[ "$(cat "$repo"/test)" =~ 'goodbye world' ]] + +path13=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/heads/branch\"; }).outPath") +[[ "$(cat "$path13"/test)" =~ 'goodbye world' ]] + +path14=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/branch\"; }).outPath") +[[ "$path14" = "$path12" ]] |