Merge remote-tracking branch 'origin/master' into flakes

11 files changed, 178 insertions, 33 deletions

diff --git a/doc/manual/expressions/builtins.xml b/doc/manual/expressions/builtins.xml
index 394e1fc32..f71a8f3be 100644
--- a/doc/manual/expressions/builtins.xml
+++ b/doc/manual/expressions/builtins.xml
@@ -422,6 +422,16 @@ stdenv.mkDerivation { … }
             </para>
           </listitem>
         </varlistentry>
+        <varlistentry>
+          <term>submodules</term>
+          <listitem>
+            <para>
+              A Boolean parameter that specifies whether submodules
+              should be checked out. Defaults to
+              <literal>false</literal>.
+            </para>
+          </listitem>
+        </varlistentry>
       </variablelist>
 
       <example>
diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index 4545bfd72..76d101b98 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -37,9 +37,6 @@ std::pair<Value *, Pos> findAlongAttrPath(EvalState & state, const string & attr
 {
     Strings tokens = parseAttrPath(attrPath);
 
-    Error attrError =
-        Error(format("attribute selection path '%1%' does not match expression") % attrPath);
-
     Value * v = &vIn;
     Pos pos = noPos;
 
diff --git a/src/libexpr/common-eval-args.cc b/src/libexpr/common-eval-args.cc
index bf5025147..f965c194f 100644
--- a/src/libexpr/common-eval-args.cc
+++ b/src/libexpr/common-eval-args.cc
@@ -3,8 +3,8 @@
 #include "download.hh"
 #include "util.hh"
 #include "eval.hh"
-#include "registry.hh"
 #include "fetchers.hh"
+#include "registry.hh"
 #include "flake/flakeref.hh"
 #include "store-api.hh"
 
diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc
index 812de9d91..1a8798fcc 100644
--- a/src/libexpr/primops/fetchGit.cc
+++ b/src/libexpr/primops/fetchGit.cc
@@ -13,6 +13,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va
     std::optional<std::string> ref;
     std::optional<Hash> rev;
     std::string name = "source";
+    bool fetchSubmodules = false;
     PathSet context;
 
     state.forceValue(*args[0]);
@@ -31,6 +32,8 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va
                 rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1);
             else if (n == "name")
                 name = state.forceStringNoCtx(*attr.value, *attr.pos);
+            else if (n == "submodules")
+                fetchSubmodules = state.forceBool(*attr.value, *attr.pos);
             else
                 throw EvalError("unsupported argument '%s' to 'fetchGit', at %s", attr.name, *attr.pos);
         }
@@ -48,15 +51,15 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va
     if (evalSettings.pureEval && !rev)
         throw Error("in pure evaluation mode, 'fetchGit' requires a Git revision");
 
-    auto parsedUrl = parseURL(
-        url.find("://") != std::string::npos
-        ? "git+" + url
-        : "git+file://" + url);
-    if (ref) parsedUrl.query.insert_or_assign("ref", *ref);
-    if (rev) parsedUrl.query.insert_or_assign("rev", rev->gitRev());
-    // FIXME: use name
-    auto input = fetchers::inputFromURL(parsedUrl);
+    fetchers::Attrs attrs;
+    attrs.insert_or_assign("type", "git");
+    attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url);
+    if (ref) attrs.insert_or_assign("ref", *ref);
+    if (rev) attrs.insert_or_assign("rev", rev->gitRev());
+    if (fetchSubmodules) attrs.insert_or_assign("submodules", true);
+    auto input = fetchers::inputFromAttrs(attrs);
 
+    // FIXME: use name?
     auto [tree, input2] = input->fetchTree(state.store);
 
     state.mkAttrs(v, 8);
@@ -70,6 +73,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va
     // Backward compatibility: set 'revCount' to 0 for a dirty tree.
     mkInt(*state.allocAttr(v, state.symbols.create("revCount")),
         tree.info.revCount.value_or(0));
+    mkBool(*state.allocAttr(v, state.symbols.create("submodules")), fetchSubmodules);
     v.attrs->sort();
 
     if (state.allowedPaths)
diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc
index 0838416df..4f66fc6c1 100644
--- a/src/libexpr/primops/fetchTree.cc
+++ b/src/libexpr/primops/fetchTree.cc
@@ -2,8 +2,8 @@
 #include "eval-inline.hh"
 #include "store-api.hh"
 #include "fetchers.hh"
-#include "registry.hh"
 #include "download.hh"
+#include "registry.hh"
 
 #include <ctime>
 #include <iomanip>
diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index dbf57aa33..210e29193 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -15,12 +15,20 @@ static std::string readHead(const Path & path)
     return chomp(runProgram("git", true, { "-C", path, "rev-parse", "--abbrev-ref", "HEAD" }));
 }
 
+static bool isNotDotGitDirectory(const Path & path)
+{
+    static const std::regex gitDirRegex("^(?:.*/)?\\.git$");
+
+    return not std::regex_match(path, gitDirRegex);
+}
+
 struct GitInput : Input
 {
     ParsedURL url;
     std::optional<std::string> ref;
     std::optional<Hash> rev;
     bool shallow = false;
+    bool submodules = false;
 
     GitInput(const ParsedURL & url) : url(url)
     { }
@@ -66,6 +74,8 @@ struct GitInput : Input
             attrs.emplace("rev", rev->gitRev());
         if (shallow)
             attrs.emplace("shallow", true);
+        if (submodules)
+            attrs.emplace("submodules", true);
         return attrs;
     }
 
@@ -144,7 +154,9 @@ struct GitInput : Input
 
         assert(!rev || rev->type == htSHA1);
 
-        auto cacheType = shallow ? "git-shallow" : "git";
+        std::string cacheType = "git";
+        if (shallow) cacheType += "-shallow";
+        if (submodules) cacheType += "-submodules";
 
         auto getImmutableAttrs = [&]()
         {
@@ -218,8 +230,12 @@ struct GitInput : Input
                 if (settings.warnDirty)
                     warn("Git tree '%s' is dirty", actualUrl);
 
+                auto gitOpts = Strings({ "-C", actualUrl, "ls-files", "-z" });
+                if (submodules)
+                    gitOpts.emplace_back("--recurse-submodules");
+
                 auto files = tokenizeString<std::set<std::string>>(
-                    runProgram("git", true, { "-C", actualUrl, "ls-files", "-z" }), "\0"s);
+                    runProgram("git", true, gitOpts), "\0"s);
 
                 PathFilter filter = [&](const Path & p) -> bool {
                     assert(hasPrefix(p, actualUrl));
@@ -356,20 +372,39 @@ struct GitInput : Input
         if (auto res = getCache()->lookup(store, getImmutableAttrs()))
             return makeResult(res->first, std::move(res->second));
 
-        // FIXME: should pipe this, or find some better way to extract a
-        // revision.
-        auto source = sinkToSource([&](Sink & sink) {
-            RunOptions gitOptions("git", { "-C", repoDir, "archive", input->rev->gitRev() });
-            gitOptions.standardOut = &sink;
-            runProgram2(gitOptions);
-        });
-
         Path tmpDir = createTempDir();
         AutoDelete delTmpDir(tmpDir, true);
+        PathFilter filter = defaultPathFilter;
+
+        if (submodules) {
+            Path tmpGitDir = createTempDir();
+            AutoDelete delTmpGitDir(tmpGitDir, true);
 
-        unpackTarfile(*source, tmpDir);
+            runProgram("git", true, { "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/*" });
 
-        auto storePath = store->addToStore(name, tmpDir);
+            runProgram("git", true, { "-C", tmpDir, "checkout", "--quiet", input->rev->gitRev() });
+            runProgram("git", true, { "-C", tmpDir, "remote", "add", "origin", actualUrl });
+            runProgram("git", true, { "-C", tmpDir, "submodule", "--quiet", "update", "--init", "--recursive" });
+
+            filter = isNotDotGitDirectory;
+        } else {
+            // FIXME: should pipe this, or find some better way to extract a
+            // revision.
+            auto source = sinkToSource([&](Sink & sink) {
+                RunOptions gitOptions("git", { "-C", repoDir, "archive", input->rev->gitRev() });
+                gitOptions.standardOut = &sink;
+                runProgram2(gitOptions);
+            });
+
+            unpackTarfile(*source, tmpDir);
+        }
+
+        auto storePath = store->addToStore(name, tmpDir, true, htSHA256, filter);
 
         auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", input->rev->gitRev() }));
 
@@ -435,7 +470,7 @@ 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")
+            if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules")
                 throw Error("unsupported Git input attribute '%s'", name);
 
         auto input = std::make_unique<GitInput>(parseURL(getStrAttr(attrs, "url")));
@@ -449,6 +484,8 @@ struct GitInputScheme : InputScheme
 
         input->shallow = maybeGetBoolAttr(attrs, "shallow").value_or(false);
 
+        input->submodules = maybeGetBoolAttr(attrs, "submodules").value_or(false);
+
         return input;
     }
 };
diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc
index 037404726..7c7e20f4e 100644
--- a/src/libfetchers/path.cc
+++ b/src/libfetchers/path.cc
@@ -63,6 +63,8 @@ struct PathInput : Input
     {
         auto input = std::make_shared<PathInput>(*this);
 
+        // FIXME: check whether access to 'path' is allowed.
+
         auto storePath = store->maybeParseStorePath(path);
 
         if (storePath)
diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index 35a35cd78..af69699a8 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -1,14 +1,10 @@
 #include "download.hh"
 #include "util.hh"
 #include "globals.hh"
-#include "hash.hh"
 #include "store-api.hh"
-#include "archive.hh"
 #include "s3.hh"
 #include "compression.hh"
-#include "pathlocks.hh"
 #include "finally.hh"
-#include "tarfile.hh"
 
 #ifdef ENABLE_S3
 #include <aws/core/client/ClientConfiguration.h>
@@ -386,6 +382,7 @@ struct CurlDownloader : public Downloader
                         case CURLE_SSL_CACERT_BADFILE:
                         case CURLE_TOO_MANY_REDIRECTS:
                         case CURLE_WRITE_ERROR:
+                        case CURLE_UNSUPPORTED_PROTOCOL:
                             err = Misc;
                             break;
                         default: // Shut up warnings
diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh
index 41bbed33f..6c3c58269 100644
--- a/src/libstore/globals.hh
+++ b/src/libstore/globals.hh
@@ -361,14 +361,14 @@ public:
 
     void requireExperimentalFeature(const std::string & name);
 
-    Setting<std::string> flakeRegistry{this, "https://github.com/NixOS/flake-registry/raw/master/flake-registry.json", "flake-registry",
-        "Path or URI of the global flake registry."};
-
     Setting<bool> allowDirty{this, true, "allow-dirty",
         "Whether to allow dirty Git/Mercurial trees."};
 
     Setting<bool> warnDirty{this, true, "warn-dirty",
         "Whether to warn about dirty Git/Mercurial trees."};
+
+    Setting<std::string> flakeRegistry{this, "https://github.com/NixOS/flake-registry/raw/master/flake-registry.json", "flake-registry",
+        "Path or URI of the global flake registry."};
 };
 
 
diff --git a/tests/fetchGitSubmodules.sh b/tests/fetchGitSubmodules.sh
new file mode 100644
index 000000000..5f104355f
--- /dev/null
+++ b/tests/fetchGitSubmodules.sh
@@ -0,0 +1,97 @@
+source common.sh
+
+set -u
+
+if [[ -z $(type -p git) ]]; then
+    echo "Git not installed; skipping Git submodule tests"
+    exit 99
+fi
+
+clearStore
+
+rootRepo=$TEST_ROOT/gitSubmodulesRoot
+subRepo=$TEST_ROOT/gitSubmodulesSub
+
+rm -rf ${rootRepo} ${subRepo} $TEST_HOME/.cache/nix
+
+initGitRepo() {
+    git init $1
+    git -C $1 config user.email "foobar@example.com"
+    git -C $1 config user.name "Foobar"
+}
+
+addGitContent() {
+    echo "lorem ipsum" > $1/content
+    git -C $1 add content
+    git -C $1 commit -m "Initial commit"
+}
+
+initGitRepo $subRepo
+addGitContent $subRepo
+
+initGitRepo $rootRepo
+
+git -C $rootRepo submodule init
+git -C $rootRepo submodule add $subRepo sub
+git -C $rootRepo add sub
+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")
+
+[[ $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")
+
+[[ $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 = false; }).submodules")
+[[ $have_submodules == false ]]
+
+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")
+
+# 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.
+[[ $pathWithSubmodulesAgain == $pathWithSubmodulesAgainWithRef ]]
+
+# The submodules flag is actually honored.
+[[ ! -e $pathWithoutSubmodules/sub/content ]]
+[[ -e $pathWithSubmodules/sub/content ]]
+
+[[ -e $pathWithSubmodulesAgainWithRef/sub/content ]]
+
+# 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")
+
+[[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]]
diff --git a/tests/local.mk b/tests/local.mk
index 62bfcaac1..884f8619e 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -17,6 +17,7 @@ nix_tests = \
   nar-access.sh \
   structured-attrs.sh \
   fetchGit.sh \
+  fetchGitSubmodules.sh \
   fetchMercurial.sh \
   signing.sh \
   run.sh \