Merge pull request #10244 from bouk/submodule-fetch-default-method

git fetcher: fix resolveSubmoduleUrl to work with all repo URLs

.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

src/libfetchers/git-utils.cc

@@ -198,6 +198,12 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
         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<GitRepoImpl>
 
     std::vector<std::tuple<Submodule, Hash>> getSubmodules(const Hash & rev, bool exportIgnore) override;
 
-    std::string resolveSubmoduleUrl(
+    std::string resolveSubmoduleUrl(const std::string & url) override
-        const std::string & url,
-        const std::string & base) override
     {
         git_buf buf = GIT_BUF_INIT;
         if (git_submodule_resolve_url(&buf, *this, url.c_str()))
@@ -312,10 +316,6 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
         Finally cleanup = [&]() { git_buf_dispose(&buf); };
 
         std::string res(buf.ptr);
-
-        if (!hasPrefix(res, "/") && res.find("://") == res.npos)
-            res = parseURL(base + "/" + res).canonicalise().to_string();
-
         return res;
     }
 

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<std::tuple<Submodule, Hash>> getSubmodules(const Hash & rev, bool exportIgnore) = 0;
 
-    virtual std::string resolveSubmoduleUrl(
+    virtual std::string resolveSubmoduleUrl(const std::string & url) = 0;
-        const std::string & url,
-        const std::string & base) = 0;
 
     virtual bool hasObject(const Hash & oid) = 0;
 

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<CanonPath, nix::ref<InputAccessor>> 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;

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;

tests/nixos/git-submodules.nix

@@ -0,0 +1,70 @@
+# 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'")
+    '';
+  };
+}