From 4971a4837ff5ac6654aa75214bdd2243d4d864a5 Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Wed, 6 Sep 2023 21:30:30 +0200
Subject: [PATCH] fix: handle submodules whose entry in the index is a file.

---
 gix/src/submodule/mod.rs                      | 12 ++++++----
 .../generated-archives/make_submodules.tar.xz |  4 ++--
 gix/tests/fixtures/make_submodules.sh         |  9 ++++++++
 gix/tests/submodule/mod.rs                    | 22 +++++++++++++++++++
 4 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/gix/src/submodule/mod.rs b/gix/src/submodule/mod.rs
index 72a69dc81a5..52c5938fc28 100644
--- a/gix/src/submodule/mod.rs
+++ b/gix/src/submodule/mod.rs
@@ -160,17 +160,21 @@ impl<'repo> Submodule<'repo> {
     /// or `None` if it was deleted from the index.
     ///
     /// If `None`, but `Some()` when calling [`Self::head_id()`], then the submodule was just deleted but the change
-    /// wasn't yet committed.
+    /// wasn't yet committed. Note that `None` is also returned if the entry at the submodule path isn't a submodule.
     /// If `Some()`, but `None` when calling [`Self::head_id()`], then the submodule was just added without having committed the change.
     pub fn index_id(&self) -> Result<Option<gix_hash::ObjectId>, index_id::Error> {
         let path = self.path()?;
-        Ok(self.state.index()?.entry_by_path(&path).map(|entry| entry.id))
+        Ok(self
+            .state
+            .index()?
+            .entry_by_path(&path)
+            .and_then(|entry| (entry.mode == gix_index::entry::Mode::COMMIT).then_some(entry.id)))
     }
 
     /// Return the object id of the submodule as stored in `HEAD^{tree}` of the superproject, or `None` if it wasn't yet committed.
     ///
     /// If `Some()`, but `None` when calling [`Self::index_id()`], then the submodule was just deleted but the change
-    /// wasn't yet committed.
+    /// wasn't yet committed. Note that `None` is also returned if the entry at the submodule path isn't a submodule.
     /// If `None`, but `Some()` when calling [`Self::index_id()`], then the submodule was just added without having committed the change.
     pub fn head_id(&self) -> Result<Option<gix_hash::ObjectId>, head_id::Error> {
         let path = self.path()?;
@@ -180,7 +184,7 @@ impl<'repo> Submodule<'repo> {
             .head_commit()?
             .tree()?
             .peel_to_entry_by_path(gix_path::from_bstr(path.as_ref()))?
-            .map(|entry| entry.inner.oid))
+            .and_then(|entry| (entry.mode() == gix_object::tree::EntryMode::Commit).then_some(entry.inner.oid)))
     }
 
     /// Return the path at which the repository of the submodule should be located.
diff --git a/gix/tests/fixtures/generated-archives/make_submodules.tar.xz b/gix/tests/fixtures/generated-archives/make_submodules.tar.xz
index 2c511780fd6..97135d2fc70 100644
--- a/gix/tests/fixtures/generated-archives/make_submodules.tar.xz
+++ b/gix/tests/fixtures/generated-archives/make_submodules.tar.xz
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:0c0437e6ee0730c0ffcea0368777daec45e75f0d0ef6e7464d6b5de19f822b83
-size 21428
+oid sha256:78fe07f2f55b0f90ed2b2b70cab81ba11bacaa71943990d4206b6fef04152110
+size 23128
diff --git a/gix/tests/fixtures/make_submodules.sh b/gix/tests/fixtures/make_submodules.sh
index 8fc0584ce60..af5b9c96786 100755
--- a/gix/tests/fixtures/make_submodules.sh
+++ b/gix/tests/fixtures/make_submodules.sh
@@ -62,3 +62,12 @@ git clone --bare with-submodules with-submodules-after-clone.git
   git clone --bare ../module1 modules/m1
 )
 
+git clone with-submodules not-a-submodule
+(cd not-a-submodule
+  git submodule update --init
+  cp .gitmodules modules.bak
+  git rm m1
+  echo fake > m1
+  mv modules.bak .gitmodules
+  git add m1 && git commit -m "no submodule in index and commit, but in configuration"
+)
diff --git a/gix/tests/submodule/mod.rs b/gix/tests/submodule/mod.rs
index 5f556deea61..89d2c29d8f2 100644
--- a/gix/tests/submodule/mod.rs
+++ b/gix/tests/submodule/mod.rs
@@ -46,6 +46,18 @@ mod open {
                     },
                 )],
             ),
+            (
+                "not-a-submodule",
+                &[(
+                    "m1",
+                    gix::submodule::State {
+                        repository_exists: true,
+                        is_old_form: false,
+                        worktree_checkout: false,
+                        superproject_configuration: true,
+                    },
+                )],
+            ),
         ] {
             let repo = repo(name)?;
             for (sm, (name, expected)) in repo.submodules()?.expect("modules present").zip(expected) {
@@ -81,6 +93,16 @@ mod open {
         Ok(())
     }
 
+    #[test]
+    fn not_a_submodule() -> crate::Result {
+        let repo = repo("not-a-submodule")?;
+        let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
+        assert!(sm.open()?.is_some(), "repo available as it was cloned");
+        assert!(sm.index_id()?.is_none(), "no actual submodule");
+        assert!(sm.head_id()?.is_none(), "no actual submodule");
+        Ok(())
+    }
+
     #[test]
     fn old_form() -> crate::Result {
         for name in ["old-form-invalid-worktree-path", "old-form"] {