Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Library repository tag checkout fails when .gitmodules file is missing #164

Open
2 tasks done
per1234 opened this issue Jan 11, 2023 · 0 comments
Open
2 tasks done
Assignees
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project

Comments

@per1234
Copy link
Contributor

per1234 commented Jan 11, 2023

Describe the problem

A Git submodule has two components:

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

  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"]:
-        # [email protected] 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
+
+    # [email protected] 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")
+
+    # [email protected] 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
@per1234 per1234 added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Jan 11, 2023
@per1234 per1234 self-assigned this Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

1 participant