test + attempt to workaround Submodule str/repr crash

ryan-williams · ryan-williams · commit eac6825acb0e · 2020-11-27T10:50:31.000-05:00
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -114,11 +114,10 @@ def __init__(self, repo, binsha, mode=None, path=None, name=None, parent_commit=
         if branch_path is not None:
             assert isinstance(branch_path, str)
             self._branch_path = branch_path
-        if name is not None:
-            self._name = name
+        self._name = name
 
     def _set_cache_(self, attr):
-        if attr in ('path', '_url', '_branch_path', '_name'):
+        if attr in ('path', '_url', '_branch_path'):
             reader = self.config_reader()
             # default submodule values
             try:
@@ -134,12 +133,6 @@ def _set_cache_(self, attr):
             self._url = reader.get('url')
             # git-python extension values - optional
             self._branch_path = reader.get_value(self.k_head_option, git.Head.to_full_path(self.k_head_default))
-            section_name = reader._section_name
-            m = re.match(SM_SECTION_NAME_REGEX, section_name)
-            if not m:
-                raise RuntimeError('Unexpected submodule section name in %s: %s' % (reader.file_or_files, section_name))
-            name = m['name']
-            self._name = name
         else:
             super(Submodule, self)._set_cache_(attr)
         # END handle attribute name
@@ -172,11 +165,11 @@ def __hash__(self):
         return hash(self._name)
 
     def __str__(self):
-        return self._name
+        return self.name
 
     def __repr__(self):
-        return "git.%s(name=%s, path=%s, url=%s, branch_path=%s)"\
-               % (type(self).__name__, self._name, self.path, self.url, self.branch_path)
+        return "git.%s(name=%s, path=%s, url=%s, branch_path=%s)" \
+               % (type(self).__name__, self.name, self.path, self.url, self.branch_path)
 
     @classmethod
     def _config_parser(cls, repo, parent_commit, read_only):
@@ -228,6 +221,51 @@ def _sio_modules(cls, parent_commit):
         sio.name = cls.k_modules_file
         return sio
 
+    def _lookup_name_by_config_section_path(self):
+        try:
+            pc = self.parent_commit
+        except ValueError:
+            pc = None
+
+        parser = self._config_parser(self.repo, pc, read_only=True)
+
+        name = None
+        config_file = parser._file_or_files
+        parser.read()
+        parser_sections = parser._sections
+        section_items = list(parser_sections.items())
+        for section_name, sections in section_items:
+            m = re.match(SM_SECTION_NAME_REGEX, section_name)
+            if not m:
+                raise RuntimeError('Unrecognized submodule section line: %s' % section_name)
+            cur_name = m['name']
+            if self.path == sections['path']:
+                # if [section['path'] == self.path for section in sections]:
+                if name is None:
+                    name = cur_name
+                else:
+                    raise AttributeError(
+                        'Submodule name not set, and multiple sections matching path (%s) found in config file %s (commit: %s): %s' % (
+                            self.path,
+                            config_file,
+                            pc or '???',
+                            ','.join([name, cur_name]),
+                        )
+                    )
+            else:
+                print('path %s not found in %d sections named %s' % (self.path, len(sections), section_name))
+        if name is None:
+            raise AttributeError(
+                'Submodule name not set, and no section matching path (%s) found in config file %s (commit: %s) with sections %s' % (
+                    self.path,
+                    config_file,
+                    pc.hexsha or '???',
+                    ','.join([k for k, _ in section_items])
+                )
+            )
+
+        self._name = name
+
     def _config_parser_constrained(self, read_only):
         """:return: Config Parser constrained to our submodule in read or write mode"""
         try:
@@ -1162,6 +1200,8 @@ def name(self):
             used for remotes, which allows to change the path of the submodule
             easily
         """
+        if self._name is None:
+            self._lookup_name_by_config_section_path()
         return self._name
 
     def config_reader(self):
diff --git a/test/test_submodule.py b/test/test_submodule.py
@@ -2,6 +2,7 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 import os
+import re
 import shutil
 import sys
 from unittest import skipIf
@@ -967,6 +968,9 @@ def test_submodule_name(self, rwdir):
         tree = parent.tree()
         sm_tree = tree['mymodules/myname']
         assert sm_tree.hexsha == 'f4d7a58b4d96200cd057a38a0758d3c84901f57e'
+        assert str(sm_tree) == 'test_module'
+        rpr = repr(sm_tree)
+        assert re.match('git.Submodule\(name=test_module, path=mymodules/myname, url=.*/git/ext/gitdb/gitdb/ext/smmap, branch_path=refs/heads/master\)', rpr)
 
         sm_repo.git.checkout('v3.0.4')
         parent.git.commit('-a','-m','update test submodule to v3.0.4')
