From 03e71c5f86c20eb0a162731a2fbec4963a133335 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Thu, 4 Nov 2021 13:23:51 -0400
Subject: [PATCH 01/17] implement passing submodules information when tree is
 not dirty

---
 src/libexpr/primops/fetchTree.cc | 13 +++++++++--
 src/libfetchers/fetchers.hh      |  1 +
 src/libfetchers/git.cc           | 39 ++++++++++++++++++++++++++------
 tests/fetchGitSubmodules.sh      |  4 ++++
 4 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc
index 079513873ec..b216601294e 100644
--- a/src/libexpr/primops/fetchTree.cc
+++ b/src/libexpr/primops/fetchTree.cc
@@ -21,7 +21,7 @@ void emitTreeAttrs(
 {
     assert(input.isImmutable());
 
-    state.mkAttrs(v, 8);
+    state.mkAttrs(v, 9);
 
     auto storePath = state.store->printStorePath(tree.storePath);
 
@@ -34,9 +34,18 @@ void emitTreeAttrs(
     mkString(*state.allocAttr(v, state.symbols.create("narHash")),
         narHash->to_string(SRI, true));
 
-    if (input.getType() == "git")
+    if (input.getType() == "git") {
+        Value *modules = state.allocAttr(v, state.symbols.create("modules"));
+        state.mkAttrs(*modules, input.modules.size());
+        for (auto & [path, url] : input.modules) {
+            Value *vUrl = state.allocValue();
+            mkString(*vUrl, std::get<string>(url).c_str());
+            modules->attrs->push_back(Attr(state.symbols.create(path), vUrl));
+        }
+        modules->attrs->sort();
         mkBool(*state.allocAttr(v, state.symbols.create("submodules")),
             fetchers::maybeGetBoolAttr(input.attrs, "submodules").value_or(false));
+    }
 
     if (!forceDirty) {
 
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index c43b047a772..8becf1e3bc5 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -35,6 +35,7 @@ struct Input
 
     std::shared_ptr<InputScheme> scheme; // note: can be null
     Attrs attrs;
+    Attrs modules;
     bool immutable = false;
     bool direct = true;
 
diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 544d2ffbf62..f2e531f3afc 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -24,6 +24,20 @@ static std::string readHead(const Path & path)
     return chomp(runProgram("git", true, { "-C", path, "rev-parse", "--abbrev-ref", "HEAD" }));
 }
 
+static Attrs readSubmodules(const Path & path)
+{
+    auto output = runProgram("git", true, { "-C", path, "submodule", "status" });
+    Attrs attrs;
+    for (auto line : tokenizeString<std::vector<string>>(output, "\n;")) {
+        auto tokens = tokenizeString<std::vector<string>>(line);
+        auto url = tokens[0];
+        auto path = tokens[1];
+        attrs.emplace(path, url);
+    }
+
+    return attrs;
+}
+
 static bool isNotDotGitDirectory(const Path & path)
 {
     static const std::regex gitDirRegex("^(?:.*/)?\\.git$");
@@ -196,7 +210,7 @@ struct GitInputScheme : InputScheme
             });
         };
 
-        auto makeResult = [&](const Attrs & infoAttrs, StorePath && storePath)
+        auto makeResult = [&](const Attrs & infoAttrs, StorePath && storePath, Attrs modulesInfo)
             -> std::pair<Tree, Input>
         {
             assert(input.getRev());
@@ -204,6 +218,7 @@ struct GitInputScheme : InputScheme
             if (!shallow)
                 input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
             input.attrs.insert_or_assign("lastModified", getIntAttr(infoAttrs, "lastModified"));
+            input.modules = modulesInfo;
             return {
                 Tree(store->toRealPath(storePath), std::move(storePath)),
                 input
@@ -211,8 +226,11 @@ struct GitInputScheme : InputScheme
         };
 
         if (input.getRev()) {
-            if (auto res = getCache()->lookup(store, getImmutableAttrs()))
-                return makeResult(res->first, std::move(res->second));
+            if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
+                Path p = store->printStorePath(res->second);
+                auto modulesInfo = readSubmodules(p);
+                return makeResult(res->first, std::move(res->second), modulesInfo);
+            }
         }
 
         auto [isLocal, actualUrl_] = getActualUrl(input);
@@ -317,7 +335,9 @@ struct GitInputScheme : InputScheme
                 auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1);
                 if (!input.getRev() || input.getRev() == rev2) {
                     input.attrs.insert_or_assign("rev", rev2.gitRev());
-                    return makeResult(res->first, std::move(res->second));
+                    Path p = store->printStorePath(res->second);
+                    auto modulesInfo = readSubmodules(p);
+                    return makeResult(res->first, std::move(res->second), modulesInfo);
                 }
             }
 
@@ -410,8 +430,11 @@ struct GitInputScheme : InputScheme
 
         /* Now that we know the ref, check again whether we have it in
            the store. */
-        if (auto res = getCache()->lookup(store, getImmutableAttrs()))
-            return makeResult(res->first, std::move(res->second));
+        if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
+            Path p = store->printStorePath(res->second);
+            auto modulesInfo = readSubmodules(p);
+            return makeResult(res->first, std::move(res->second), modulesInfo);
+        }
 
         Path tmpDir = createTempDir();
         AutoDelete delTmpDir(tmpDir, true);
@@ -475,6 +498,8 @@ struct GitInputScheme : InputScheme
             {"lastModified", lastModified},
         });
 
+        auto modulesInfo = readSubmodules(actualUrl);
+
         if (!shallow)
             infoAttrs.insert_or_assign("revCount",
                 std::stoull(runProgram("git", true, { "-C", repoDir, "rev-list", "--count", input.getRev()->gitRev() })));
@@ -494,7 +519,7 @@ struct GitInputScheme : InputScheme
             storePath,
             true);
 
-        return makeResult(infoAttrs, std::move(storePath));
+        return makeResult(infoAttrs, std::move(storePath), modulesInfo);
     }
 };
 
diff --git a/tests/fetchGitSubmodules.sh b/tests/fetchGitSubmodules.sh
index 5f104355f69..087095956cf 100644
--- a/tests/fetchGitSubmodules.sh
+++ b/tests/fetchGitSubmodules.sh
@@ -38,6 +38,10 @@ git -C $rootRepo commit -m "Add submodule"
 
 rev=$(git -C $rootRepo rev-parse HEAD)
 
+nix eval --json --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).modules"
+
+false
+
 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")

From 4d6b7387de6dc55c281659c00393630da2c58534 Mon Sep 17 00:00:00 2001
From: Michael Bishop <cleverca22@gmail.com>
Date: Sat, 16 Oct 2021 01:47:31 -0300
Subject: [PATCH 02/17] parse the cat-file output to find the submodules

---
 src/libfetchers/git.cc | 115 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 103 insertions(+), 12 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index f2e531f3afc..0d2065fe64e 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -24,15 +24,99 @@ static std::string readHead(const Path & path)
     return chomp(runProgram("git", true, { "-C", path, "rev-parse", "--abbrev-ref", "HEAD" }));
 }
 
-static Attrs readSubmodules(const Path & path)
+static string getRootTree(const Path & path, const string & gitrev) {
+    auto commitOutput = runProgram("git", true, { "-C", path, "cat-file", "-p", gitrev });
+    auto lines = tokenizeString<std::vector<string>>(commitOutput, "\n");
+    auto words = tokenizeString<std::vector<string>>(lines[0]);
+    auto rootTree = words[1];
+    return rootTree;
+}
+
+enum gitType {
+  blob,
+  tree,
+  commit
+};
+
+string getBlob(const Path & path, const string & gitrev, const string & blobhash) {
+  return runProgram("git", true, { "-C", path, "cat-file", "-p", blobhash });
+}
+
+static std::map<string, std::pair<gitType, string>> gitTreeList(const Path & path, const string & treehash) {
+  auto treeOutput = runProgram("git", true, { "-C", path, "cat-file", "-p", treehash });
+  auto lines = tokenizeString<std::vector<string>>(treeOutput, "\n");
+  std::map<string, std::pair<gitType, string>> results;
+  for (auto line : lines) {
+    auto words = tokenizeString<std::vector<string>>(line);
+    auto hash = words[2];
+    auto name = words[3];
+    gitType type;
+    if (words[1] == "blob") type = blob;
+    else if (words[1] == "tree") type = tree;
+    else if (words[1] == "commit") type = commit;
+    else assert(0); // FIXME
+    results.insert(std::pair{name, std::pair{type, hash}});
+  }
+
+  return results;
+}
+
+std::map<string,string> parseSubmodules(const string & contents) {
+  auto lines = tokenizeString<std::vector<string>>(contents, "\n");
+  std::map<string,string> results;
+  string path;
+  for (auto line : lines) {
+    auto words = tokenizeString<std::vector<string>>(line);
+    if (words[1] != "=") continue;
+
+    if (words[0] == "path") path = words[2];
+    if (words[0] == "url") {
+      string url = words[2];
+      results.insert(std::pair{path, url});
+    }
+  }
+  return results;
+}
+
+string findCommitHash(const Path & path, const std::map<string, std::pair<gitType, string>> & _entries, const string & pathToFind) {
+  auto entries = _entries;
+
+  auto tokens = tokenizeString<std::vector<string>>(pathToFind, "/");
+  std::pair<gitType, string> x;
+  for (auto token : tokens) {
+    auto i = entries.find(token);
+    if (i == entries.end()) return "fail";
+
+    x = i->second;
+    if (x.first == tree) entries = gitTreeList(path, x.second);
+  }
+  if (x.first != commit) return "fail";
+
+  return x.second;
+}
+
+static Attrs readSubmodules(const Path & path, const string & gitrev)
 {
-    auto output = runProgram("git", true, { "-C", path, "submodule", "status" });
+    auto rootTree = getRootTree(path, gitrev);
+    printTalkative("root tree is %s", rootTree);
+    auto entries = gitTreeList(path, rootTree);
     Attrs attrs;
-    for (auto line : tokenizeString<std::vector<string>>(output, "\n;")) {
-        auto tokens = tokenizeString<std::vector<string>>(line);
-        auto url = tokens[0];
-        auto path = tokens[1];
-        attrs.emplace(path, url);
+
+    auto i = entries.find(".gitmodules");
+
+    if (i == entries.end()) return attrs;
+
+    auto submodules = getBlob(path, gitrev, i->second.second);
+
+    printTalkative("submodule file %s", submodules);
+
+    auto parsedModules = parseSubmodules(submodules);
+    for (auto & [path, url] : parsedModules) {
+      printTalkative("submodule %s fetched from %s", path, url);
+      auto commitHash = findCommitHash(path, entries, path);
+      printTalkative("found %s", commitHash);
+
+      attrs.emplace(path, url + "?rev=" + commitHash);
     }
 
     return attrs;
@@ -201,6 +285,8 @@ struct GitInputScheme : InputScheme
         if (submodules) cacheType += "-submodules";
         if (allRefs) cacheType += "-all-refs";
 
+        printTalkative("fetch is getting ran");
+
         auto getImmutableAttrs = [&]()
         {
             return Attrs({
@@ -228,7 +314,8 @@ struct GitInputScheme : InputScheme
         if (input.getRev()) {
             if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
                 Path p = store->printStorePath(res->second);
-                auto modulesInfo = readSubmodules(p);
+                printTalkative("case 1");
+                auto modulesInfo = readSubmodules(p, "FIXME");
                 return makeResult(res->first, std::move(res->second), modulesInfo);
             }
         }
@@ -303,6 +390,8 @@ struct GitInputScheme : InputScheme
                     "lastModified",
                     haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0);
 
+                auto modulesInfo = readSubmodules(actualUrl, "HEAD");
+                input.modules = modulesInfo;
                 return {
                     Tree(store->toRealPath(storePath), std::move(storePath)),
                     input
@@ -336,7 +425,8 @@ struct GitInputScheme : InputScheme
                 if (!input.getRev() || input.getRev() == rev2) {
                     input.attrs.insert_or_assign("rev", rev2.gitRev());
                     Path p = store->printStorePath(res->second);
-                    auto modulesInfo = readSubmodules(p);
+                    printTalkative("case 2");
+                    auto modulesInfo = readSubmodules(p, "FIXME");
                     return makeResult(res->first, std::move(res->second), modulesInfo);
                 }
             }
@@ -431,8 +521,8 @@ struct GitInputScheme : InputScheme
         /* Now that we know the ref, check again whether we have it in
            the store. */
         if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
-            Path p = store->printStorePath(res->second);
-            auto modulesInfo = readSubmodules(p);
+            printTalkative("case 3");
+            auto modulesInfo = readSubmodules(repoDir, input.getRev()->gitRev());
             return makeResult(res->first, std::move(res->second), modulesInfo);
         }
 
@@ -498,7 +588,8 @@ struct GitInputScheme : InputScheme
             {"lastModified", lastModified},
         });
 
-        auto modulesInfo = readSubmodules(actualUrl);
+        printTalkative("case 4");
+        auto modulesInfo = readSubmodules(actualUrl, "FIXME");
 
         if (!shallow)
             infoAttrs.insert_or_assign("revCount",

From 73bbc24095726d4241358b6d0b7532b9b8f2153f Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 19 Oct 2021 20:32:37 +0000
Subject: [PATCH 03/17] serialize submodule info into the cache

---
 src/libexpr/primops/fetchTree.cc | 11 ++++++---
 src/libfetchers/fetchers.hh      |  1 -
 src/libfetchers/git.cc           | 41 +++++++++++++-------------------
 tests/fetchGit.sh                |  2 ++
 tests/fetchGitSubmodules.sh      |  8 +++----
 5 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc
index b216601294e..6d7676a37d7 100644
--- a/src/libexpr/primops/fetchTree.cc
+++ b/src/libexpr/primops/fetchTree.cc
@@ -9,6 +9,8 @@
 #include <iomanip>
 #include <regex>
 
+#include <nlohmann/json.hpp>
+
 namespace nix {
 
 void emitTreeAttrs(
@@ -20,7 +22,6 @@ void emitTreeAttrs(
     bool forceDirty)
 {
     assert(input.isImmutable());
-
     state.mkAttrs(v, 9);
 
     auto storePath = state.store->printStorePath(tree.storePath);
@@ -36,8 +37,12 @@ void emitTreeAttrs(
 
     if (input.getType() == "git") {
         Value *modules = state.allocAttr(v, state.symbols.create("modules"));
-        state.mkAttrs(*modules, input.modules.size());
-        for (auto & [path, url] : input.modules) {
+
+        auto modulesJson = fetchers::getStrAttr(input.attrs, "modules");
+        auto modulesInfo = fetchers::jsonToAttrs(nlohmann::json::parse(modulesJson));
+
+        state.mkAttrs(*modules, modulesInfo.size());
+        for (auto & [path, url] : modulesInfo) {
             Value *vUrl = state.allocValue();
             mkString(*vUrl, std::get<string>(url).c_str());
             modules->attrs->push_back(Attr(state.symbols.create(path), vUrl));
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index 8becf1e3bc5..c43b047a772 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -35,7 +35,6 @@ struct Input
 
     std::shared_ptr<InputScheme> scheme; // note: can be null
     Attrs attrs;
-    Attrs modules;
     bool immutable = false;
     bool direct = true;
 
diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 0d2065fe64e..4ac6655d538 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -9,6 +9,8 @@
 #include <sys/time.h>
 #include <sys/wait.h>
 
+#include <nlohmann/json.hpp>
+
 using namespace std::string_literals;
 
 namespace nix::fetchers {
@@ -296,7 +298,7 @@ struct GitInputScheme : InputScheme
             });
         };
 
-        auto makeResult = [&](const Attrs & infoAttrs, StorePath && storePath, Attrs modulesInfo)
+        auto makeResult = [&](const Attrs & infoAttrs, StorePath && storePath)
             -> std::pair<Tree, Input>
         {
             assert(input.getRev());
@@ -304,7 +306,7 @@ struct GitInputScheme : InputScheme
             if (!shallow)
                 input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
             input.attrs.insert_or_assign("lastModified", getIntAttr(infoAttrs, "lastModified"));
-            input.modules = modulesInfo;
+            input.attrs.insert_or_assign("modules", getStrAttr(infoAttrs, "modules"));
             return {
                 Tree(store->toRealPath(storePath), std::move(storePath)),
                 input
@@ -313,10 +315,7 @@ struct GitInputScheme : InputScheme
 
         if (input.getRev()) {
             if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
-                Path p = store->printStorePath(res->second);
-                printTalkative("case 1");
-                auto modulesInfo = readSubmodules(p, "FIXME");
-                return makeResult(res->first, std::move(res->second), modulesInfo);
+                return makeResult(res->first, std::move(res->second));
             }
         }
 
@@ -330,14 +329,11 @@ struct GitInputScheme : InputScheme
 
             /* Check whether this repo has any commits. There are
                probably better ways to do this. */
-            auto gitDir = actualUrl + "/.git";
-            auto commonGitDir = chomp(runProgram(
+            auto gitDir = chomp(runProgram(
                 "git",
                 true,
                 { "-C", actualUrl, "rev-parse", "--git-common-dir" }
             ));
-            if (commonGitDir != ".git")
-                    gitDir = commonGitDir;
 
             bool haveCommits = !readDirectory(gitDir + "/refs/heads").empty();
 
@@ -390,8 +386,8 @@ struct GitInputScheme : InputScheme
                     "lastModified",
                     haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0);
 
-                auto modulesInfo = readSubmodules(actualUrl, "HEAD");
-                input.modules = modulesInfo;
+                input.attrs.insert_or_assign("modules", "{}");
+
                 return {
                     Tree(store->toRealPath(storePath), std::move(storePath)),
                     input
@@ -424,10 +420,7 @@ struct GitInputScheme : InputScheme
                 auto rev2 = Hash::parseAny(getStrAttr(res->first, "rev"), htSHA1);
                 if (!input.getRev() || input.getRev() == rev2) {
                     input.attrs.insert_or_assign("rev", rev2.gitRev());
-                    Path p = store->printStorePath(res->second);
-                    printTalkative("case 2");
-                    auto modulesInfo = readSubmodules(p, "FIXME");
-                    return makeResult(res->first, std::move(res->second), modulesInfo);
+                    return makeResult(res->first, std::move(res->second));
                 }
             }
 
@@ -521,9 +514,7 @@ struct GitInputScheme : InputScheme
         /* Now that we know the ref, check again whether we have it in
            the store. */
         if (auto res = getCache()->lookup(store, getImmutableAttrs())) {
-            printTalkative("case 3");
-            auto modulesInfo = readSubmodules(repoDir, input.getRev()->gitRev());
-            return makeResult(res->first, std::move(res->second), modulesInfo);
+            return makeResult(res->first, std::move(res->second));
         }
 
         Path tmpDir = createTempDir();
@@ -582,18 +573,18 @@ struct GitInputScheme : InputScheme
         auto storePath = store->addToStore(name, tmpDir, FileIngestionMethod::Recursive, htSHA256, filter);
 
         auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", "--no-show-signature", input.getRev()->gitRev() }));
+        auto rev = input.getRev()->gitRev();
+        auto modulesInfo = readSubmodules(actualUrl, rev);
 
         Attrs infoAttrs({
-            {"rev", input.getRev()->gitRev()},
+            {"rev", rev},
             {"lastModified", lastModified},
+            {"modules", attrsToJSON(modulesInfo).dump()},
         });
 
-        printTalkative("case 4");
-        auto modulesInfo = readSubmodules(actualUrl, "FIXME");
-
         if (!shallow)
             infoAttrs.insert_or_assign("revCount",
-                std::stoull(runProgram("git", true, { "-C", repoDir, "rev-list", "--count", input.getRev()->gitRev() })));
+                std::stoull(runProgram("git", true, { "-C", repoDir, "rev-list", "--count", rev })));
 
         if (!_input.getRev())
             getCache()->add(
@@ -610,7 +601,7 @@ struct GitInputScheme : InputScheme
             storePath,
             true);
 
-        return makeResult(infoAttrs, std::move(storePath), modulesInfo);
+        return makeResult(infoAttrs, std::move(storePath));
     }
 };
 
diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh
index 89294d8d2bc..84e5026e5c9 100644
--- a/tests/fetchGit.sh
+++ b/tests/fetchGit.sh
@@ -37,6 +37,8 @@ path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; u
 export _NIX_FORCE_HTTP=1
 [[ $(tail -n 1 $path0/hello) = "hello" ]]
 
+ls -l $repo
+nix eval --debug --impure --raw --expr "(builtins.fetchGit file://$repo).outPath"
 # Fetch the default branch.
 path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath")
 [[ $(cat $path/hello) = world ]]
diff --git a/tests/fetchGitSubmodules.sh b/tests/fetchGitSubmodules.sh
index 087095956cf..199521cd238 100644
--- a/tests/fetchGitSubmodules.sh
+++ b/tests/fetchGitSubmodules.sh
@@ -38,10 +38,6 @@ git -C $rootRepo commit -m "Add submodule"
 
 rev=$(git -C $rootRepo rev-parse HEAD)
 
-nix eval --json --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).modules"
-
-false
-
 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")
@@ -99,3 +95,7 @@ noSubmoduleRepoBaseline=$(nix eval --raw --expr "(builtins.fetchGit { url = file
 noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subRepo; rev = \"$subRev\"; submodules = true; }).outPath")
 
 [[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]]
+
+subUrl=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).modules.sub")
+
+[[ $subUrl =~ rev=$subRev$ ]]

From d54d8ec55e39953d425a476a7bd031748d186138 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Wed, 20 Oct 2021 16:11:55 +0000
Subject: [PATCH 04/17] Fix fetchGit.sh tests

---
 src/libfetchers/git.cc | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 4ac6655d538..f181f9726d0 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -329,11 +329,14 @@ struct GitInputScheme : InputScheme
 
             /* Check whether this repo has any commits. There are
                probably better ways to do this. */
-            auto gitDir = chomp(runProgram(
+            auto gitDir = actualUrl + "/.git";
+            auto commonGitDir = chomp(runProgram(
                 "git",
                 true,
                 { "-C", actualUrl, "rev-parse", "--git-common-dir" }
             ));
+            if (commonGitDir != ".git")
+                    gitDir = commonGitDir;
 
             bool haveCommits = !readDirectory(gitDir + "/refs/heads").empty();
 
@@ -574,7 +577,7 @@ struct GitInputScheme : InputScheme
 
         auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", "--no-show-signature", input.getRev()->gitRev() }));
         auto rev = input.getRev()->gitRev();
-        auto modulesInfo = readSubmodules(actualUrl, rev);
+        auto modulesInfo = readSubmodules(repoDir, rev);
 
         Attrs infoAttrs({
             {"rev", rev},

From 2c9ec9dba96065d470d29441e8b772d3cbcebbb5 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Wed, 20 Oct 2021 16:28:42 +0000
Subject: [PATCH 05/17] Get flake.sh tests to pass

---
 src/libfetchers/git.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index f181f9726d0..786e3fd8506 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -167,7 +167,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" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs" && name != "name")
+            if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs" && name != "name" && name != "modules")
                 throw Error("unsupported Git input attribute '%s'", name);
 
         parseURL(getStrAttr(attrs, "url"));

From 1ca14d58a615646094ea61bd1a554f31d7687f32 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Wed, 20 Oct 2021 17:11:31 +0000
Subject: [PATCH 06/17] Fix wrong path

---
 src/libfetchers/git.cc | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 786e3fd8506..c2508115b28 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -113,12 +113,12 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
     printTalkative("submodule file %s", submodules);
 
     auto parsedModules = parseSubmodules(submodules);
-    for (auto & [path, url] : parsedModules) {
-      printTalkative("submodule %s fetched from %s", path, url);
-      auto commitHash = findCommitHash(path, entries, path);
+    for (auto & [subPath, url] : parsedModules) {
+      printTalkative("submodule %s fetched from %s", subPath, url);
+      auto commitHash = findCommitHash(path, entries, subPath);
       printTalkative("found %s", commitHash);
 
-      attrs.emplace(path, url + "?rev=" + commitHash);
+      attrs.emplace(subPath, url + "?rev=" + commitHash);
     }
 
     return attrs;

From f5fa4b0ee3c7f742f5946969570902737ed099bf Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Wed, 20 Oct 2021 18:39:10 +0000
Subject: [PATCH 07/17] Fix dirty tree scenario

---
 src/libfetchers/git.cc      | 5 ++++-
 tests/fetchGitSubmodules.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index c2508115b28..75b6ed8c304 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -389,7 +389,10 @@ struct GitInputScheme : InputScheme
                     "lastModified",
                     haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0);
 
-                input.attrs.insert_or_assign("modules", "{}");
+                input.attrs.insert_or_assign(
+                    "modules",
+                    haveCommits ?
+                    attrsToJSON(readSubmodules(actualUrl, "HEAD")).dump() : "{}");
 
                 return {
                     Tree(store->toRealPath(storePath), std::move(storePath)),
diff --git a/tests/fetchGitSubmodules.sh b/tests/fetchGitSubmodules.sh
index 199521cd238..0a037d815e1 100644
--- a/tests/fetchGitSubmodules.sh
+++ b/tests/fetchGitSubmodules.sh
@@ -30,6 +30,7 @@ initGitRepo $subRepo
 addGitContent $subRepo
 
 initGitRepo $rootRepo
+addGitContent $rootRepo
 
 git -C $rootRepo submodule init
 git -C $rootRepo submodule add $subRepo sub
@@ -99,3 +100,11 @@ noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subR
 subUrl=$(nix eval --raw --expr "(builtins.fetchGit { url = $rootRepo; rev = \"$rev\"; }).modules.sub")
 
 [[ $subUrl =~ rev=$subRev$ ]]
+
+# Beschmutzigen...
+echo etwa dreck > $rootRepo/content
+
+subUrl=$(nix eval --impure --raw --expr "(builtins.fetchGit $rootRepo).modules.sub")
+
+# Submodule is still clean so should be the same as above
+[[ $subUrl =~ rev=$subRev$ ]]

From 28d671594d6535b6acfbea2afd41211ac4945d86 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Thu, 21 Oct 2021 21:35:25 +0000
Subject: [PATCH 08/17] Explicitly add file:// when bare path is given

---
 src/libfetchers/git.cc | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 75b6ed8c304..284cebd9428 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -118,7 +118,13 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
       auto commitHash = findCommitHash(path, entries, subPath);
       printTalkative("found %s", commitHash);
 
-      attrs.emplace(subPath, url + "?rev=" + commitHash);
+      static const std::regex barePathRegex("^/.*$");
+      std::string prefix = "git+";
+      if (std::regex_match(url, barePathRegex))
+          prefix = prefix + "file://";
+
+
+      attrs.emplace(subPath, prefix + url + "?allRefs=1&rev=" + commitHash);
     }
 
     return attrs;

From 2ff9b2044f43c7d2c5cd7932a7b7af8fc64cf717 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Thu, 4 Nov 2021 12:59:16 -0400
Subject: [PATCH 09/17] Add test to demonstrate

---
 tests/flakes-with-submodules.sh | 117 ++++++++++++++++++++++++++++++++
 tests/local.mk                  |   1 +
 2 files changed, 118 insertions(+)
 create mode 100644 tests/flakes-with-submodules.sh

diff --git a/tests/flakes-with-submodules.sh b/tests/flakes-with-submodules.sh
new file mode 100644
index 00000000000..eb243e17178
--- /dev/null
+++ b/tests/flakes-with-submodules.sh
@@ -0,0 +1,117 @@
+source common.sh
+
+if [[ -z $(type -p git) ]]; then
+    echo "Git not installed; skipping flake tests"
+    exit 99
+fi
+
+clearStore
+rm -rf $TEST_HOME/.{cache,config}
+
+registry=$TEST_ROOT/registry.json
+
+nonFlakeDir=$TEST_ROOT/nonFlake
+flakeDir=$TEST_ROOT/flake
+flakeWithSubmodules=$TEST_ROOT/flakeWithSubmodules
+
+for repo in $flakeDir $nonFlakeDir $flakeWithSubmodules; do
+    rm -rf $repo $repo.tmp
+    mkdir -p $repo
+    git -C $repo init
+    git -C $repo config user.email "foobar@example.com"
+    git -C $repo config user.name "Foobar"
+done
+
+for dir in $nonFlakeDir $flakeDir; do
+cat > $dir/README.md <<EOF
+FNORD
+EOF
+git -C $dir add README.md
+git -C $dir commit -m 'Initial'
+done
+
+cp config.nix $flakeDir
+
+cat > $flakeDir/flake.nix <<EOF
+{
+    description = "Flake whose defaultPackage prints out its README.md";
+
+    outputs = { self }: with import ./config.nix; let
+        inherit (self.modules) nonFlake;
+        writeShellScriptBin = name: script: mkDerivation {
+            inherit name;
+            buildCommand = ''
+                mkdir -p \$out/bin
+                bin=\$_/\${name}
+                echo '#!/bin/sh' > \$bin
+                echo '\${script}' >> \$bin
+                chmod +x \$bin
+            '';
+        };
+    in {
+        packages.$system = {
+            cat-own-readme = writeShellScriptBin "cat-own-readme" ''
+                cat \${self}/README.md
+            '';
+        };
+
+        defaultPackage.$system = self.packages.$system.cat-own-readme;
+    };
+}
+EOF
+
+git -C $flakeDir add .
+git -C $flakeDir commit -m'add flake.nix'
+
+[[ $(nix run $flakeDir#cat-own-readme) == "FNORD" ]]
+
+git -C $flakeWithSubmodules submodule add $nonFlakeDir
+git -C $flakeWithSubmodules submodule add $flakeDir
+git -C $flakeWithSubmodules add .
+git -C $flakeWithSubmodules commit -m'add submodules'
+
+cp config.nix $flakeWithSubmodules
+
+cat > $flakeWithSubmodules/flake.nix <<EOF
+{
+    description = "Flake with Submodules";
+
+    outputs = { self }: with import ./config.nix; let
+        traceVal = val: builtins.trace val val;
+        nonFlake = builtins.fetchTree (traceVal self.modules.nonFlake);
+        flake = builtins.getFlake (traceVal self.modules.flake);
+
+        writeShellScriptBin = name: script: mkDerivation {
+            inherit name;
+            buildCommand = ''
+                mkdir -p \$out/bin
+                bin=\$_/\${name}
+                echo '#!/bin/sh' > \$bin
+                echo '\${script}' >> \$bin
+                chmod +x \$bin
+            '';
+        };
+    in {
+        packages.$system = {
+            cat-submodule-readme = writeShellScriptBin "cat-submodule-readme" ''
+                cat \${nonFlake}/README.md
+            '';
+            use-submodule-as-flake = flake.packages.$system.cat-own-readme;
+        };
+
+        defaultPackage.$system = self.packages.$system.cat-submodule-readme;
+    };
+}
+EOF
+
+git -C $flakeWithSubmodules add .
+git -C $flakeWithSubmodules commit -m'add flake.nix'
+
+[[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FNORD" ]]
+[[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FNORD" ]]
+
+# TODO make the dirty case do the Right Thing as well.
+#echo FSOUTH > $flakeWithSubmodules/nonFlake/README.md
+#[[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FSOUTH" ]]
+# [[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FSOUTH" ]]
+# nix run -vv $flakeWithSubmodules#cat-submodule-readme
diff --git a/tests/local.mk b/tests/local.mk
index 936b72c2a09..a7d43e1a6e5 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -47,6 +47,7 @@ nix_tests = \
   describe-stores.sh \
   flakes.sh \
   flake-local-settings.sh \
+  flakes-with-submodules.sh \
   build.sh \
   repl.sh ca/repl.sh \
   ca/build.sh \

From b2bf6ccc75c1c222b5fa89f4ca16863e134bc7b7 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 16 Nov 2021 21:26:39 +0000
Subject: [PATCH 10/17] Add tests for unclean submodule state

---
 tests/flakes-with-submodules.sh | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/flakes-with-submodules.sh b/tests/flakes-with-submodules.sh
index eb243e17178..ce9e083483f 100644
--- a/tests/flakes-with-submodules.sh
+++ b/tests/flakes-with-submodules.sh
@@ -110,8 +110,10 @@ git -C $flakeWithSubmodules commit -m'add flake.nix'
 [[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FNORD" ]]
 [[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FNORD" ]]
 
-# TODO make the dirty case do the Right Thing as well.
-#echo FSOUTH > $flakeWithSubmodules/nonFlake/README.md
-#[[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FSOUTH" ]]
-# [[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FSOUTH" ]]
-# nix run -vv $flakeWithSubmodules#cat-submodule-readme
+# apply dirt
+echo FSUD > $flakeWithSubmodules/nonFlake/README.md
+[[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FSUD" ]]
+
+# should work for flake as well
+echo FSUD > $flakeWithSubmodules/flake/README.md
+[[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FSUD" ]]

From 594a51ed1eeeecd1ebc9eaa81dc5c48b874a4348 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 16 Nov 2021 21:30:11 +0000
Subject: [PATCH 11/17] Factor out some logic I might need

not necessarily necessary but it helps me wrap my head around things
---
 src/libfetchers/git.cc | 148 +++++++++++++++++++++++------------------
 1 file changed, 83 insertions(+), 65 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 284cebd9428..e5872ed2e27 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -44,6 +44,69 @@ string getBlob(const Path & path, const string & gitrev, const string & blobhash
   return runProgram("git", true, { "-C", path, "cat-file", "-p", blobhash });
 }
 
+std::pair<bool, std::string> getActualUrl(const Input &input) {
+  // file:// URIs are normally not cloned (but otherwise treated the
+  // same as remote URIs, i.e. we don't use the working tree or
+  // HEAD). Exception: If _NIX_FORCE_HTTP is set, or the repo is a bare git
+  // repo, treat as a remote URI to force a clone.
+  static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; // for testing
+  auto url = parseURL(getStrAttr(input.attrs, "url"));
+  bool isBareRepository =
+      url.scheme == "file" && !pathExists(url.path + "/.git");
+  bool isLocal = url.scheme == "file" && !forceHttp && !isBareRepository;
+  return {isLocal, isLocal ? url.path : url.base};
+}
+
+std::string getGitDir(const std::string & actualUrl) {
+  auto gitDir = actualUrl + "/.git";
+  auto commonGitDir = chomp(runProgram(
+      "git", true, {"-C", actualUrl, "rev-parse", "--git-common-dir"}));
+  if (commonGitDir != ".git")
+    gitDir = commonGitDir;
+
+  return gitDir;
+}
+
+/* Check whether this repo has any commits. There are
+   probably better ways to do this. */
+bool hasCommits(const std::string & actualUrl) {
+  auto gitDir = getGitDir(actualUrl);
+
+  return !readDirectory(gitDir + "/refs/heads").empty();
+}
+
+Tree copyTrackedFiles(ref<Store> store, const std::string & inputName, const std::string & actualUrl, bool submodules) {
+  auto gitDir = getGitDir(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, gitOpts), "\0"s);
+
+  PathFilter filter = [&](const Path &p) -> bool {
+    assert(hasPrefix(p, actualUrl));
+    std::string file(p, actualUrl.size() + 1);
+
+    auto st = lstat(p);
+
+    if (S_ISDIR(st.st_mode)) {
+      auto prefix = file + "/";
+      auto i = files.lower_bound(prefix);
+      return i != files.end() && hasPrefix(*i, prefix);
+    }
+
+    return files.count(file);
+  };
+
+  auto storePath =
+      store->addToStore(inputName, actualUrl,
+                        FileIngestionMethod::Recursive, htSHA256, filter);
+
+  return Tree(store->toRealPath(storePath), std::move(storePath));
+}
+
 static std::map<string, std::pair<gitType, string>> gitTreeList(const Path & path, const string & treehash) {
   auto treeOutput = runProgram("git", true, { "-C", path, "cat-file", "-p", treehash });
   auto lines = tokenizeString<std::vector<string>>(treeOutput, "\n");
@@ -80,6 +143,22 @@ std::map<string,string> parseSubmodules(const string & contents) {
   return results;
 }
 
+bool isClean(const string & actualUrl) {
+  bool haveCommits = hasCommits(actualUrl);
+
+  try {
+    if (haveCommits) {
+      runProgram("git", true,
+                 {"-C", actualUrl, "diff-index", "--quiet", "HEAD", "--"});
+      return true;
+    }
+  } catch (ExecError &e) {
+    if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 1)
+      throw;
+  }
+  return false;
+}
+
 string findCommitHash(const Path & path, const std::map<string, std::pair<gitType, string>> & _entries, const string & pathToFind) {
   auto entries = _entries;
 
@@ -265,19 +344,6 @@ struct GitInputScheme : InputScheme
                 { "-C", *sourcePath, "commit", std::string(file), "-m", *commitMsg });
     }
 
-    std::pair<bool, std::string> getActualUrl(const Input & input) const
-    {
-        // file:// URIs are normally not cloned (but otherwise treated the
-        // same as remote URIs, i.e. we don't use the working tree or
-        // HEAD). Exception: If _NIX_FORCE_HTTP is set, or the repo is a bare git
-        // repo, treat as a remote URI to force a clone.
-        static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; // for testing
-        auto url = parseURL(getStrAttr(input.attrs, "url"));
-        bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git");
-        bool isLocal = url.scheme == "file" && !forceHttp && !isBareRepository;
-        return {isLocal, isLocal ? url.path : url.base};
-    }
-
     std::pair<Tree, Input> fetch(ref<Store> store, const Input & _input) override
     {
         Input input(_input);
@@ -331,31 +397,7 @@ struct GitInputScheme : InputScheme
         // If this is a local directory and no ref or revision is
         // given, then allow the use of an unclean working tree.
         if (!input.getRef() && !input.getRev() && isLocal) {
-            bool clean = false;
-
-            /* Check whether this repo has any commits. There are
-               probably better ways to do this. */
-            auto gitDir = actualUrl + "/.git";
-            auto commonGitDir = chomp(runProgram(
-                "git",
-                true,
-                { "-C", actualUrl, "rev-parse", "--git-common-dir" }
-            ));
-            if (commonGitDir != ".git")
-                    gitDir = commonGitDir;
-
-            bool haveCommits = !readDirectory(gitDir + "/refs/heads").empty();
-
-            try {
-                if (haveCommits) {
-                    runProgram("git", true, { "-C", actualUrl, "diff-index", "--quiet", "HEAD", "--" });
-                    clean = true;
-                }
-            } catch (ExecError & e) {
-                if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 1) throw;
-            }
-
-            if (!clean) {
+            if (!isClean(actualUrl)) {
 
                 /* This is an unclean working tree. So copy all tracked files. */
 
@@ -365,32 +407,11 @@ struct GitInputScheme : InputScheme
                 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, gitOpts), "\0"s);
-
-                PathFilter filter = [&](const Path & p) -> bool {
-                    assert(hasPrefix(p, actualUrl));
-                    std::string file(p, actualUrl.size() + 1);
-
-                    auto st = lstat(p);
-
-                    if (S_ISDIR(st.st_mode)) {
-                        auto prefix = file + "/";
-                        auto i = files.lower_bound(prefix);
-                        return i != files.end() && hasPrefix(*i, prefix);
-                    }
-
-                    return files.count(file);
-                };
-
-                auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter);
+                auto tree = copyTrackedFiles(store, input.getName(), actualUrl, submodules);
 
                 // FIXME: maybe we should use the timestamp of the last
                 // modified dirty file?
+                bool haveCommits = hasCommits(actualUrl);
                 input.attrs.insert_or_assign(
                     "lastModified",
                     haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0);
@@ -400,10 +421,7 @@ struct GitInputScheme : InputScheme
                     haveCommits ?
                     attrsToJSON(readSubmodules(actualUrl, "HEAD")).dump() : "{}");
 
-                return {
-                    Tree(store->toRealPath(storePath), std::move(storePath)),
-                    input
-                };
+                return { tree, input };
             }
         }
 

From f55b99f7b6655396b03136f5efb35ae7f6b98ced Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 16 Nov 2021 21:31:02 +0000
Subject: [PATCH 12/17] Implement support for unclean submodule scenario

---
 src/libfetchers/git.cc | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index e5872ed2e27..b68e1ec5ab9 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -5,6 +5,7 @@
 #include "store-api.hh"
 #include "url-parts.hh"
 #include "pathlocks.hh"
+#include "archive.hh"
 
 #include <sys/time.h>
 #include <sys/wait.h>
@@ -193,17 +194,29 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
 
     auto parsedModules = parseSubmodules(submodules);
     for (auto & [subPath, url] : parsedModules) {
-      printTalkative("submodule %s fetched from %s", subPath, url);
-      auto commitHash = findCommitHash(path, entries, subPath);
-      printTalkative("found %s", commitHash);
-
-      static const std::regex barePathRegex("^/.*$");
-      std::string prefix = "git+";
-      if (std::regex_match(url, barePathRegex))
-          prefix = prefix + "file://";
-
-
-      attrs.emplace(subPath, prefix + url + "?allRefs=1&rev=" + commitHash);
+      auto fullPath = path + "/" + subPath;
+      auto initialized = !readDirectory(fullPath).empty();
+      auto clean = isClean(fullPath);
+
+      if (!initialized || clean) {
+        printTalkative("submodule %s fetched from %s", subPath, url);
+        auto commitHash = findCommitHash(path, entries, subPath);
+        printTalkative("found %s", commitHash);
+
+        static const std::regex barePathRegex("^/.*$");
+        std::string prefix = "git+";
+        if (std::regex_match(url, barePathRegex))
+          prefix += "file://";
+
+        std::string suffix = "?allRefs=1&rev=" + commitHash;
+        attrs.emplace(subPath, prefix + url + suffix);
+      } else {
+        StringSink sink;
+        dumpPath(fullPath, sink);
+
+        auto narHash = hashString(htSHA256, *sink.s);
+        attrs.emplace(subPath, "path://" + fullPath + "?narHash=" + narHash.to_string(SRI, false));
+      }
     }
 
     return attrs;

From ce081f0f24540adc3ad827c06c8a5010b737469f Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 30 Nov 2021 18:27:09 +0000
Subject: [PATCH 13/17] printTalkative spam

---
 src/libfetchers/git.cc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index b68e1ec5ab9..2aed0ef975c 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -73,6 +73,7 @@ std::string getGitDir(const std::string & actualUrl) {
 bool hasCommits(const std::string & actualUrl) {
   auto gitDir = getGitDir(actualUrl);
 
+  printTalkative("in hasCommits\nactualUrl %s\ngitDir %s", actualUrl, gitDir);
   return !readDirectory(gitDir + "/refs/heads").empty();
 }
 
@@ -141,6 +142,7 @@ std::map<string,string> parseSubmodules(const string & contents) {
       results.insert(std::pair{path, url});
     }
   }
+  printTalkative("Saving %s\t%s\t%s\n", *path, *url, branch.value_or("master"));
   return results;
 }
 
@@ -190,7 +192,7 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
 
     auto submodules = getBlob(path, gitrev, i->second.second);
 
-    printTalkative("submodule file %s", submodules);
+    printTalkative("submodule file\n %s", submodules);
 
     auto parsedModules = parseSubmodules(submodules);
     for (auto & [subPath, url] : parsedModules) {
@@ -216,6 +218,7 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
 
         auto narHash = hashString(htSHA256, *sink.s);
         attrs.emplace(subPath, "path://" + fullPath + "?narHash=" + narHash.to_string(SRI, false));
+        printTalkative("DIRTY\nsubPath: %s\nurl: %s\ndirtyUrl: %s\n", subPath, url, "path://" + fullPath + "?narHash=" + narHash.to_string(SRI, false));
       }
     }
 

From 124e703024ee5a0aedb537c0d6bfe59cc3e81fbf Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 30 Nov 2021 18:27:26 +0000
Subject: [PATCH 14/17] Fix wrong repo path

---
 src/libfetchers/git.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index 2aed0ef975c..fe3e7c756c0 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -61,7 +61,7 @@ std::pair<bool, std::string> getActualUrl(const Input &input) {
 std::string getGitDir(const std::string & actualUrl) {
   auto gitDir = actualUrl + "/.git";
   auto commonGitDir = chomp(runProgram(
-      "git", true, {"-C", actualUrl, "rev-parse", "--git-common-dir"}));
+      "git", true, {"-C", actualUrl, "rev-parse", "--path-format=absolute", "--git-common-dir"}));
   if (commonGitDir != ".git")
     gitDir = commonGitDir;
 

From 50d13e727535e3c9576310fdd481bec417b7ae77 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 30 Nov 2021 18:27:53 +0000
Subject: [PATCH 15/17] Support specifying `branch` in `.gitmodules`

---
 src/libfetchers/git.cc | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc
index fe3e7c756c0..57a5f58fc1e 100644
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@@ -131,18 +131,26 @@ static std::map<string, std::pair<gitType, string>> gitTreeList(const Path & pat
 std::map<string,string> parseSubmodules(const string & contents) {
   auto lines = tokenizeString<std::vector<string>>(contents, "\n");
   std::map<string,string> results;
-  string path;
+  std::optional<string> path, url, branch;
   for (auto line : lines) {
     auto words = tokenizeString<std::vector<string>>(line);
-    if (words[1] != "=") continue;
+    if (words[1] != "=") {
+        if (line != lines[0]) {
+            printTalkative("Saving %s\t%s\t%s\n", *path, *url, branch.value_or("master"));
 
-    if (words[0] == "path") path = words[2];
-    if (words[0] == "url") {
-      string url = words[2];
-      results.insert(std::pair{path, url});
+            results.insert(std::pair{*path, *url + "?ref=" + branch.value_or("master")});
+            path = url = branch = {};
+        }
+        continue;
     }
+
+    if (words[0] == "path") path = words[2];
+    if (words[0] == "branch") branch = words[2];
+    if (words[0] == "url") url = words[2];
+
   }
   printTalkative("Saving %s\t%s\t%s\n", *path, *url, branch.value_or("master"));
+  results.insert(std::pair{*path, *url + "?ref=" + branch.value_or("master")});
   return results;
 }
 
@@ -197,10 +205,9 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
     auto parsedModules = parseSubmodules(submodules);
     for (auto & [subPath, url] : parsedModules) {
       auto fullPath = path + "/" + subPath;
-      auto initialized = !readDirectory(fullPath).empty();
-      auto clean = isClean(fullPath);
+      auto initialized = pathExists(fullPath) && !readDirectory(fullPath).empty();
 
-      if (!initialized || clean) {
+      if (!initialized || isClean(fullPath)) {
         printTalkative("submodule %s fetched from %s", subPath, url);
         auto commitHash = findCommitHash(path, entries, subPath);
         printTalkative("found %s", commitHash);
@@ -210,7 +217,8 @@ static Attrs readSubmodules(const Path & path, const string & gitrev)
         if (std::regex_match(url, barePathRegex))
           prefix += "file://";
 
-        std::string suffix = "?allRefs=1&rev=" + commitHash;
+        // std::string suffix = "?allRefs=1&rev=" + commitHash;
+        std::string suffix = "&rev=" + commitHash;
         attrs.emplace(subPath, prefix + url + suffix);
       } else {
         StringSink sink;

From 551cb987b66414d724e27fd5ee2ccc381c222079 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Tue, 30 Nov 2021 18:28:21 +0000
Subject: [PATCH 16/17] Test recomposing source with submodules

oddly this doesn't reproduce the problem I'm experiencing IRL where
"dirtying" the submodule has no effect until the cache is either nuked
or runs out of TTL
---
 tests/flakes-with-submodules.sh | 46 +++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/tests/flakes-with-submodules.sh b/tests/flakes-with-submodules.sh
index ce9e083483f..904c433f707 100644
--- a/tests/flakes-with-submodules.sh
+++ b/tests/flakes-with-submodules.sh
@@ -20,14 +20,9 @@ for repo in $flakeDir $nonFlakeDir $flakeWithSubmodules; do
     git -C $repo init
     git -C $repo config user.email "foobar@example.com"
     git -C $repo config user.name "Foobar"
-done
-
-for dir in $nonFlakeDir $flakeDir; do
-cat > $dir/README.md <<EOF
-FNORD
-EOF
-git -C $dir add README.md
-git -C $dir commit -m 'Initial'
+    echo FNORD > $repo/README.md
+    git -C $repo add README.md
+    git -C $repo commit -m 'Initial'
 done
 
 cp config.nix $flakeDir
@@ -81,6 +76,11 @@ cat > $flakeWithSubmodules/flake.nix <<EOF
         nonFlake = builtins.fetchTree (traceVal self.modules.nonFlake);
         flake = builtins.getFlake (traceVal self.modules.flake);
 
+        mapAttrsToList = f: attrs:
+          map (name: f name attrs.\${name}) (builtins.attrNames attrs);
+        pipe = val: functions:
+          let reverseApply = x: f: f x;
+          in builtins.foldl' reverseApply val functions;
         writeShellScriptBin = name: script: mkDerivation {
             inherit name;
             buildCommand = ''
@@ -91,12 +91,35 @@ cat > $flakeWithSubmodules/flake.nix <<EOF
                 chmod +x \$bin
             '';
         };
+
+        combineModules = self: let
+          srcs = builtins.mapAttrs (_: url: fetchTree (traceVal url)) self.modules;
+        in mkDerivation {
+          name = "source-with-submodules";
+          buildCommand = ''
+            cp -r \${self} \$out
+            chmod -R +w \$_
+
+            \${
+              pipe srcs [
+                (mapAttrsToList (path: src: "cp -rT \${src} \$out/\${path}"))
+                (builtins.concatStringsSep "\n")
+              ]
+            }
+          '';
+        };
     in {
         packages.$system = {
             cat-submodule-readme = writeShellScriptBin "cat-submodule-readme" ''
                 cat \${nonFlake}/README.md
             '';
             use-submodule-as-flake = flake.packages.$system.cat-own-readme;
+
+            cat-own-readme = writeShellScriptBin "cat-own-readme" ''
+                cat \${self}/README.md
+            '';
+
+            source-with-submodules = combineModules self;
         };
 
         defaultPackage.$system = self.packages.$system.cat-submodule-readme;
@@ -107,12 +130,19 @@ EOF
 git -C $flakeWithSubmodules add .
 git -C $flakeWithSubmodules commit -m'add flake.nix'
 
+[[ $(nix run $flakeWithSubmodules#cat-own-readme) == "FNORD" ]]
 [[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FNORD" ]]
 [[ $(nix run $flakeWithSubmodules#use-submodule-as-flake) == "FNORD" ]]
+nix build -o $TEST_ROOT/result $flakeWithSubmodules#source-with-submodules
+[[ $(cat $TEST_ROOT/result/nonFlake/README.md) == "FNORD" ]]
+
+echo FOST > $flakeWithSubmodules/README.md
 
 # apply dirt
 echo FSUD > $flakeWithSubmodules/nonFlake/README.md
 [[ $(nix run $flakeWithSubmodules#cat-submodule-readme) == "FSUD" ]]
+nix build -o $TEST_ROOT/result $flakeWithSubmodules#source-with-submodules
+[[ $(cat $TEST_ROOT/result/nonFlake/README.md) == "FSUD" ]]
 
 # should work for flake as well
 echo FSUD > $flakeWithSubmodules/flake/README.md

From c2cde79247fc9bf44ab6f3c9359726b5dc168d71 Mon Sep 17 00:00:00 2001
From: Shay Bergmann <shaybergmann@gmail.com>
Date: Mon, 6 Dec 2021 16:36:55 +0000
Subject: [PATCH 17/17] Add the submodule info to flake fingerprint

This allows changes to dirty submodules to propagate correctly and
trigger a re-eval as one would expect. This previously only worked with
`__contentAddressed = true;`.
---
 src/libexpr/flake/flake.cc  | 3 ++-
 src/libfetchers/fetchers.cc | 7 +++++++
 src/libfetchers/fetchers.hh | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc
index 33d253eee79..c30119482e0 100644
--- a/src/libexpr/flake/flake.cc
+++ b/src/libexpr/flake/flake.cc
@@ -727,9 +727,10 @@ Fingerprint LockedFlake::getFingerprint() const
     // and we haven't changed it, then it's sufficient to use
     // flake.sourceInfo.storePath for the fingerprint.
     return hashString(htSHA256,
-        fmt("%s;%s;%d;%d;%s",
+        fmt("%s;%s;%s;%d;%d;%s",
             flake.sourceInfo->storePath.to_string(),
             flake.lockedRef.subdir,
+            flake.lockedRef.input.getModules().value_or(""),
             flake.lockedRef.input.getRevCount().value_or(0),
             flake.lockedRef.input.getLastModified().value_or(0),
             lockFile));
diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc
index e158d914bb3..8fd7366ae9e 100644
--- a/src/libfetchers/fetchers.cc
+++ b/src/libfetchers/fetchers.cc
@@ -257,6 +257,13 @@ std::optional<time_t> Input::getLastModified() const
     return {};
 }
 
+std::optional<std::string> Input::getModules() const
+{
+    if (auto s = maybeGetStrAttr(attrs, "modules"))
+        return *s;
+    return {};
+}
+
 ParsedURL InputScheme::toURL(const Input & input)
 {
     throw Error("don't know how to convert input '%s' to a URL", attrsToJSON(input.attrs));
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index c43b047a772..92588d8a9b6 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -95,6 +95,7 @@ public:
     std::optional<Hash> getRev() const;
     std::optional<uint64_t> getRevCount() const;
     std::optional<time_t> getLastModified() const;
+    std::optional<std::string> getModules() const;
 };