submodule: Fixed capital error when handling the submodule's branch, which was returned in the submodules super repository, not in the submodule's module

Author: Byron

Diff for: lib/git/objects/submodule/base.py

@@ -51,41 +51,41 @@ class Submodule(util.IndexObject, Iterable, Traversable):
 	# this is a bogus type for base class compatability
 	type = 'submodule'
 
-	__slots__ = ('_parent_commit', '_url', '_branch', '_name', '__weakref__')
-	_cache_attrs = ('path', '_url', '_branch')
+	__slots__ = ('_parent_commit', '_url', '_branch_path', '_name', '__weakref__')
+	_cache_attrs = ('path', '_url', '_branch_path')
 
-	def __init__(self, repo, binsha, mode=None, path=None, name = None, parent_commit=None, url=None, branch=None):
+	def __init__(self, repo, binsha, mode=None, path=None, name = None, parent_commit=None, url=None, branch_path=None):
 		"""Initialize this instance with its attributes. We only document the ones 
 		that differ from ``IndexObject``
 		
 		:param repo: Our parent repository
 		:param binsha: binary sha referring to a commit in the remote repository, see url parameter
 		:param parent_commit: see set_parent_commit()
 		:param url: The url to the remote repository which is the submodule
-		:param branch: Head instance to checkout when cloning the remote repository"""
+		:param branch_path: full (relative) path to ref to checkout when cloning the remote repository"""
 		super(Submodule, self).__init__(repo, binsha, mode, path)
 		self.size = 0
 		if parent_commit is not None:
 			self._parent_commit = parent_commit
 		if url is not None:
 			self._url = url
-		if branch is not None:
-			assert isinstance(branch, git.Head)
-			self._branch = branch
+		if branch_path is not None:
+			assert isinstance(branch_path, basestring)
+			self._branch_path = branch_path
 		if name is not None:
 			self._name = name
 
 	def _set_cache_(self, attr):
 		if attr == '_parent_commit':
 			# set a default value, which is the root tree of the current head
 			self._parent_commit = self.repo.commit()
-		elif attr in ('path', '_url', '_branch'):
+		elif attr in ('path', '_url', '_branch_path'):
 			reader = self.config_reader()
 			# default submodule values
 			self.path = reader.get_value('path')
 			self._url = reader.get_value('url')
 			# git-python extension values - optional
-			self._branch = mkhead(self.repo, reader.get_value(self.k_head_option, self.k_head_default))
+			self._branch_path = reader.get_value(self.k_head_option, git.Head.to_full_path(self.k_head_default))
 		elif attr == '_name':
 			raise AttributeError("Cannot retrieve the name of a submodule if it was not set initially")
 		else:
@@ -119,7 +119,7 @@ def __str__(self):
 		return self._name
 
 	def __repr__(self):
-		return "git.%s(name=%s, path=%s, url=%s, branch=%s)" % (type(self).__name__, self._name, self.path, self.url, self.branch) 
+		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):
@@ -226,7 +226,7 @@ def add(cls, repo, name, path, url=None, branch=None, no_checkout=False):
 			# END handle exceptions
 		# END handle existing
 
-		br = mkhead(repo, branch or cls.k_head_default)
+		br = git.Head.to_full_path(str(branch) or cls.k_head_default)
 		has_module = sm.module_exists()
 		branch_is_default = branch is None
 		if has_module and url is not None:
@@ -250,7 +250,7 @@ def add(cls, repo, name, path, url=None, branch=None, no_checkout=False):
 			# clone new repo
 			kwargs = {'n' : no_checkout}
 			if not branch_is_default:
-				kwargs['b'] = str(br)
+				kwargs['b'] = br
 			# END setup checkout-branch
 			mrepo = git.Repo.clone_from(url, path, **kwargs)
 		# END verify url
@@ -264,8 +264,8 @@ def add(cls, repo, name, path, url=None, branch=None, no_checkout=False):
 		sm._url = url
 		if not branch_is_default:
 			# store full path
-			writer.set_value(cls.k_head_option, br.path)
-			sm._branch = br.path
+			writer.set_value(cls.k_head_option, br)
+			sm._branch_path = br
 		# END handle path
 		del(writer)
 
@@ -327,13 +327,8 @@ def update(self, recursive=False, init=True, to_latest_revision=False):
 			# see whether we have a valid branch to checkout
 			try:
 				# find  a remote which has our branch - we try to be flexible
-				remote_branch = find_first_remote_branch(mrepo.remotes, self.branch)
-				local_branch = self.branch
-				if not local_branch.is_valid():
-					# Setup a tracking configuration - branch doesn't need to 
-					# exist to do that
-					local_branch.set_tracking_branch(remote_branch)
-				#END handle local branch
+				remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name)
+				local_branch = mkhead(mrepo, self.branch_path)
 
 				# have a valid branch, but no checkout - make sure we can figure
 				# that out by marking the commit with a null_sha
@@ -349,8 +344,9 @@ def update(self, recursive=False, init=True, to_latest_revision=False):
 
 				# make sure HEAD is not detached
 				mrepo.head.ref = local_branch
+				mrepo.head.ref.set_tracking_branch(remote_branch)
 			except IndexError:
-				print >> sys.stderr, "Warning: Failed to checkout tracking branch %s" % self.branch 
+				print >> sys.stderr, "Warning: Failed to checkout tracking branch %s" % self.branch_path 
 			#END handle tracking branch
 
 			# NOTE: Have to write the repo config file as well, otherwise
@@ -516,8 +512,8 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
 		:param module: If True, the module we point to will be deleted 
 			as well. If the module is currently on a commit which is not part 
 			of any branch in the remote, if the currently checked out branch 
-			is ahead of its tracking branch,  if you have modifications in the
 			working tree, or untracked files,
+			is ahead of its tracking branch,  if you have modifications in the
 			In case the removal of the repository fails for these reasons, the 
 			submodule status will not have been altered.
 			If this submodule has child-modules on its own, these will be deleted
@@ -611,6 +607,9 @@ def remove(self, module=True, force=False, configuration=True, dry_run=False):
 			self.repo.config_writer().remove_section(sm_section(self.name))
 			self.config_writer().remove_section()
 		# END delete configuration
+
+		# void our data not to delay invalid access
+		self._clear_cache()
 
 		return self
 
@@ -732,8 +731,22 @@ def exists(self):
 
 	@property
 	def branch(self):
-		""":return: The branch instance that we are to checkout"""
-		return self._branch
+		""":return: The branch instance that we are to checkout
+		:raise InvalidGitRepositoryError: if our module is not yet checked out"""
+		return mkhead(self.module(), self._branch_path)
+	
+	@property
+	def branch_path(self):
+		""":return: full (relative) path as string to the branch we would checkout
+			from the remote and track"""
+		return self._branch_path
+		
+	@property
+	def branch_name(self):
+		""":return: the name of the branch, which is the shortest possible branch name"""
+		# use an instance method, for this we create a temporary Head instance
+		# which uses a repository that is available at least ( it makes no difference )
+		return git.Head(self.repo, self._branch_path).name
 
 	@property
 	def url(self):
@@ -814,7 +827,7 @@ def iter_items(cls, repo, parent_commit='HEAD'):
 			# fill in remaining info - saves time as it doesn't have to be parsed again
 			sm._name = n
 			sm._parent_commit = pc
-			sm._branch = mkhead(repo, b)
+			sm._branch_path = git.Head.to_full_path(b)
 			sm._url = u
 
 			yield sm

Diff for: lib/git/objects/submodule/root.py

@@ -1,6 +1,5 @@
 from base import Submodule
 from util import (
-					mkhead, 
 					find_first_remote_branch
 				)
 from git.exc import InvalidGitRepositoryError
@@ -29,7 +28,7 @@ def __init__(self, repo):
 										name = self.k_root_name, 
 										parent_commit = repo.head.commit,
 										url = '',
-										branch = mkhead(repo, self.k_head_default)
+										branch_path = git.Head.to_full_path(self.k_head_default)
 										)
 
 
@@ -135,8 +134,8 @@ def update(self, previous_commit=None, recursive=True, force_remove=False, init=
 
 						# 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))
+						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
@@ -181,7 +180,7 @@ def update(self, previous_commit=None, recursive=True, force_remove=False, init=
 						# tracking branch.
 						smsha = sm.binsha
 						found = False
-						rref = smr.refs[self.branch.name]
+						rref = smr.refs[self.branch_name]
 						for c in rref.commit.traverse():
 							if c.binsha == smsha:
 								found = True
@@ -203,28 +202,28 @@ def update(self, previous_commit=None, recursive=True, force_remove=False, init=
 					# END skip remote handling if new url already exists in module
 				# END handle url
 
-				if sm.branch != psm.branch:
+				if sm.branch_path != psm.branch_path:
 					# finally, create a new tracking branch which tracks the 
 					# new remote branch
 					smm = sm.module()
 					smmr = smm.remotes
 					try:
-						tbr = git.Head.create(smm, sm.branch.name)
+						tbr = git.Head.create(smm, sm.branch_name)
 					except git.GitCommandError, e:
 						if e.status != 128:
 							raise
 						#END handle something unexpected
 
 						# ... or reuse the existing one
-						tbr = git.Head(smm, git.Head.to_full_path(sm.branch.name))
+						tbr = git.Head(smm, sm.branch_path)
 					#END assure tracking branch exists
 
-					tbr.set_tracking_branch(find_first_remote_branch(smmr, sm.branch))
+					tbr.set_tracking_branch(find_first_remote_branch(smmr, sm.branch_name))
 					# figure out whether the previous tracking branch contains
 					# new commits compared to the other one, if not we can 
 					# delete it.
 					try:
-						tbr = find_first_remote_branch(smmr, psm.branch)
+						tbr = find_first_remote_branch(smmr, psm.branch_name)
 						if len(smm.git.cherry(tbr, psm.branch)) == 0:
 							psm.branch.delete(smm, psm.branch)
 						#END delete original tracking branch if there are no changes

Diff for: lib/git/objects/submodule/util.py

@@ -34,16 +34,16 @@ def wrapper(self, *args, **kwargs):
 	wrapper.__name__ = func.__name__
 	return wrapper
 
-def find_first_remote_branch(remotes, branch):
+def find_first_remote_branch(remotes, branch_name):
 	"""Find the remote branch matching the name of the given branch or raise InvalidGitRepositoryError"""
 	for remote in remotes:
 		try:
-			return remote.refs[branch.name]
+			return remote.refs[branch_name]
 		except IndexError:
 			continue
 		# END exception handling
 	#END for remote
-	raise InvalidGitRepositoryError("Didn't find remote branch %r in any of the given remotes", branch)
+	raise InvalidGitRepositoryError("Didn't find remote branch %r in any of the given remotes", branch_name)
 
 #} END utilities
 

Diff for: test/git/test_submodule.py

@@ -35,10 +35,19 @@ def _do_base_tests(self, rwrepo):
 		assert sm.path == 'lib/git/ext/gitdb'
 		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.branch_path == 'refs/heads/master'			# the default ...
+		assert sm.branch_name == 'master'
 		assert sm.parent_commit == rwrepo.head.commit
 		# size is always 0
 		assert sm.size == 0
+		# the module is not checked-out yet
+		self.failUnlessRaises(InvalidGitRepositoryError, sm.module)
+		
+		# which is why we can't get the branch either - it points into the module() repository
+		self.failUnlessRaises(InvalidGitRepositoryError, getattr, sm, 'branch')
+		
+		# branch_path works, as its just a string
+		assert isinstance(sm.branch_path, basestring)
 
 		# some commits earlier we still have a submodule, but its at a different commit
 		smold = Submodule.iter_items(rwrepo, self.k_subm_changed).next()
@@ -455,7 +464,7 @@ def test_root_module(self, rwrepo):
 		nsmm = nsm.module()
 		prev_commit = nsmm.head.commit
 		for branch in ("some_virtual_branch", cur_branch.name):
-			nsm.config_writer().set_value(Submodule.k_head_option, branch)
+			nsm.config_writer().set_value(Submodule.k_head_option, git.Head.to_full_path(branch))
 			csmbranchchange = rwrepo.index.commit("changed branch to %s" % branch)
 			nsm.set_parent_commit(csmbranchchange)
 		# END for each branch to change