Skip to content

Add allRefs = true argument to fetchGit to explicitly perform a full checkout #3814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edolstra merged 4 commits into from
Dec 22, 2020
Merged

Add allRefs = true argument to fetchGit to explicitly perform a full checkout #3814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
629af83
Provide a more meaningful error-message for `builtins.fetchGit` if a …
Ma27 Jul 14, 2020
2857b1b
Add explicit `allRefs = true;` argument to `fetchGit`
Ma27 Jul 17, 2020
e54971d
Document `allRefs` argument of `builtins.fetchTree`
Ma27 Sep 8, 2020
897ae23
tests/fetchGit: test behavior of `allRefs = true;`
Ma27 Dec 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libexpr/primops/fetchTree.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,11 @@ static RegisterPrimOp primop_fetchGit({
A Boolean parameter that specifies whether submodules should be
checked out. Defaults to `false`.

- allRefs
Whether to fetch all refs of the repository. With this argument being
true, it's possible to load a `rev` from *any* `ref` (by default only
`rev`s from the specified `ref` are supported).

Here are some examples of how to use `fetchGit`.

- To fetch a private repository over SSH:
Expand Down
49 changes: 40 additions & 9 deletions src/libfetchers/git.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ struct GitInputScheme : InputScheme
if (maybeGetStrAttr(attrs, "type") != "git") return {};

for (auto & [name, value] : attrs)
if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash")
if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs")
throw Error("unsupported Git input attribute '%s'", name);

parseURL(getStrAttr(attrs, "url"));
maybeGetBoolAttr(attrs, "shallow");
maybeGetBoolAttr(attrs, "submodules");
maybeGetBoolAttr(attrs, "allRefs");

if (auto ref = maybeGetStrAttr(attrs, "ref")) {
if (std::regex_search(*ref, badGitRefRegex))
Expand Down Expand Up @@ -169,10 +170,12 @@ struct GitInputScheme : InputScheme

bool shallow = maybeGetBoolAttr(input.attrs, "shallow").value_or(false);
bool submodules = maybeGetBoolAttr(input.attrs, "submodules").value_or(false);
bool allRefs = maybeGetBoolAttr(input.attrs, "allRefs").value_or(false);

std::string cacheType = "git";
if (shallow) cacheType += "-shallow";
if (submodules) cacheType += "-submodules";
if (allRefs) cacheType += "-all-refs";

auto getImmutableAttrs = [&]()
{
Expand Down Expand Up @@ -338,11 +341,15 @@ struct GitInputScheme : InputScheme
}
}
} 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 ||
(uint64_t) st.st_mtime + settings.tarballTtl <= (uint64_t) now;
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 ||
(uint64_t) st.st_mtime + settings.tarballTtl <= (uint64_t) now;
}
}

if (doFetch) {
Expand All @@ -352,9 +359,11 @@ struct GitInputScheme : InputScheme
// we're using --quiet for now. Should process its stderr.
try {
auto ref = input.getRef();
auto fetchRef = ref->compare(0, 5, "refs/") == 0
? *ref
: "refs/heads/" + *ref;
auto fetchRef = allRefs
? "refs/*"
: ref->compare(0, 5, "refs/") == 0
? *ref
: "refs/heads/" + *ref;
runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) });
} catch (Error & e) {
if (!pathExists(localRefFile)) throw;
Expand Down Expand Up @@ -392,6 +401,28 @@ struct GitInputScheme : InputScheme
AutoDelete delTmpDir(tmpDir, true);
PathFilter filter = defaultPathFilter;

RunOptions checkCommitOpts(
"git",
{ "-C", repoDir, "cat-file", "commit", input.getRev()->gitRev() }
);
checkCommitOpts.searchPath = true;
checkCommitOpts.mergeStderrToStdout = true;

auto result = runProgram(checkCommitOpts);
if (WEXITSTATUS(result.first) == 128
&& result.second.find("bad file") != std::string::npos
Copy link
Member

@edolstra edolstra Dec 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for strings like "bad file" in stderr seems pretty fragile... Do we actually need this test? Presumably a wrong rev will fail in the subsequent call to git archive.

Copy link
Member Author

@Ma27 Ma27 Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid no: if git cat-file errors, the exit code is always 128, the reason for the error is on stderr.

) {
throw Error(
"Cannot find Git revision '%s' in ref '%s' of repository '%s'! "
"Please make sure that the " ANSI_BOLD "rev" ANSI_NORMAL " exists on the "
ANSI_BOLD "ref" ANSI_NORMAL " you've specified or add " ANSI_BOLD
"allRefs = true;" ANSI_NORMAL " to " ANSI_BOLD "fetchGit" ANSI_NORMAL ".",
input.getRev()->gitRev(),
*input.getRef(),
actualUrl
);
}

if (submodules) {
Path tmpGitDir = createTempDir();
AutoDelete delTmpGitDir(tmpGitDir, true);
Expand Down
13 changes: 13 additions & 0 deletions tests/fetchGit.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,19 @@ export _NIX_FORCE_HTTP=1
path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
[[ $(cat $path/hello) = world ]]

# Fetch a rev from another branch
git -C $repo checkout -b devtest
echo "different file" >> $TEST_ROOT/git/differentbranch
git -C $repo add differentbranch
git -C $repo commit -m 'Test2'
git -C $repo checkout master
devrev=$(git -C $repo rev-parse devtest)
out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; }" 2>&1) || status=$?
[[ $status == 1 ]]
[[ $out =~ 'Cannot find Git revision' ]]

[[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = file://$repo; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]]

# 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\")")
Expand Down