Updated tests and implementation to verify functionality for handling submodule removals, as well as url changes

3 files changed, 111 insertions, 20 deletions

diff --git a/lib/git/objects/submodule.py b/lib/git/objects/submodule.py
index 9fb8ce8f..948a267f 100644
--- a/lib/git/objects/submodule.py
+++ b/lib/git/objects/submodule.py
@@ -958,7 +958,6 @@ class RootModule(Submodule):
 		# END handle previous commit
 		
 		
-		# HANDLE REMOVALS
 		psms = self.list_items(repo, parent_commit=previous_commit)
 		sms = self.list_items(self.module())
 		spsms = set(psms)
@@ -974,7 +973,9 @@ class RootModule(Submodule):
 			rsm.remove(configuration=False, module=True, force=force_remove)
 		# END for each removed submodule
 		
-		# HANDLE PATH RENAMES + url changes + branch changes
+		# HANDLE PATH RENAMES
+		#####################
+		# url changes + branch changes
 		for csm in (spsms & ssms):
 			psm = psms[csm.name]
 			sm = sms[csm.name]
@@ -996,35 +997,79 @@ class RootModule(Submodule):
 					
 					# don't do anything if we already have the url we search in place
 					if len([r for r in rmts if r.url == sm.url]) == 0:
+						
+						
 						assert nn not in [r.name for r in rmts]
 						smr = smm.create_remote(nn, sm.url)
 						smr.fetch()
 						
+						# If we have a tracking branch, it should be available
+						# in the new remote as well.
+						if len([r for r in smr.refs if r.remote_head == sm.branch.name]) == 0:
+							raise ValueError("Submodule branch named %r was not available in new submodule remote at %r" % (sm.branch.name, sm.url))
+						# END head is not detached
+						
 						# now delete the changed one
-						orig_name = None
+						rmt_for_deletion = None
 						for remote in rmts:
 							if remote.url == psm.url:
-								orig_name = remote.name
-								smm.delete_remote(remote)
+								rmt_for_deletion = remote
 								break
 							# END if urls match
 						# END for each remote
 						
 						# if we didn't find a matching remote, but have exactly one, 
 						# we can safely use this one
-						if len(rmts) == 1:
-							orig_name = rmts[0].name
-							smm.delete_remote(rmts[0])
-						else:
-							# if we have not found any remote with the original url
-							# we may not have a name. This is a special case, 
-							# and its okay to fail here
-							# Alternatively we could just generate a unique name
-							raise InvalidGitRepositoryError("Couldn't find original remote-repo at url %r" % psm.url)
-						# END only one remove
+						if rmt_for_deletion is None:
+							if len(rmts) == 1:
+								rmt_for_deletion = rmts[0]
+							else:
+								# if we have not found any remote with the original url
+								# we may not have a name. This is a special case, 
+								# and its okay to fail here
+								# Alternatively we could just generate a unique name and leave all
+								# existing ones in place
+								raise InvalidGitRepositoryError("Couldn't find original remote-repo at url %r" % psm.url)
+							#END handle one single remote
+						# END handle check we found a remote
+						
+						orig_name = rmt_for_deletion.name
+						smm.delete_remote(rmt_for_deletion)
+						# NOTE: Currently we leave tags from the deleted remotes
+						# as well as separate tracking branches in the possibly totally 
+						# changed repository ( someone could have changed the url to 
+						# another project ). At some point, one might want to clean
+						# it up, but the danger is high to remove stuff the user
+						# has added explicitly
 						
 						# rename the new remote back to what it was
 						smr.rename(orig_name)
+						
+						# early on, we verified that the our current tracking branch
+						# exists in the remote. Now we have to assure that the 
+						# sha we point to is still contained in the new remote
+						# tracking branch.
+						smsha = sm.binsha
+						found = False
+						rref = smr.refs[self.branch.name]
+						for c in rref.commit.traverse():
+							if c.binsha == smsha:
+								found = True
+								break
+							# END traverse all commits in search for sha
+						# END for each commit
+						
+						if not found:
+							# adjust our internal binsha to use the one of the remote
+							# this way, it will be checked out in the next step
+							# This will change the submodule relative to us, so 
+							# the user will be able to commit the change easily
+							print >> sys.stderr, "WARNING: Current sha %s was not contained in the tracking branch at the new remote, setting it the the remote's tracking branch" % sm.hexsha
+							sm.binsha = rref.commit.binsha
+						#END reset binsha
+						
+						#NOTE: All checkout is performed by the base implementation of update
+						
 					# END skip remote handling if new url already exists in module
 				# END handle url
 				
@@ -1049,12 +1094,15 @@ class RootModule(Submodule):
 						# current remotes, this just means we can't handle it
 						pass
 					# END exception handling
+					
+					#NOTE: All checkout is done in the base implementation of update
+					
 				#END handle branch
 			#END handle 
 		# END for each common submodule 
 		
 		# FINALLY UPDATE ALL ACTUAL SUBMODULES
-		##########################################
+		######################################
 		for sm in sms:
 			# update the submodule using the default method
 			sm.update(recursive=True, init=init, to_latest_revision=to_latest_revision)
diff --git a/lib/git/remote.py b/lib/git/remote.py
index 5124c603..a06da222 100644
--- a/lib/git/remote.py
+++ b/lib/git/remote.py
@@ -28,6 +28,7 @@ from gitdb.util import join
 
 import re
 import os
+import sys
 
 __all__ = ('RemoteProgress', 'PushInfo', 'FetchInfo', 'Remote')
 
@@ -591,6 +592,10 @@ class Remote(LazyMixin, Iterable):
 		for line in self._digest_process_messages(proc.stderr, progress):
 			if line.startswith('From') or line.startswith('remote: Total'):
 				continue
+			elif line.startswith('warning:'):
+				print >> sys.stderr, line
+				continue
+			# END handle special messages
 			fetch_info_lines.append(line)
 		# END for each line
 		
diff --git a/test/git/test_submodule.py b/test/git/test_submodule.py
index ad09f483..5e209f1b 100644
--- a/test/git/test_submodule.py
+++ b/test/git/test_submodule.py
@@ -358,6 +358,7 @@ class TestSubmodule(TestBase):
 		
 		# modify path without modifying the index entry
 		# ( which is what the move method would do properly )
+		#==================================================
 		sm = rm.children()[0]
 		pp = "path/prefix"
 		fp = join_path_native(pp, sm.path)
@@ -388,26 +389,58 @@ class TestSubmodule(TestBase):
 		rm.update(recursive=False)
 		sm.set_parent_commit(cpathchange)
 		assert sm.module_exists()
-		assert False
 		
 		# add submodule
+		#================
 		nsmn = "newsubmodule"
 		nsmp = "submrepo"
 		nsm = Submodule.add(rwrepo, nsmn, nsmp, url=join_path_native(self.rorepo.working_tree_dir, rsms[0].path, rsms[1].path))
 		csmadded = rwrepo.index.commit("Added submodule")
+		nsm.set_parent_commit(csmadded)
+		assert nsm.module_exists()
+		# in our case, the module should not exist, which happens if we update a parent
+		# repo and a new submodule comes into life
+		nsm.remove(configuration=False, module=True)
+		assert not nsm.module_exists() and nsm.exists()
+		
+		rm.update(recursive=False)
+		assert nsm.module_exists()
+		
+		
 		
 		# remove submodule - the previous one
+		#====================================
 		sm.set_parent_commit(csmadded)
-		assert not sm.remove().exists()
+		smp = sm.abspath
+		assert not sm.remove(module=False).exists()
+		assert os.path.isdir(smp)			# module still exists
 		csmremoved = rwrepo.index.commit("Removed submodule")
 		
-		# change url - to the first repository, this way we have a fast checkout, and a completely different 
+		# an update will remove the module
+		rm.update(recursive=False)
+		assert not os.path.isdir(smp)
+		
+		
+		# change url 
+		#=============
+		# to the first repository, this way we have a fast checkout, and a completely different 
 		# repository at the different url
 		nsm.set_parent_commit(csmremoved)
-		nsm.config_writer().set_value('url', join_path_native(self.rorepo.working_tree_dir, rsms[0].path))
+		nsmurl = join_path_native(self.rorepo.working_tree_dir, rsms[0].path)
+		nsm.config_writer().set_value('url', nsmurl)
 		csmpathchange = rwrepo.index.commit("changed url")
+		nsm.set_parent_commit(csmpathchange)
+		
+		prev_commit = nsm.module().head.commit
+		rm.update(recursive=False)
+		assert nsm.module().remotes.origin.url == nsmurl
+		# head changed, as the remote url and its commit changed
+		assert prev_commit != nsm.module().head.commit
+		
+		assert False
 		
 		# change branch
+		#=================
 		nsm.set_parent_commit(csmpathchange)
 		# the branch used here is an old failure branch which should ideally stay ... lets see how long that works ;)
 		nbn = 'pack_offset_cache'
@@ -415,3 +448,8 @@ class TestSubmodule(TestBase):
 		nsm.config_writer().set_value(Submodule.k_head_option, nbn)
 		csmbranchchange = rwrepo.index.commit("changed branch")
 		
+		
+		# recursive update
+		# =================
+		# finally we recursively update a module, just to run the code at least once
+