repo: renamed directories to more descriptive identifiers and made them safer to use in case of bare repositories

14 files changed, 112 insertions, 63 deletions

diff --git a/CHANGES b/CHANGES
index 4ee1a0e7..5d677b06 100644
--- a/CHANGES
+++ b/CHANGES
@@ -57,6 +57,7 @@ GitCommand
 * git.subcommand call scheme now prunes out None from the argument list, allowing 
   to be called more confortably as None can never be a valid to the git command 
   if converted to a string.
+* Renamed 'git_dir' attribute to 'working_dir' which is exactly how it is used
 
 Commit
 ------
@@ -135,6 +136,11 @@ Repo
   - 'config_reader' method
   - 'config_writer' method
   - 'bare' property, previously it was a simple attribute that could be written
+* Renamed the following attributes
+  - 'path' is now 'git_dir'
+  - 'wd' is now 'working_dir'
+* Added attribute
+  - 'working_tree_dir' which may be None in case of bare repositories
   
 Remote
 ------
diff --git a/lib/git/cmd.py b/lib/git/cmd.py
index b97381f4..9095eb34 100644
--- a/lib/git/cmd.py
+++ b/lib/git/cmd.py
@@ -43,6 +43,8 @@ class Git(object):
 		of the command to stdout.
 		Set its value to 'full' to see details about the returned values.
 	"""
+	__slots__ = ("_working_dir", "cat_file_all", "cat_file_header")
+	
 	class AutoInterrupt(object):
 		"""
 		Kill/Interrupt the stored process instance once this instance goes out of scope. It is 
@@ -92,18 +94,18 @@ class Git(object):
 			
 	
 	
-	def __init__(self, git_dir=None):
+	def __init__(self, working_dir=None):
 		"""
 		Initialize this instance with:
 		
-		``git_dir``
+		``working_dir``
 		   Git directory we should work in. If None, we always work in the current 
 		   directory as returned by os.getcwd().
 		   It is meant to be the working tree directory if available, or the 
 		   .git directory in case of bare repositories.
 		"""
 		super(Git, self).__init__()
-		self.git_dir = git_dir
+		self._working_dir = working_dir
 		
 		# cached command slots
 		self.cat_file_header = None
@@ -121,12 +123,12 @@ class Git(object):
 		return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
 
 	@property
-	def get_dir(self):
+	def working_dir(self):
 		"""
 		Returns
 			Git directory we are working on
 		"""
-		return self.git_dir
+		return self._working_dir
 
 	def execute(self, command,
 				istream=None,
@@ -150,8 +152,8 @@ class Git(object):
 
 		``with_keep_cwd``
 			Whether to use the current working directory from os.getcwd().
-			GitPython uses get_work_tree() as its working directory by
-			default and get_git_dir() for bare repositories.
+			The cmd otherwise uses its own working_dir that it has been initialized
+			with if possible.
 
 		``with_extended_output``
 			Whether to return a (status, stdout, stderr) tuple.
@@ -198,10 +200,10 @@ class Git(object):
 			print ' '.join(command)
 
 		# Allow the user to have the command executed in their working dir.
-		if with_keep_cwd or self.git_dir is None:
+		if with_keep_cwd or self._working_dir is None:
 		  cwd = os.getcwd()
 		else:
-		  cwd=self.git_dir
+		  cwd=self._working_dir
 		  
 		# Start the process
 		proc = subprocess.Popen(command,
diff --git a/lib/git/index.py b/lib/git/index.py
index 9effd3cd..3c895a78 100644
--- a/lib/git/index.py
+++ b/lib/git/index.py
@@ -325,7 +325,7 @@ class IndexFile(LazyMixin, diff.Diffable):
 			super(IndexFile, self)._set_cache_(attr)
 	
 	def _index_path(self):
-		return join_path_native(self.repo.path, "index")
+		return join_path_native(self.repo.git_dir, "index")
 	
 	
 	@property
@@ -539,7 +539,7 @@ class IndexFile(LazyMixin, diff.Diffable):
 		
 		# tmp file created in git home directory to be sure renaming 
 		# works - /tmp/ dirs could be on another device
-		tmp_index = tempfile.mktemp('','',repo.path)
+		tmp_index = tempfile.mktemp('','',repo.git_dir)
 		arg_list.append("--index-output=%s" % tmp_index)
 		arg_list.extend(treeish)
 		
@@ -547,7 +547,7 @@ class IndexFile(LazyMixin, diff.Diffable):
 		# as it considers existing entries. moving it essentially clears the index.
 		# Unfortunately there is no 'soft' way to do it.
 		# The _TemporaryFileSwap assure the original file get put back
-		index_handler = _TemporaryFileSwap(join_path_native(repo.path, 'index'))
+		index_handler = _TemporaryFileSwap(join_path_native(repo.git_dir, 'index'))
 		try:
 			repo.git.read_tree(*arg_list, **kwargs)
 			index = cls(repo, tmp_index)
@@ -579,7 +579,7 @@ class IndexFile(LazyMixin, diff.Diffable):
 		return ret
 	
 	
-	# UTILITIES 
+	# UTILITIES
 	def _iter_expand_paths(self, paths):
 		"""Expand the directories in list of paths to the corresponding paths accordingly, 
 		
@@ -588,7 +588,7 @@ class IndexFile(LazyMixin, diff.Diffable):
 		times - we respect that and do not prune"""
 		def raise_exc(e):
 			raise e
-		r = self.repo.git.git_dir
+		r = self.repo.working_tree_dir
 		rs = r + '/'
 		for path in paths:
 			abs_path = path
@@ -798,9 +798,9 @@ class IndexFile(LazyMixin, diff.Diffable):
 		"""
 		if not os.path.isabs(path):
 			return path
-		relative_path = path.replace(self.repo.git.git_dir+os.sep, "")
+		relative_path = path.replace(self.repo.working_tree_dir+os.sep, "")
 		if relative_path == path:
-			raise ValueError("Absolute path %r is not in git repository at %r" % (path,self.repo.git.git_dir))
+			raise ValueError("Absolute path %r is not in git repository at %r" % (path,self.repo.working_tree_dir))
 		return relative_path
 	
 	def _preprocess_add_items(self, items):
diff --git a/lib/git/objects/base.py b/lib/git/objects/base.py
index ddd03400..8d6860de 100644
--- a/lib/git/objects/base.py
+++ b/lib/git/objects/base.py
@@ -223,5 +223,5 @@ class IndexObject(Object):
 			
 			The returned path will be native to the system and contains '\' on windows. 
 		"""
-		return join_path_native(self.repo.git.git_dir, self.path)
+		return join_path_native(self.repo.working_tree_dir, self.path)
 		
diff --git a/lib/git/refs.py b/lib/git/refs.py
index cf829cb6..f826691d 100644
--- a/lib/git/refs.py
+++ b/lib/git/refs.py
@@ -52,7 +52,7 @@ class SymbolicReference(object):
 		return self.path	
 	
 	def _get_path(self):
-		return join_path_native(self.repo.path, self.path)
+		return join_path_native(self.repo.git_dir, self.path)
 		
 	@classmethod
 	def _iter_packed_refs(cls, repo):
@@ -60,7 +60,7 @@ class SymbolicReference(object):
 		refs.
 		NOTE: The packed refs file will be kept open as long as we iterate"""
 		try:
-			fp = open(os.path.join(repo.path, 'packed-refs'), 'r')
+			fp = open(os.path.join(repo.git_dir, 'packed-refs'), 'r')
 			for line in fp:
 				line = line.strip()
 				if not line:
@@ -258,7 +258,7 @@ class SymbolicReference(object):
 			Alternatively the symbolic reference to be deleted
 		"""
 		full_ref_path = cls._to_full_path(repo, path)
-		abs_path = os.path.join(repo.path, full_ref_path)
+		abs_path = os.path.join(repo.git_dir, full_ref_path)
 		if os.path.exists(abs_path):
 			os.remove(abs_path)
 			
@@ -271,7 +271,7 @@ class SymbolicReference(object):
 		instead"""
 		full_ref_path = cls._to_full_path(repo, path)
 		
-		abs_ref_path = os.path.join(repo.path, full_ref_path)
+		abs_ref_path = os.path.join(repo.git_dir, full_ref_path)
 		if not force and os.path.isfile(abs_ref_path):
 			raise OSError("Reference at %s does already exist" % full_ref_path)
 		
@@ -401,10 +401,10 @@ class Reference(SymbolicReference, LazyMixin, Iterable):
 		
 		# walk loose refs
 		# Currently we do not follow links 
-		for root, dirs, files in os.walk(join_path_native(repo.path, common_path)):
+		for root, dirs, files in os.walk(join_path_native(repo.git_dir, common_path)):
 			for f in files:
 				abs_path = to_native_path_linux(join_path(root, f))
-				rela_paths.add(abs_path.replace(to_native_path_linux(repo.path) + '/', ""))
+				rela_paths.add(abs_path.replace(to_native_path_linux(repo.git_dir) + '/', ""))
 			# END for each file in root directory
 		# END for each directory to walk
 		
diff --git a/lib/git/remote.py b/lib/git/remote.py
index 5eddfa2d..e1d0d743 100644
--- a/lib/git/remote.py
+++ b/lib/git/remote.py
@@ -575,7 +575,7 @@ class Remote(LazyMixin, Iterable):
 		err_info = stderr.splitlines()[1:]
 		
 		# read head information 
-		fp = open(os.path.join(self.repo.path, 'FETCH_HEAD'),'r')
+		fp = open(os.path.join(self.repo.git_dir, 'FETCH_HEAD'),'r')
 		fetch_head_info = fp.readlines()
 		fp.close()
 		
diff --git a/lib/git/repo.py b/lib/git/repo.py
index 3250230e..333b71de 100644
--- a/lib/git/repo.py
+++ b/lib/git/repo.py
@@ -41,9 +41,19 @@ class Repo(object):
 	Represents a git repository and allows you to query references, 
 	gather commit information, generate diffs, create and clone repositories query
 	the log.
+	
+	The following attributes are worth using:
+	
+	'working_dir' is the working directory of the git command, wich is the working tree 
+	directory if available or the .git directory in case of bare repositories
+	
+	'working_tree_dir' is the working tree directory, but will raise AssertionError
+	if we are a bare repository.
+	
+	'git_dir' is the .git repository directoy, which is always set.
 	"""
 	DAEMON_EXPORT_FILE = 'git-daemon-export-ok'
-	__slots__ = ( "wd", "path", "_bare", "git" )
+	__slots__ = ( "working_dir", "_working_tree_dir", "git_dir", "_bare", "git" )
 	
 	# precompiled regex
 	re_whitespace = re.compile(r'\s+')
@@ -81,26 +91,28 @@ class Repo(object):
 		if not os.path.exists(epath):
 			raise NoSuchPathError(epath)
 
-		self.path = None
+		self.working_dir = None
+		self._working_tree_dir = None
+		self.git_dir = None
 		curpath = epath
 		
 		# walk up the path to find the .git dir
 		while curpath:
 			if is_git_dir(curpath):
-				self.path = curpath
-				self.wd = os.path.dirname(curpath)
+				self.git_dir = curpath
+				self._working_tree_dir = os.path.dirname(curpath)
 				break
 			gitpath = os.path.join(curpath, '.git')
 			if is_git_dir(gitpath):
-				self.path = gitpath
-				self.wd = curpath
+				self.git_dir = gitpath
+				self._working_tree_dir = curpath
 				break
 			curpath, dummy = os.path.split(curpath)
 			if not dummy:
 				break
 		# END while curpath
 		
-		if self.path is None:
+		if self.git_dir is None:
 		   raise InvalidGitRepositoryError(epath)
 
 		self._bare = False
@@ -113,17 +125,19 @@ class Repo(object):
 		# adjust the wd in case we are actually bare - we didn't know that 
 		# in the first place
 		if self._bare:
-			self.wd = self.path
-
-		self.git = Git(self.wd)
+			self._working_tree_dir = None
+		# END working dir handling
+		
+		self.working_dir = self._working_tree_dir or self.git_dir
+		self.git = Git(self.working_dir)
 
 	# Description property
 	def _get_description(self):
-		filename = os.path.join(self.path, 'description')
+		filename = os.path.join(self.git_dir, 'description')
 		return file(filename).read().rstrip()
 
 	def _set_description(self, descr):
-		filename = os.path.join(self.path, 'description')
+		filename = os.path.join(self.git_dir, 'description')
 		file(filename, 'w').write(descr+'\n')
 
 	description = property(_get_description, _set_description,
@@ -131,6 +145,18 @@ class Repo(object):
 	del _get_description
 	del _set_description
 	
+	@property
+	def working_tree_dir(self):
+		"""
+		Returns
+			The working tree directory of our git repository
+			
+		Raises AssertionError
+			If we are a bare repository
+		"""
+		if self._working_tree_dir is None:
+			raise AssertionError( "Repository at %r is bare and does not have a working tree directory" % self.git_dir )
+		return self._working_tree_dir
 	
 	@property
 	def bare(self):
@@ -286,7 +312,7 @@ class Repo(object):
 		elif config_level == "global":
 			return os.path.expanduser("~/.gitconfig")
 		elif config_level == "repository":
-			return "%s/config" % self.path
+			return "%s/config" % self.git_dir
 		
 		raise ValueError( "Invalid configuration level: %r" % config_level )
 			
@@ -413,11 +439,11 @@ class Repo(object):
 		return Commit.iter_items(self, rev, paths, **kwargs)
 
 	def _get_daemon_export(self):
-		filename = os.path.join(self.path, self.DAEMON_EXPORT_FILE)
+		filename = os.path.join(self.git_dir, self.DAEMON_EXPORT_FILE)
 		return os.path.exists(filename)
 
 	def _set_daemon_export(self, value):
-		filename = os.path.join(self.path, self.DAEMON_EXPORT_FILE)
+		filename = os.path.join(self.git_dir, self.DAEMON_EXPORT_FILE)
 		fileexists = os.path.exists(filename)
 		if value and not fileexists:
 			touch(filename)
@@ -436,7 +462,7 @@ class Repo(object):
 		Returns
 			list of strings being pathnames of alternates
 		"""
-		alternates_path = os.path.join(self.path, 'objects', 'info', 'alternates')
+		alternates_path = os.path.join(self.git_dir, 'objects', 'info', 'alternates')
 
 		if os.path.exists(alternates_path):
 			try:
@@ -466,7 +492,7 @@ class Repo(object):
 		Returns
 			None
 		"""
-		alternates_path = os.path.join(self.path, 'objects', 'info', 'alternates') 
+		alternates_path = os.path.join(self.git_dir, 'objects', 'info', 'alternates') 
 		if not alts:
 			if os.path.isfile(alternates_path):
 				os.remove(alternates_path)
@@ -706,7 +732,7 @@ class Repo(object):
 		# END windows handling 
 		
 		try:
-			self.git.clone(self.path, path, **kwargs)
+			self.git.clone(self.git_dir, path, **kwargs)
 		finally:
 			if prev_cwd is not None:
 				os.chdir(prev_cwd)
@@ -754,4 +780,4 @@ class Repo(object):
 		return self
 
 	def __repr__(self):
-		return '<git.Repo "%s">' % self.path
+		return '<git.Repo "%s">' % self.git_dir
diff --git a/lib/git/utils.py b/lib/git/utils.py
index 5deed556..433f96d5 100644
--- a/lib/git/utils.py
+++ b/lib/git/utils.py
@@ -362,3 +362,18 @@ class Iterable(object):
 		"""
 		raise NotImplementedError("To be implemented by Subclass")
 		
+def needs_working_tree(func):
+	"""
+	Decorator assuring the wrapped method may only run if the repository has a
+	working tree, hence it is not bare.
+	"""
+	def check_default_index(self, *args, **kwargs):
+		if self.repo.working_tree_dir is None:
+			raise AssertionError( "Cannot call %r bare git repositories" % func.__name__ )
+		return func(self, *args, **kwargs)
+	# END wrpaper method
+	
+	check_default_index.__name__ = func.__name__
+	return check_default_index
+
+		
diff --git a/test/git/test_base.py b/test/git/test_base.py
index ab46ded4..ec85c2a7 100644
--- a/test/git/test_base.py
+++ b/test/git/test_base.py
@@ -84,15 +84,15 @@ class TestBase(TestBase):
 	@with_bare_rw_repo
 	def test_with_bare_rw_repo(self, bare_rw_repo):
 		assert bare_rw_repo.config_reader("repository").getboolean("core", "bare")
-		assert os.path.isfile(os.path.join(bare_rw_repo.path,'HEAD'))
+		assert os.path.isfile(os.path.join(bare_rw_repo.git_dir,'HEAD'))
 		
 	@with_rw_repo('0.1.6')
 	def test_with_rw_repo(self, rw_repo):
 		assert not rw_repo.config_reader("repository").getboolean("core", "bare")
-		assert os.path.isdir(os.path.join(rw_repo.git.git_dir,'lib'))
+		assert os.path.isdir(os.path.join(rw_repo.working_tree_dir,'lib'))
 		
 	@with_rw_and_rw_remote_repo('0.1.6')
 	def test_with_rw_remote_and_rw_repo(self, rw_repo, rw_remote_repo):
 		assert not rw_repo.config_reader("repository").getboolean("core", "bare")
 		assert rw_remote_repo.config_reader("repository").getboolean("core", "bare")
-		assert os.path.isdir(os.path.join(rw_repo.git.git_dir,'lib'))
+		assert os.path.isdir(os.path.join(rw_repo.working_tree_dir,'lib'))
diff --git a/test/git/test_index.py b/test/git/test_index.py
index 3a7edc7e..459cfc0e 100644
--- a/test/git/test_index.py
+++ b/test/git/test_index.py
@@ -254,7 +254,7 @@ class TestTree(TestBase):
 		
 		# reset the working copy as well to current head,to pull 'back' as well
 		new_data = "will be reverted"
-		file_path = os.path.join(rw_repo.git.git_dir, "CHANGES")
+		file_path = os.path.join(rw_repo.working_tree_dir, "CHANGES")
 		fp = open(file_path, "wb")
 		fp.write(new_data)
 		fp.close()
@@ -269,7 +269,7 @@ class TestTree(TestBase):
 			fp.close()
 			
 		# test full checkout
-		test_file = os.path.join(rw_repo.git.git_dir, "CHANGES")
+		test_file = os.path.join(rw_repo.working_tree_dir, "CHANGES")
 		open(test_file, 'ab').write("some data")
 		rval = index.checkout(None, force=True, fprogress=self._fprogress)
 		assert 'CHANGES' in list(rval)
@@ -312,7 +312,7 @@ class TestTree(TestBase):
 		assert not open(test_file).read().endswith(append_data)
 		
 		# checkout directory
-		shutil.rmtree(os.path.join(rw_repo.git.git_dir, "lib"))
+		shutil.rmtree(os.path.join(rw_repo.working_tree_dir, "lib"))
 		rval = index.checkout('lib')
 		assert len(list(rval)) > 1
 	
@@ -321,7 +321,7 @@ class TestTree(TestBase):
 		Returns count of files that actually exist in the repository directory.
 		"""
 		existing = 0
-		basedir = repo.git.git_dir
+		basedir = repo.working_tree_dir
 		for f in files:
 			existing += os.path.isfile(os.path.join(basedir, f))
 		# END for each deleted file
@@ -375,7 +375,7 @@ class TestTree(TestBase):
 		self.failUnlessRaises(TypeError, index.remove, [1])
 		
 		# absolute path
-		deleted_files = index.remove([os.path.join(rw_repo.git.git_dir,"lib")], r=True)
+		deleted_files = index.remove([os.path.join(rw_repo.working_tree_dir,"lib")], r=True)
 		assert len(deleted_files) > 1
 		self.failUnlessRaises(ValueError, index.remove, ["/doesnt/exists"])
 		
@@ -411,7 +411,7 @@ class TestTree(TestBase):
 		index.reset(new_commit.parents[0], working_tree=True).reset(new_commit, working_tree=False)
 		lib_file_path = "lib/git/__init__.py"
 		assert (lib_file_path, 0) not in index.entries
-		assert os.path.isfile(os.path.join(rw_repo.git.git_dir, lib_file_path))
+		assert os.path.isfile(os.path.join(rw_repo.working_tree_dir, lib_file_path))
 		
 		# directory
 		entries = index.add(['lib'], fprogress=self._fprogress_add)
@@ -452,7 +452,7 @@ class TestTree(TestBase):
 		
 		# add symlink
 		if sys.platform != "win32":
-			link_file = os.path.join(rw_repo.git.git_dir, "my_real_symlink")
+			link_file = os.path.join(rw_repo.working_tree_dir, "my_real_symlink")
 			os.symlink("/etc/that", link_file)
 			entries = index.reset(new_commit).add([link_file], fprogress=self._fprogress_add)
 			self._assert_fprogress(entries)
diff --git a/test/git/test_refs.py b/test/git/test_refs.py
index 55982b23..13297512 100644
--- a/test/git/test_refs.py
+++ b/test/git/test_refs.py
@@ -271,7 +271,7 @@ class TestRefs(TestBase):
 		symbol_ref_path = "refs/symbol_ref"
 		symref = SymbolicReference(rw_repo, symbol_ref_path)
 		assert symref.path == symbol_ref_path
-		symbol_ref_abspath = os.path.join(rw_repo.path, symref.path)
+		symbol_ref_abspath = os.path.join(rw_repo.git_dir, symref.path)
 		
 		# set it
 		symref.reference = new_head
diff --git a/test/git/test_remote.py b/test/git/test_remote.py
index 4849dfd0..edbf758c 100644
--- a/test/git/test_remote.py
+++ b/test/git/test_remote.py
@@ -62,7 +62,7 @@ class TestPushProgress(PushProgress):
 class TestRemote(TestBase):
 	
 	def _print_fetchhead(self, repo):
-		fp = open(os.path.join(repo.path, "FETCH_HEAD"))
+		fp = open(os.path.join(repo.git_dir, "FETCH_HEAD"))
 		fp.close()
 		
 		
@@ -213,7 +213,7 @@ class TestRemote(TestBase):
 		# must clone with a local path for the repo implementation not to freak out
 		# as it wants local paths only ( which I can understand )
 		other_repo = remote_repo.clone(other_repo_dir, shared=False)
-		remote_repo_url = "git://localhost%s"%remote_repo.path
+		remote_repo_url = "git://localhost%s"%remote_repo.git_dir
 		
 		# put origin to git-url
 		other_origin = other_repo.remotes.origin 
diff --git a/test/git/test_repo.py b/test/git/test_repo.py
index e94a45bd..0e913ff1 100644
--- a/test/git/test_repo.py
+++ b/test/git/test_repo.py
@@ -21,11 +21,11 @@ class TestRepo(TestBase):
 		Repo("repos/foobar")
 
 	def test_repo_creation_from_different_paths(self):
-		r_from_gitdir = Repo(self.rorepo.path)
-		assert r_from_gitdir.path == self.rorepo.path
-		assert r_from_gitdir.path.endswith('.git')
-		assert not self.rorepo.git.git_dir.endswith('.git')
-		assert r_from_gitdir.git.git_dir == self.rorepo.git.git_dir
+		r_from_gitdir = Repo(self.rorepo.git_dir)
+		assert r_from_gitdir.git_dir == self.rorepo.git_dir
+		assert r_from_gitdir.git_dir.endswith('.git')
+		assert not self.rorepo.git.working_dir.endswith('.git')
+		assert r_from_gitdir.git.working_dir == self.rorepo.git.working_dir
 
 	def test_description(self):
 		txt = "Test repository"
@@ -225,7 +225,7 @@ class TestRepo(TestBase):
 		assert_true( len( tlist ) < sum( len(t) for t in tlist ) )				 # test for single-char bug
 		
 	def test_untracked_files(self):
-		base = self.rorepo.git.git_dir
+		base = self.rorepo.working_tree_dir
 		files = (	join_path_native(base, "__test_myfile"), 
 					join_path_native(base, "__test_other_file")	)
 		num_recently_untracked = 0
diff --git a/test/testlib/helper.py b/test/testlib/helper.py
index e33961a7..da4a4207 100644
--- a/test/testlib/helper.py
+++ b/test/testlib/helper.py
@@ -241,7 +241,7 @@ class TestBase(TestCase):
 		with the given data. Returns absolute path to created file.
 		"""
 		repo = repo or self.rorepo
-		abs_path = os.path.join(repo.git.git_dir, rela_path)
+		abs_path = os.path.join(repo.working_tree_dir, rela_path)
 		fp = open(abs_path, "w")
 		fp.write(data)
 		fp.close()