Library repository tag checkout fails when .gitmodules file is missing

[per1234]

Describe the problem

A Git submodule has two components:

• An entry in the .gitmodules configuration file
• A submodule object

It turns out it is possible to have a submodule object in a repository even without an accompanying .gitmodules entry. For example:

https://github.com/m5stack/M5AtomS3/tree/eff2afccf4aebd0ff947b058d0206852f41002e0/examples/Basics/FactoryTest/lib

The MahonyAHRS is a submodule, yet there is no .gitmodules file in the repo (which is why no repo is opened when you click on MahonyAHRS in that folder listing).

🐛 If a library repository has this state, the checkout of the tag fails under these conditions with a cryptic error:

Error syncing library: Error checking out repo: failed to get repository to clean state

Even though submodules in libraries are not supported in that the release archive will not contain the submodule contents, the presence of a submodule in the library is not prohibited in the Arduino Library Manager requirements. Normally the presence of a submodule only result in a warning from Arduino Lint in the engine run logs:

WARNING: Git submodule detected. Library Manager installations and installations from GitHub's "Download ZIP" will only 
         contain an empty folder in place of the submodule. (Rule LS004)                                                

To reproduce

• Add a submodule to a library repository.

1. Delete the generated .gitmodules file before committing the submodule object.
2. Create a compliant tag in the library repository.
3. Run libraries-repository-engine using a registry that contains the library repository.

🐛 The tag is not indexed.

---

Integration test coverage for the bug (currently fails):

diff --git a/tests/test_sync.py b/tests/test_sync.py
index 0fa7d5f..a8c6531 100644
--- a/tests/test_sync.py
+++ b/tests/test_sync.py
@@ -273,9 +273,22 @@ def test_clean_checkout(configuration, run_command):
     with pathlib.Path(configuration.data["LibrariesIndex"]).open(mode="r", encoding="utf-8") as library_index_file:
         library_index = json.load(fp=library_index_file)
 
-    for release in library_index["libraries"]:
-        # ssd1306@1.0.0 contains a .exe and so should fail validation.
-        assert not (release["name"] == "ssd1306" and release["version"] == "1.0.0")
+    def index_has_release(library_name, version):
+        for release in library_index["libraries"]:
+            if release["name"] == library_name and release["version"] == version:
+                return True
+
+        return False
+
+    # ssd1306@1.0.0 contains a .exe and so should fail validation.
+    # Validation could pass if the .exe file was missing due to the repo being in a dirty state after checkout:
+    # https://github.com/arduino/libraries-repository-engine/commit/356139bcba9e69cbcfee6d1db4b985e04a589943
+    assert not index_has_release(library_name="ssd1306", version="1.0.0")
+
+    # M5AtomS3@0.0.1 is compliant and so should be added to the index.
+    # This release was not indexed due to the failure to achieve a clean repository after checkout of the 0.0.1 tag:
+    # https://github.com/arduino/library-registry/pull/2352#issuecomment-1378119654
+    assert index_has_release(library_name="M5AtomS3", version="0.0.1")
 
 
 # A Git "thin pack" may reference objects outside the pack.
diff --git a/tests/testdata/test_clean_checkout/repos.txt b/tests/testdata/test_clean_checkout/repos.txt
index 722e1a0..1ca97db 100644
--- a/tests/testdata/test_clean_checkout/repos.txt
+++ b/tests/testdata/test_clean_checkout/repos.txt
@@ -1 +1,2 @@
 https://github.com/lexus2k/ssd1306.git|Contributed|ssd1306
+https://github.com/m5stack/M5AtomS3.git|Contributed|M5AtomS3

ⓘ This was a quick test I threw together with the idea it would help with the investigation. Since the specific conditions the bug occurs under are known and simple to produce, it would be better to use a Go-based test and a generated test data repository to avoid the fragility that results from depending on an external resource (https://github.com/m5stack/M5AtomS3) as test data.

Expected behavior

Library releases that contain a submodule without .gitmodules entry are treated the same as any other release with a submodule:

• Added to the index if found compliant with the requirements.
• Archive contains an empty folder in place of the submodule.

- OR -

If it is not possible to work with a repository in this condition:

• Error message shown in the logs clearly communicates the problem.
• Arduino Lint has a rule for this problem.

Additional context

After finding that github.com/go-git/go-git/v5#Worktree.Status shows the Deleted github.com/go-git/go-git/v5.StatusCode for examples/Basics/FactoryTest/lib/MahonyAHRS, I assumed that the post checkout repository cleaning code was causing the deletion of the submodule object from the repository. However, this is not the case. examples/Basics/FactoryTest/lib/MahonyAHRS is still present even though go-git reports it as being deleted. It reports it as deleted even immediately after the checkout and before any "cleaning" is done on the repository.

---

I tested with the latest version (go-git/go-git@5dabd83) of github.com/go-git/go-git and the bug is still present.

---

I expected I could use github.com/go-git/go-git/v5.Worktree.Submodules to detect submodules comprehensively. Unfortunately I found it does not detect the submodule when the .gitmodules file is missing.

This makes it more challenging to accomplish the second alternative mentioned under the "Expected behavior" section.

---

Originally reported by @Tinyu-Zhao at arduino/library-registry#2352 (comment)

Issue checklist

• I searched for previous reports in the issue tracker
• My report contains all necessary details