From 1f73de262961510318328463f943175833994a50 Mon Sep 17 00:00:00 2001 From: Bouke van der Bijl Date: Thu, 14 Mar 2024 16:51:44 +0100 Subject: [PATCH 1/4] git fetcher: relax absolute URL check of resolveSubmoduleUrl This matches up the behavior with the internals of libgit2 Fixes #9979 --- src/libfetchers/git-utils.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 5e560f5f3..5777240ad 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -313,7 +313,11 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this std::string res(buf.ptr); - if (!hasPrefix(res, "/") && res.find("://") == res.npos) + // Git has a default protocol of 'ssh' for URLs without a protocol: + // https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_ssh_protocol + // This code matches what git_submodule_resolve_url does inside of itself to see if the URL is already absolute. + // Inside libgit2 it just checks whether there's any ':' in the URL to default to the ssh:// protocol. + if (!hasPrefix(res, "/") && res.find(":") == res.npos) res = parseURL(base + "/" + res).canonicalise().to_string(); return res; From 1a76ca416108c8aefbafbf20f58f66725166b9f9 Mon Sep 17 00:00:00 2001 From: Bouke van der Bijl Date: Fri, 15 Mar 2024 09:30:58 +0100 Subject: [PATCH 2/4] Set the origin instead of hacking in the URL resolving --- src/libfetchers/git-utils.cc | 18 +++++++----------- src/libfetchers/git-utils.hh | 6 +++--- src/libfetchers/unix/git.cc | 5 ++++- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 5777240ad..5ecd825b7 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -198,6 +198,12 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this return git_repository_is_shallow(*this); } + void setRemote(const std::string & name, const std::string & url) override + { + if (git_remote_set_url(*this, name.c_str(), url.c_str())) + throw Error("setting remote '%s' URL to '%s': %s", name, url, git_error_last()->message); + } + Hash resolveRef(std::string ref) override { Object object; @@ -302,9 +308,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this std::vector> getSubmodules(const Hash & rev, bool exportIgnore) override; - std::string resolveSubmoduleUrl( - const std::string & url, - const std::string & base) override + std::string resolveSubmoduleUrl(const std::string & url) override { git_buf buf = GIT_BUF_INIT; if (git_submodule_resolve_url(&buf, *this, url.c_str())) @@ -312,14 +316,6 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this Finally cleanup = [&]() { git_buf_dispose(&buf); }; std::string res(buf.ptr); - - // Git has a default protocol of 'ssh' for URLs without a protocol: - // https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols#_the_ssh_protocol - // This code matches what git_submodule_resolve_url does inside of itself to see if the URL is already absolute. - // Inside libgit2 it just checks whether there's any ':' in the URL to default to the ssh:// protocol. - if (!hasPrefix(res, "/") && res.find(":") == res.npos) - res = parseURL(base + "/" + res).canonicalise().to_string(); - return res; } diff --git a/src/libfetchers/git-utils.hh b/src/libfetchers/git-utils.hh index fbb2d947b..600a42da0 100644 --- a/src/libfetchers/git-utils.hh +++ b/src/libfetchers/git-utils.hh @@ -32,6 +32,8 @@ struct GitRepo /* Return the commit hash to which a ref points. */ virtual Hash resolveRef(std::string ref) = 0; + virtual void setRemote(const std::string & name, const std::string & url) = 0; + /** * Info about a submodule. */ @@ -69,9 +71,7 @@ struct GitRepo */ virtual std::vector> getSubmodules(const Hash & rev, bool exportIgnore) = 0; - virtual std::string resolveSubmoduleUrl( - const std::string & url, - const std::string & base) = 0; + virtual std::string resolveSubmoduleUrl(const std::string & url) = 0; virtual bool hasObject(const Hash & oid) = 0; diff --git a/src/libfetchers/unix/git.cc b/src/libfetchers/unix/git.cc index 0966c4710..45e62ebe1 100644 --- a/src/libfetchers/unix/git.cc +++ b/src/libfetchers/unix/git.cc @@ -526,6 +526,9 @@ struct GitInputScheme : InputScheme auto repo = GitRepo::openRepo(cacheDir, true, true); + // We need to set the origin so resolving submodule URLs works + repo->setRemote("origin", repoInfo.url); + Path localRefFile = ref.compare(0, 5, "refs/") == 0 ? cacheDir + "/" + ref @@ -629,7 +632,7 @@ struct GitInputScheme : InputScheme std::map> mounts; for (auto & [submodule, submoduleRev] : repo->getSubmodules(rev, exportIgnore)) { - auto resolved = repo->resolveSubmoduleUrl(submodule.url, repoInfo.url); + auto resolved = repo->resolveSubmoduleUrl(submodule.url); debug("Git submodule %s: %s %s %s -> %s", submodule.path, submodule.url, submodule.branch, submoduleRev.gitRev(), resolved); fetchers::Attrs attrs; From cd06193d1387921caed8aa78b3f3617bea11fd00 Mon Sep 17 00:00:00 2001 From: Bouke van der Bijl Date: Thu, 11 Apr 2024 14:58:42 +0200 Subject: [PATCH 3/4] Add nixos test --- tests/nixos/default.nix | 2 ++ tests/nixos/git-submodules.nix | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 tests/nixos/git-submodules.nix diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix index 627728424..4edf40c16 100644 --- a/tests/nixos/default.nix +++ b/tests/nixos/default.nix @@ -145,6 +145,8 @@ in githubFlakes = runNixOSTestFor "x86_64-linux" ./github-flakes.nix; + gitSubmodules = runNixOSTestFor "x86_64-linux" ./git-submodules.nix; + sourcehutFlakes = runNixOSTestFor "x86_64-linux" ./sourcehut-flakes.nix; tarballFlakes = runNixOSTestFor "x86_64-linux" ./tarball-flakes.nix; diff --git a/tests/nixos/git-submodules.nix b/tests/nixos/git-submodules.nix new file mode 100644 index 000000000..9264369ef --- /dev/null +++ b/tests/nixos/git-submodules.nix @@ -0,0 +1,54 @@ +# Test Nix's remote build feature. + +{ lib, hostPkgs, ... }: + +{ + config = { + name = lib.mkDefault "git-submodules"; + + nodes = + { + remote = + { config, pkgs, ... }: + { + services.openssh.enable = true; + environment.systemPackages = [ pkgs.git ]; + }; + + client = + { config, lib, pkgs, ... }: + { + programs.ssh.extraConfig = "ConnectTimeout 30"; + environment.systemPackages = [ pkgs.git ]; + nix.extraOptions = "experimental-features = nix-command flakes"; + }; + }; + + testScript = { nodes }: '' + # fmt: off + import subprocess + + start_all() + + # Create an SSH key on the client. + subprocess.run([ + "${hostPkgs.openssh}/bin/ssh-keygen", "-t", "ed25519", "-f", "key", "-N", "" + ], capture_output=True, check=True) + client.succeed("mkdir -p -m 700 /root/.ssh") + client.copy_from_host("key", "/root/.ssh/id_ed25519") + client.succeed("chmod 600 /root/.ssh/id_ed25519") + + # Install the SSH key on the builders. + client.wait_for_unit("network.target") + + remote.succeed("mkdir -p -m 700 /root/.ssh") + remote.copy_from_host("key.pub", "/root/.ssh/authorized_keys") + remote.wait_for_unit("sshd") + client.succeed(f"ssh -o StrictHostKeyChecking=no {remote.name} 'echo hello world'") + + remote.succeed("git init bar && git -C bar config user.email foobar@example.com && git -C bar config user.name Foobar && echo test >> bar/content && git -C bar add content && git -C bar commit -m 'Initial commit'") + client.succeed(f"git init foo && git -C foo config user.email foobar@example.com && git -C foo config user.name Foobar && git -C foo submodule add root@{remote.name}:/tmp/bar sub && git -C foo add sub && git -C foo commit -m 'Add submodule'") + client.succeed("nix --flake-registry \"\" flake prefetch 'git+file:///tmp/foo?submodules=1&ref=master'") + ''; + }; +} From 1e4f902b28231497c6f2fe6db3f2d85352efc752 Mon Sep 17 00:00:00 2001 From: Bouke van der Bijl Date: Thu, 11 Apr 2024 15:31:21 +0200 Subject: [PATCH 4/4] Add gitSubmodules test to github actions --- .github/workflows/ci.yml | 2 +- tests/nixos/git-submodules.nix | 20 ++++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b8eac49d..cfd1dec5d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -166,4 +166,4 @@ jobs: - uses: actions/checkout@v4 - uses: DeterminateSystems/nix-installer-action@main - uses: DeterminateSystems/magic-nix-cache-action@main - - run: nix build -L .#hydraJobs.tests.githubFlakes .#hydraJobs.tests.tarballFlakes + - run: nix build -L .#hydraJobs.tests.githubFlakes .#hydraJobs.tests.tarballFlakes .#hydraJobs.tests.gitSubmodules diff --git a/tests/nixos/git-submodules.nix b/tests/nixos/git-submodules.nix index 9264369ef..570b1822b 100644 --- a/tests/nixos/git-submodules.nix +++ b/tests/nixos/git-submodules.nix @@ -46,8 +46,24 @@ remote.wait_for_unit("sshd") client.succeed(f"ssh -o StrictHostKeyChecking=no {remote.name} 'echo hello world'") - remote.succeed("git init bar && git -C bar config user.email foobar@example.com && git -C bar config user.name Foobar && echo test >> bar/content && git -C bar add content && git -C bar commit -m 'Initial commit'") - client.succeed(f"git init foo && git -C foo config user.email foobar@example.com && git -C foo config user.name Foobar && git -C foo submodule add root@{remote.name}:/tmp/bar sub && git -C foo add sub && git -C foo commit -m 'Add submodule'") + remote.succeed(""" + git init bar + git -C bar config user.email foobar@example.com + git -C bar config user.name Foobar + echo test >> bar/content + git -C bar add content + git -C bar commit -m 'Initial commit' + """) + + client.succeed(f""" + git init foo + git -C foo config user.email foobar@example.com + git -C foo config user.name Foobar + git -C foo submodule add root@{remote.name}:/tmp/bar sub + git -C foo add sub + git -C foo commit -m 'Add submodule' + """) + client.succeed("nix --flake-registry \"\" flake prefetch 'git+file:///tmp/foo?submodules=1&ref=master'") ''; };