From ae4e0023cdab6a890e8dfdee9bf57f7b0b41c460 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Sun, 10 Oct 2021 17:27:04 +0200 Subject: [PATCH 1/2] git: Optimize submodule fetching --- src/libfetchers/git.cc | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 8468d2afc7b..52a65615f64 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -439,19 +439,27 @@ struct GitInputScheme : InputScheme } if (submodules) { - Path tmpGitDir = createTempDir(); - AutoDelete delTmpGitDir(tmpGitDir, true); - - runProgram("git", true, { "-c", "init.defaultBranch=" + gitInitialBranch, "init", tmpDir, "--separate-git-dir", tmpGitDir }); - // TODO: repoDir might lack the ref (it only checks if rev - // exists, see FIXME above) so use a big hammer and fetch - // everything to ensure we get the rev. - runProgram("git", true, { "-C", tmpDir, "fetch", "--quiet", "--force", - "--update-head-ok", "--", repoDir, "refs/*:refs/*" }); - - runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input.getRev()->gitRev() }); - runProgram("git", true, { "-C", tmpDir, "remote", "add", "origin", actualUrl }); - runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }); + if (isLocal) { + // `git submodule update` modifies the source repo, so do a temporary, cheap clone with its own HEADs + runProgram("git", true, { "clone", "--quiet", "--no-checkout", "--shared", "-c", "submodule.alternateLocation=superproject", repoDir, tmpDir }); + runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input.getRev()->gitRev() }); + // try checking out submodules without fetching first since otherwise git seems to unnecessarily do a fetch when the + // submodule commit is present but not reachable + try { + runProgram2({ + .program ="git", + .args = { "-C", tmpDir, "submodule", "update", "--init", "--recursive", "--quiet", "--no-fetch" } + }); + } catch (ExecError &) { + // TODO: cache these fetches + runProgram("git", true, { "-C", tmpDir, "submodule", "update", "--init", "--recursive", "--quiet" }); + } + } else { + // it's fine to modify `repoDir` HEADs if it is in the Nix cache; otherwise *no* submodule fetches would be cached + runProgram("git", true, { "--git-dir", repoDir, "--work-tree", tmpDir, "checkout", "--quiet", input.getRev()->gitRev(), "." }); + // `-C` should be redundant, probably a bug in `git submodule` + runProgram("git", true, { "--git-dir", repoDir, "--work-tree", tmpDir, "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" }); + } filter = isNotDotGitDirectory; } else { From 819d42d24c7584126998eb99c5633773c84f2ce0 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Sun, 17 Oct 2021 15:18:39 +0200 Subject: [PATCH 2/2] Test submodules with remote repo --- tests/fetchGitSubmodules.sh | 92 ++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/tests/fetchGitSubmodules.sh b/tests/fetchGitSubmodules.sh index 5f104355f69..ebc1516ff6b 100644 --- a/tests/fetchGitSubmodules.sh +++ b/tests/fetchGitSubmodules.sh @@ -38,60 +38,68 @@ git -C $rootRepo commit -m "Add submodule" rev=$(git -C $rootRepo rev-parse HEAD) -r1=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; }).outPath") -r2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = false; }).outPath") -r3=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") +doTests() { + r1=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; }).outPath") + r2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = false; }).outPath") + r3=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -[[ $r1 == $r2 ]] -[[ $r2 != $r3 ]] + [[ $r1 == $r2 ]] + [[ $r2 != $r3 ]] -r4=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; }).outPath") -r5=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = false; }).outPath") -r6=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") -r7=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") -r8=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = true; }).outPath") + r4=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; }).outPath") + r5=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = false; }).outPath") + r6=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") + r7=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") + r8=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -[[ $r1 == $r4 ]] -[[ $r4 == $r5 ]] -[[ $r3 == $r6 ]] -[[ $r6 == $r7 ]] -[[ $r7 == $r8 ]] + [[ $r1 == $r4 ]] + [[ $r4 == $r5 ]] + [[ $r3 == $r6 ]] + [[ $r6 == $r7 ]] + [[ $r7 == $r8 ]] -have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).submodules") -[[ $have_submodules == false ]] + have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).submodules") + [[ $have_submodules == false ]] -have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = false; }).submodules") -[[ $have_submodules == false ]] + have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = false; }).submodules") + [[ $have_submodules == false ]] -have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = true; }).submodules") -[[ $have_submodules == true ]] + have_submodules=$(nix eval --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; submodules = true; }).submodules") + [[ $have_submodules == true ]] -pathWithoutSubmodules=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; }).outPath") -pathWithSubmodules=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -pathWithSubmodulesAgain=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") -pathWithSubmodulesAgainWithRef=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") + pathWithoutSubmodules=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; }).outPath") + pathWithSubmodules=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") + pathWithSubmodulesAgain=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }).outPath") + pathWithSubmodulesAgainWithRef=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$rootRepo; ref = \"master\"; rev = \"$rev\"; submodules = true; }).outPath") -# The resulting store path cannot be the same. -[[ $pathWithoutSubmodules != $pathWithSubmodules ]] + # The resulting store path cannot be the same. + [[ $pathWithoutSubmodules != $pathWithSubmodules ]] -# Checking out the same repo with submodules returns in the same store path. -[[ $pathWithSubmodules == $pathWithSubmodulesAgain ]] + # Checking out the same repo with submodules returns in the same store path. + [[ $pathWithSubmodules == $pathWithSubmodulesAgain ]] -# Checking out the same repo with submodules returns in the same store path. -[[ $pathWithSubmodulesAgain == $pathWithSubmodulesAgainWithRef ]] + # Checking out the same repo with submodules returns in the same store path. + [[ $pathWithSubmodulesAgain == $pathWithSubmodulesAgainWithRef ]] -# The submodules flag is actually honored. -[[ ! -e $pathWithoutSubmodules/sub/content ]] -[[ -e $pathWithSubmodules/sub/content ]] + # The submodules flag is actually honored. + [[ ! -e $pathWithoutSubmodules/sub/content ]] + [[ -e $pathWithSubmodules/sub/content ]] -[[ -e $pathWithSubmodulesAgainWithRef/sub/content ]] + [[ -e $pathWithSubmodulesAgainWithRef/sub/content ]] -# No .git directory or submodule reference files must be left -test "$(find "$pathWithSubmodules" -name .git)" = "" + # No .git directory or submodule reference files must be left + test "$(find "$pathWithSubmodules" -name .git)" = "" -# Git repos without submodules can be fetched with submodules = true. -subRev=$(git -C $subRepo rev-parse HEAD) -noSubmoduleRepoBaseline=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; }).outPath") -noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; submodules = true; }).outPath") + # Git repos without submodules can be fetched with submodules = true. + subRev=$(git -C $subRepo rev-parse HEAD) + noSubmoduleRepoBaseline=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; }).outPath") + noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; submodules = true; }).outPath") -[[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]] + [[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]] +} + +doTests + +# again, with forced clones +export _NIX_FORCE_HTTP=1 +doTests