aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMaximilian Bosch <maximilian@mbosch.me>2024-09-29 08:56:16 +0000
committerGerrit Code Review <gerrit@localhost>2024-09-29 08:56:16 +0000
commit289e7a6b5a84c64142a10bdd875f8a06e3987579 (patch)
treeed431802ae10ab0fc30e7327a202b0898561589f
parentf12b60273b7e93184dd175ef9daa75ec490cbd5e (diff)
parent04daff94e347c03e466577c8c00414f03e27a96f (diff)
Merge "libfetchers/git: restore compat with `builtins.fetchGit` from 2.3" into main
-rw-r--r--doc/manual/rl-next/fetchGit-regression.md23
-rw-r--r--src/libexpr/primops/fetchTree.cc3
-rw-r--r--src/libfetchers/git.cc107
-rw-r--r--tests/functional/fetchGit.sh45
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" ]]