Merge pull request #2205 from gitpython-developers/fixup-2202

Handle invalid submodule names during recursive updates

Author: Byron

.basedpyright/baseline.json

@@ -491,14 +491,6 @@
                     "lineCount": 1
                 }
             },
-            {
-                "code": "reportPossiblyUnboundVariable",
-                "range": {
-                    "startColumn": 15,
-                    "endColumn": 20,
-                    "lineCount": 1
-                }
-            },
             {
                 "code": "reportArgumentType",
                 "range": {

git/objects/submodule/base.py

@@ -316,8 +316,9 @@ def _validated_name(cls, name: str) -> str:
 
     @classmethod
     def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> PathLike:
+        name = cls._validated_name(name)
         if cls._need_gitfile_submodules(parent_repo.git):
-            return osp.join(parent_repo.git_dir, "modules", cls._validated_name(name))
+            return osp.join(parent_repo.git_dir, "modules", name)
         if parent_repo.working_tree_dir:
             return osp.join(parent_repo.working_tree_dir, path)
         raise NotADirectoryError()
@@ -747,9 +748,7 @@ def update(
             prefix = "DRY-RUN: "
         # END handle prefix
 
-        # To keep things plausible in dry-run mode.
-        if dry_run:
-            mrepo = None
+        mrepo = None
         # END init mrepo
 
         def fetch_remotes(module_repo: "Repo") -> None:

test/test_submodule.py

@@ -971,6 +971,10 @@ def test_update_rejects_parent_component_in_name(self, rwdir):
             with pytest.raises(ValueError, match="submodule name"):
                 Submodule._module_abspath(clone, "module", name)
 
+        with mock.patch.object(Submodule, "_need_gitfile_submodules", return_value=False):
+            with pytest.raises(ValueError, match="submodule name"):
+                Submodule._module_abspath(clone, "module", "../module")
+
     @with_rw_directory
     @_patch_git_config("protocol.file.allow", "always")
     def test_root_update_keeps_going_after_invalid_submodule_name(self, rwdir):
@@ -994,6 +998,8 @@ def test_root_update_keeps_going_after_invalid_submodule_name(self, rwdir):
         assert not clone.submodule("../invalid").module_exists()
         assert clone.submodule("valid").module_exists()
 
+        clone.submodule("../invalid").update(recursive=True, keep_going=True)
+
     @with_rw_directory
     @_patch_git_config("protocol.file.allow", "always")
     def test_list_only_valid_submodules(self, rwdir):