From 9d24080090539c717015add8f2d8ce02d1d84a2d Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Wed, 4 Sep 2024 14:43:43 +0200
Subject: [PATCH] Git fetcher: Ignore .gitmodules entries that are not
 submodules

Fixes #10739.
---
 src/libfetchers/git-utils.cc           | 18 ++++++++++++------
 tests/functional/fetchGitSubmodules.sh | 21 +++++++++++++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc
index 114aa4ec0..0bc930ab2 100644
--- a/src/libfetchers/git-utils.cc
+++ b/src/libfetchers/git-utils.cc
@@ -601,12 +601,16 @@ struct GitSourceAccessor : SourceAccessor
         return readBlob(path, true);
     }
 
-    Hash getSubmoduleRev(const CanonPath & path)
+    /**
+     * If `path` exists and is a submodule, return its
+     * revision. Otherwise return nothing.
+     */
+    std::optional<Hash> getSubmoduleRev(const CanonPath & path)
     {
-        auto entry = need(path);
+        auto entry = lookup(path);
 
-        if (git_tree_entry_type(entry) != GIT_OBJECT_COMMIT)
-            throw Error("'%s' is not a submodule", showPath(path));
+        if (!entry || git_tree_entry_type(entry) != GIT_OBJECT_COMMIT)
+            return std::nullopt;
 
         return toHash(*git_tree_entry_id(entry));
     }
@@ -1074,8 +1078,10 @@ std::vector<std::tuple<GitRepoImpl::Submodule, Hash>> GitRepoImpl::getSubmodules
     auto rawAccessor = getRawAccessor(rev);
 
     for (auto & submodule : parseSubmodules(pathTemp)) {
-        auto rev = rawAccessor->getSubmoduleRev(submodule.path);
-        result.push_back({std::move(submodule), rev});
+        /* Filter out .gitmodules entries that don't exist or are not
+           submodules. */
+        if (auto rev = rawAccessor->getSubmoduleRev(submodule.path))
+            result.push_back({std::move(submodule), *rev});
     }
 
     return result;
diff --git a/tests/functional/fetchGitSubmodules.sh b/tests/functional/fetchGitSubmodules.sh
index 4a3e4c347..cd3b51674 100755
--- a/tests/functional/fetchGitSubmodules.sh
+++ b/tests/functional/fetchGitSubmodules.sh
@@ -104,6 +104,27 @@ noSubmoduleRepo=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$subR
 
 [[ $noSubmoduleRepoBaseline == $noSubmoduleRepo ]]
 
+# Test .gitmodules with entries that refer to non-existent objects or objects that are not submodules.
+cat >> $rootRepo/.gitmodules <<EOF
+[submodule "missing"]
+        path = missing
+        url = https://example.org/missing.git
+
+[submodule "file"]
+        path = file
+        url = https://example.org/file.git
+EOF
+echo foo > $rootRepo/file
+git -C $rootRepo add file
+git -C $rootRepo commit -a -m "Add bad submodules"
+
+rev=$(git -C $rootRepo rev-parse HEAD)
+
+r=$(nix eval --raw --expr "builtins.fetchGit { url = file://$rootRepo; rev = \"$rev\"; submodules = true; }")
+
+[[ -f $r/file ]]
+[[ ! -e $r/missing ]]
+
 # Test relative submodule URLs.
 rm $TEST_HOME/.cache/nix/fetcher-cache*
 rm -rf $rootRepo/.git $rootRepo/.gitmodules $rootRepo/sub