Made sure dry-run is properly implemented in Submodule.remove()

Byron · Byron · commit ae2b59625e9b · 2015-01-17T16:30:56.000+01:00
The root-submodule test is still failing though, this time even earlier
than before
diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
@@ -672,7 +672,7 @@ def move(self, module_path, configuration=True, module=True):
         return self
 
     @unbare_repo
-    def remove(self, module=True, force=False, configuration=True, dry_run=False):
+    def remove(self, module=True, force=False, configuration=True, dry_run=False, _is_recursive=False):
         """Remove this submodule from the repository. This will remove our entry
         from the .gitmodules file and the entry in the .git/config file.
 
@@ -705,7 +705,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
 
         # Recursively remove children of this submodule
         for csm in self.children():
-            csm.remove(module, force, configuration, dry_run)
+            csm.remove(module, force, configuration, dry_run, _is_recursive=True)
             del(csm)
         # end 
 
@@ -772,7 +772,7 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
                 # END delete tree if possible
             # END handle force
 
-            if os.path.isdir(git_dir):
+            if not dry_run and os.path.isdir(git_dir):
                 rmtree(git_dir)
             # end handle separate bare repository
         # END handle module deletion
@@ -781,22 +781,28 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
         ######################
         if configuration and not dry_run:
             # first the index-entry
-            index = self.repo.index
+            parent_index = self.repo.index
             try:
-                del(index.entries[index.entry_key(self.path, 0)])
+                del(parent_index.entries[parent_index.entry_key(self.path, 0)])
             except KeyError:
                 pass
             # END delete entry
-            index.write()
+            parent_index.write()
 
             # now git config - need the config intact, otherwise we can't query
-            # inforamtion anymore
+            # information anymore
             writer = self.repo.config_writer()
             writer.remove_section(sm_section(self.name))
             writer.release()
             writer = self.config_writer()
             writer.remove_section()
             writer.release()
+
+            # Assure we don't leave the parent repository in a dirty state, and commit our changes
+            # It's important for recursive, unforced, deletions to work as expected
+            if _is_recursive:
+                self.module().index.commit("Removed submodule '%s'" % self.name)
+            # end
         # END delete configuration
 
         # void our data not to delay invalid access
diff --git a/git/test/test_submodule.py b/git/test/test_submodule.py
@@ -666,9 +666,12 @@ def test_git_submodule_compatibility(self, rwdir):
 
         # remove
         sm_module_path = sm.module().git_dir
-        sm.remove()
-        assert not sm.exists()
-        assert not sm.module_exists()
-        assert not os.path.isdir(sm_module_path)
+
+        for dry_run in (True, False):
+            sm.remove(dry_run=dry_run)
+            assert sm.exists() == dry_run
+            assert sm.module_exists() == dry_run
+            assert os.path.isdir(sm_module_path) == dry_run
+        # end for each dry-run mode
 
 
