test_submodule: fixed failures that arose due to changes of the original submodule names. Also, a major bug was fixed that cased submodules to always being updated recursively when using the RootModule.update method

submodule: previously, it would update the repository configuration during add(), but in fact it must be done during update() when the module is cloned, which is how the git-submodule implementation works

4 files changed, 21 insertions, 18 deletions

diff --git a/lib/git/objects/submodule/base.py b/lib/git/objects/submodule/base.py
index 347af58e..b72eac82 100644
--- a/lib/git/objects/submodule/base.py
+++ b/lib/git/objects/submodule/base.py
@@ -261,12 +261,6 @@ class Submodule(util.IndexObject, Iterable, Traversable):
 		# END handle path
 		del(writer)
 		
-		# NOTE: Have to write the repo config file as well, otherwise
-		# the default implementation will be offended and not update the repository
-		# Maybe this is a good way to assure it doesn't get into our way, but 
-		# we want to stay backwards compatible too ... . Its so redundant !
-		repo.config_writer().set_value(sm_section(sm.name), 'url', url)
-		
 		# we deliberatly assume that our head matches our index !
 		pcommit = repo.head.commit
 		sm._parent_commit = pcommit
@@ -350,6 +344,12 @@ class Submodule(util.IndexObject, Iterable, Traversable):
 			except IndexError:
 				print >> sys.stderr, "Warning: Failed to checkout tracking branch %s" % self.branch 
 			#END handle tracking branch
+			
+			# NOTE: Have to write the repo config file as well, otherwise
+			# the default implementation will be offended and not update the repository
+			# Maybe this is a good way to assure it doesn't get into our way, but 
+			# we want to stay backwards compatible too ... . Its so redundant !
+			self.repo.config_writer().set_value(sm_section(self.name), 'url', self.url)
 		#END handle initalization
 		
 		
diff --git a/lib/git/objects/submodule/root.py b/lib/git/objects/submodule/root.py
index 82b8b271..06649136 100644
--- a/lib/git/objects/submodule/root.py
+++ b/lib/git/objects/submodule/root.py
@@ -244,7 +244,7 @@ class RootModule(Submodule):
 		######################################
 		for sm in sms:
 			# update the submodule using the default method
-			sm.update(recursive=True, init=init, to_latest_revision=to_latest_revision)
+			sm.update(recursive=False, init=init, to_latest_revision=to_latest_revision)
 			
 			# update recursively depth first - question is which inconsitent 
 			# state will be better in case it fails somewhere. Defective branch
diff --git a/test/git/test_repo.py b/test/git/test_repo.py
index a6047bf5..62b4c476 100644
--- a/test/git/test_repo.py
+++ b/test/git/test_repo.py
@@ -560,7 +560,7 @@ class TestRepo(TestBase):
 		assert len(self.rorepo.submodules) == 1		# non-recursive
 		assert len(list(self.rorepo.iter_submodules())) == 2
 		
-		assert isinstance(self.rorepo.submodule("lib/git/ext/gitdb"), Submodule)
+		assert isinstance(self.rorepo.submodule("gitdb"), Submodule)
 		self.failUnlessRaises(ValueError, self.rorepo.submodule, "doesn't exist")
 		
 	@with_rw_repo('HEAD', bare=False)
diff --git a/test/git/test_submodule.py b/test/git/test_submodule.py
index e7807dcd..e2261d65 100644
--- a/test/git/test_submodule.py
+++ b/test/git/test_submodule.py
@@ -12,7 +12,7 @@ import os
 
 class TestSubmodule(TestBase):
 
-	k_subm_current = "00ce31ad308ff4c7ef874d2fa64374f47980c85c"
+	k_subm_current = "45c0f285a6d9d9214f8167742d12af2855f527fb"
 	k_subm_changed = "394ed7006ee5dc8bddfd132b64001d5dfc0ffdd3"
 	k_no_subm_tag = "0.1.6"
 	
@@ -33,7 +33,7 @@ class TestSubmodule(TestBase):
 		assert len(Submodule.list_items(rwrepo, self.k_no_subm_tag)) == 0
 		
 		assert sm.path == 'lib/git/ext/gitdb'
-		assert sm.path == sm.name					# for now, this is True
+		assert sm.path != sm.name					# in our case, we have ids there, which don't equal the path
 		assert sm.url == 'git://gitorious.org/git-python/gitdb.git'
 		assert sm.branch.name == 'master'			# its unset in this case
 		assert sm.parent_commit == rwrepo.head.commit
@@ -43,7 +43,7 @@ class TestSubmodule(TestBase):
 		# some commits earlier we still have a submodule, but its at a different commit
 		smold = Submodule.iter_items(rwrepo, self.k_subm_changed).next()
 		assert smold.binsha != sm.binsha
-		assert smold == sm					# the name is still the same
+		assert smold != sm					# the name changed
 		
 		# force it to reread its information
 		del(smold._url)
@@ -71,12 +71,11 @@ class TestSubmodule(TestBase):
 			self.failUnlessRaises(ValueError, smold.config_writer)
 		# END handle bare repo
 		
-		# make the old into a new
+		# make the old into a new - this doesn't work as the name changed
 		prev_parent_commit = smold.parent_commit
-		assert smold.set_parent_commit(self.k_subm_current) is smold 
-		assert smold.parent_commit != prev_parent_commit
-		assert smold.binsha == sm.binsha
-		smold.set_parent_commit(prev_parent_commit)
+		self.failUnlessRaises(ValueError, smold.set_parent_commit, self.k_subm_current)
+		# the sha is properly updated
+		smold.set_parent_commit(self.k_subm_changed+"~1")
 		assert smold.binsha != sm.binsha
 		
 		# raises if the sm didn't exist in new parent - it keeps its 
@@ -181,6 +180,10 @@ class TestSubmodule(TestBase):
 			csm.module().head.ref.set_tracking_branch(None)
 			sm.update(recursive=True, to_latest_revision=True)
 			
+			# to_latest_revision changes the child submodule's commit, it needs an
+			# update now
+			csm.set_parent_commit(csm.repo.head.commit)
+			
 			# undo the changes
 			sm.module().head.ref = smref
 			csm.module().head.ref.set_tracking_branch(csm_tracking_branch)
@@ -191,8 +194,8 @@ class TestSubmodule(TestBase):
 			self.failUnlessRaises(ValueError, csm.remove, module=False, configuration=False)
 			# We have modified the configuration, hence the index is dirty, and the
 			# deletion will fail
-			# NOTE: As we did  a few updates in the meanwhile, the indices where reset
-			# Hence we restore some changes
+			# NOTE: As we did  a few updates in the meanwhile, the indices were reset
+			# Hence we create some changes
 			sm.config_writer().set_value("somekey", "somevalue")
 			csm.config_writer().set_value("okey", "ovalue")
 			self.failUnlessRaises(InvalidGitRepositoryError, sm.remove)