diff options
| -rw-r--r-- | TODO | 10 | ||||
| -rw-r--r-- | lib/git/cmd.py | 27 | ||||
| -rw-r--r-- | lib/git/config.py | 1 | ||||
| -rw-r--r-- | lib/git/index.py | 24 | ||||
| -rw-r--r-- | lib/git/objects/base.py | 8 | ||||
| -rw-r--r-- | lib/git/objects/commit.py | 8 | ||||
| -rw-r--r-- | lib/git/objects/tree.py | 3 | ||||
| -rw-r--r-- | lib/git/refs.py | 12 | ||||
| -rw-r--r-- | lib/git/repo.py | 33 | ||||
| -rw-r--r-- | lib/git/utils.py | 111 | ||||
| -rw-r--r-- | test/git/test_commit.py | 2 | ||||
| -rw-r--r-- | test/git/test_git.py | 7 | ||||
| -rw-r--r-- | test/git/test_index.py | 18 | ||||
| -rw-r--r-- | test/git/test_remote.py | 8 | ||||
| -rw-r--r-- | test/git/test_repo.py | 12 | ||||
| -rw-r--r-- | test/git/test_utils.py | 4 | ||||
| -rw-r--r-- | test/testlib/helper.py | 32 |
17 files changed, 225 insertions, 95 deletions
@@ -11,9 +11,13 @@ General a sha or ref unless cat-file is used where it must be a sha * Overhaul command caching - currently its possible to create many instances of the std-in command types, as it appears they are not killed when the repo gets - deleted. A clear() method could already help to allow long-running programs - to remove cached commands after an idle time. - + deleted. +* git command on windows may be killed using the /F options which probably is like + SIGKILL. This is bad as the process might be doing something, leaving the resource + locked and/or in an inconsistent state. Without the /F option, git does not terminate + itself, probably it listens to SIGINT which is not sent by TASKKILL. We can't really + help it, but may be at some point git handles windows signals properly. + Object ------ * DataStream method should read the data itself. This would be easy once you have diff --git a/lib/git/cmd.py b/lib/git/cmd.py index fb6f2998..bccfb611 100644 --- a/lib/git/cmd.py +++ b/lib/git/cmd.py @@ -17,8 +17,13 @@ execute_kwargs = ('istream', 'with_keep_cwd', 'with_extended_output', 'output_stream' ) extra = {} -if sys.platform == 'win32': - extra = {'shell': True} +# NOTE: Execution through a shell on windows appears to be slightly faster, but in fact +# I consider it a problem whenever complex strings are passed and *interpreted* +# by the shell beforehand. This can cause great confusion and reduces compatability +# between the OS which is why the shell should not be used ( unless it does not work +# otherwise ) +#if sys.platform == 'win32': +# extra = {'shell': False} def dashify(string): return string.replace('_', '-') @@ -63,7 +68,10 @@ class Git(object): os.kill(self.proc.pid, 2) # interrupt signal except AttributeError: # try windows - subprocess.call(("TASKKILL", "/T", "/PID", self.proc.pid)) + # for some reason, providing None for stdout/stderr still prints something. This is why + # we simply use the shell and redirect to nul. Its slower than CreateProcess, question + # is whether we really want to see all these messages. Its annoying no matter what. + subprocess.call(("TASKKILL /F /T /PID %s 2>nul 1>nul" % str(self.proc.pid)), shell=True) # END exception handling def __getattr__(self, attr): @@ -393,3 +401,16 @@ class Git(object): cmd.stdout.read(1) # finishing newlines return (hexsha, typename, size, data) + + def clear_cache(self): + """ + Clear all kinds of internal caches to release resources. + + Currently persistent commands will be interrupted. + + Returns + self + """ + self.cat_file_all = None + self.cat_file_header = None + return self diff --git a/lib/git/config.py b/lib/git/config.py index ccfbae48..bf4c6469 100644 --- a/lib/git/config.py +++ b/lib/git/config.py @@ -116,6 +116,7 @@ class GitConfigParser(cp.RawConfigParser, LockFile): If True, the ConfigParser may only read the data , but not change it. If False, only a single file path or file object may be given. """ + super(GitConfigParser, self).__init__() # initialize base with ordered dictionaries to be sure we write the same # file back self._sections = OrderedDict() diff --git a/lib/git/index.py b/lib/git/index.py index e368f531..45bf617f 100644 --- a/lib/git/index.py +++ b/lib/git/index.py @@ -19,7 +19,7 @@ import subprocess import git.diff as diff from git.objects import Blob, Tree, Object, Commit -from git.utils import SHA1Writer, LazyMixin, ConcurrentWriteOperation +from git.utils import SHA1Writer, LazyMixin, ConcurrentWriteOperation, join_path_native class _TemporaryFileSwap(object): @@ -36,7 +36,7 @@ class _TemporaryFileSwap(object): def __del__(self): if os.path.isfile(self.tmp_file_path): - if sys.platform == "win32" and os.path.exists(self.file_path): + if os.name == 'nt' and os.path.exists(self.file_path): os.remove(self.file_path) os.rename(self.tmp_file_path, self.file_path) # END temp file exists @@ -243,9 +243,10 @@ class IndexFile(LazyMixin, diff.Diffable): if attr == "entries": # read the current index # try memory map for speed - fp = open(self._file_path, "r") + fp = open(self._file_path, "rb") stream = fp try: + raise Exception() stream = mmap.mmap(fp.fileno(), 0, access=mmap.ACCESS_READ) except Exception: pass @@ -254,13 +255,18 @@ class IndexFile(LazyMixin, diff.Diffable): try: self._read_from_stream(stream) finally: - fp.close() + pass + # make sure we close the stream ( possibly an mmap ) + # and the file + #stream.close() + #if stream is not fp: + # fp.close() # END read from default index on demand else: super(IndexFile, self)._set_cache_(attr) def _index_path(self): - return os.path.join(self.repo.path, "index") + return join_path_native(self.repo.path, "index") @property @@ -440,7 +446,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(os.path.join(repo.path, 'index')) + index_handler = _TemporaryFileSwap(join_path_native(repo.path, 'index')) try: repo.git.read_tree(*arg_list, **kwargs) index = cls(repo, tmp_index) @@ -603,7 +609,7 @@ class IndexFile(LazyMixin, diff.Diffable): """ if not os.path.isabs(path): return path - relative_path = path.replace(self.repo.git.git_dir+"/", "") + relative_path = path.replace(self.repo.git.git_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)) return relative_path @@ -839,10 +845,10 @@ class IndexFile(LazyMixin, diff.Diffable): # create message stream tmp_file_path = tempfile.mktemp() - fp = open(tmp_file_path,"w") + fp = open(tmp_file_path,"wb") fp.write(str(message)) fp.close() - fp = open(tmp_file_path,"r") + fp = open(tmp_file_path,"rb") fp.seek(0) try: diff --git a/lib/git/objects/base.py b/lib/git/objects/base.py index b0989a43..c66263c0 100644 --- a/lib/git/objects/base.py +++ b/lib/git/objects/base.py @@ -4,7 +4,7 @@ # This module is part of GitPython and is released under # the BSD License: http://www.opensource.org/licenses/bsd-license.php import os -from git.utils import LazyMixin +from git.utils import LazyMixin, join_path_native import utils _assertion_msg_format = "Created object %r whose python type %r disagrees with the acutal git object type %r" @@ -209,7 +209,9 @@ class IndexObject(Object): """ Returns Absolute path to this index object in the file system ( as opposed to the - .path field which is a path relative to the git repository ) + .path field which is a path relative to the git repository ). + + The returned path will be native to the system and contains '\' on windows. """ - return os.path.join(self.repo.git.git_dir, self.path) + return join_path_native(self.repo.git.git_dir, self.path) diff --git a/lib/git/objects/commit.py b/lib/git/objects/commit.py index 80b3ad23..4ec806fb 100644 --- a/lib/git/objects/commit.py +++ b/lib/git/objects/commit.py @@ -116,7 +116,13 @@ class Commit(base.Object, Iterable, diff.Diffable): Returns int """ - return len(self.repo.git.rev_list(self.sha, '--', paths, **kwargs).strip().splitlines()) + # yes, it makes a difference whether empty paths are given or not in our case + # as the empty paths version will ignore merge commits for some reason. + if paths: + return len(self.repo.git.rev_list(self.sha, '--', paths, **kwargs).splitlines()) + else: + return len(self.repo.git.rev_list(self.sha, **kwargs).splitlines()) + @property def name_rev(self): diff --git a/lib/git/objects/tree.py b/lib/git/objects/tree.py index 27bd84d0..f88096b3 100644 --- a/lib/git/objects/tree.py +++ b/lib/git/objects/tree.py @@ -9,6 +9,7 @@ import blob import base import binascii import git.diff as diff +from git.utils import join_path def sha_to_hex(sha): """Takes a string and returns the hex of the sha within""" @@ -110,7 +111,7 @@ class Tree(base.IndexObject, diff.Diffable): i += 1 # END while not reached NULL name = data[ns:i] - path = os.path.join(self.path, name) + path = join_path(self.path, name) # byte is NULL, get next 20 i += 1 diff --git a/lib/git/refs.py b/lib/git/refs.py index 5b94ea07..b0996878 100644 --- a/lib/git/refs.py +++ b/lib/git/refs.py @@ -8,7 +8,7 @@ import os from objects import Object, Commit from objects.utils import get_object_type_by_name -from utils import LazyMixin, Iterable +from utils import LazyMixin, Iterable, join_path, join_path_native, to_native_path_linux class Reference(LazyMixin, Iterable): """ @@ -136,15 +136,15 @@ class Reference(LazyMixin, Iterable): # walk loose refs # Currently we do not follow links - for root, dirs, files in os.walk(os.path.join(repo.path, common_path)): + for root, dirs, files in os.walk(join_path_native(repo.path, common_path)): for f in files: - abs_path = os.path.join(root, f) - rela_paths.add(abs_path.replace(repo.path + '/', "")) + abs_path = to_native_path_linux(join_path(root, f)) + rela_paths.add(abs_path.replace(to_native_path_linux(repo.path) + '/', "")) # END for each file in root directory # END for each directory to walk # read packed refs - packed_refs_path = os.path.join(repo.path, 'packed-refs') + packed_refs_path = join_path_native(repo.path, 'packed-refs') if os.path.isfile(packed_refs_path): fp = open(packed_refs_path, 'r') try: @@ -230,7 +230,7 @@ class SymbolicReference(object): return hash(self.name) def _get_path(self): - return os.path.join(self.repo.path, self.name) + return join_path_native(self.repo.path, self.name) def _get_commit(self): """ diff --git a/lib/git/repo.py b/lib/git/repo.py index 6d388633..d39f11d1 100644 --- a/lib/git/repo.py +++ b/lib/git/repo.py @@ -674,7 +674,38 @@ class Repo(object): Returns ``git.Repo`` (the newly cloned repo) """ - self.git.clone(self.path, path, **kwargs) + # special handling for windows for path at which the clone should be + # created. + # tilde '~' will be expanded to the HOME no matter where the ~ occours. Hence + # we at least give a proper error instead of letting git fail + prev_cwd = None + prev_path = None + if os.name == 'nt': + if '~' in path: + raise OSError("Git cannot handle the ~ character in path %r correctly" % path) + + # on windows, git will think paths like c: are relative and prepend the + # current working dir ( before it fails ). We temporarily adjust the working + # dir to make this actually work + match = re.match("(\w:[/\\\])(.*)", path) + if match: + prev_cwd = os.getcwd() + prev_path = path + drive, rest_of_path = match.groups() + os.chdir(drive) + path = rest_of_path + kwargs['with_keep_cwd'] = True + # END cwd preparation + # END windows handling + + try: + self.git.clone(self.path, path, **kwargs) + finally: + if prev_cwd is not None: + os.chdir(prev_cwd) + path = prev_path + # END reset previous working dir + # END bad windows handling return Repo(path) diff --git a/lib/git/utils.py b/lib/git/utils.py index 48427ff2..5deed556 100644 --- a/lib/git/utils.py +++ b/lib/git/utils.py @@ -9,55 +9,88 @@ import sys import tempfile try: - import hashlib + import hashlib except ImportError: - import sha + import sha def make_sha(source=''): - """ - A python2.4 workaround for the sha/hashlib module fiasco - + """ + A python2.4 workaround for the sha/hashlib module fiasco + Note From the dulwich project """ - try: - return hashlib.sha1(source) - except NameError: - sha1 = sha.sha(source) - return sha1 + try: + return hashlib.sha1(source) + except NameError: + sha1 = sha.sha(source) + return sha1 + +def join_path(a, *p): + """Join path tokens together similar to os.path.join, but always use + '/' instead of possibly '\' on windows.""" + path = a + for b in p: + if b.startswith('/'): + path += b[1:] + elif path == '' or path.endswith('/'): + path += b + else: + path += '/' + b + return path + +def to_native_path_windows(path): + return path.replace('/','\\') + +def to_native_path_linux(path): + return path.replace('\\','/') + +if sys.platform.startswith('win'): + to_native_path = to_native_path_windows +else: + # no need for any work on linux + def to_native_path_linux(path): + return path + to_native_path = to_native_path_linux + +def join_path_native(a, *p): + """As join path, but makes sure an OS native path is returned. This is only + needed to play it safe on my dear windows and to assure nice paths that only + use '\'""" + return to_native_path(join_path(a, *p)) class SHA1Writer(object): - """ - Wrapper around a file-like object that remembers the SHA1 of - the data written to it. It will write a sha when the stream is closed - or if the asked for explicitly usign write_sha. - - Note: - Based on the dulwich project - """ - __slots__ = ("f", "sha1") - - def __init__(self, f): - self.f = f - self.sha1 = make_sha("") + """ + Wrapper around a file-like object that remembers the SHA1 of + the data written to it. It will write a sha when the stream is closed + or if the asked for explicitly usign write_sha. + + Note: + Based on the dulwich project + """ + __slots__ = ("f", "sha1") + + def __init__(self, f): + self.f = f + self.sha1 = make_sha("") - def write(self, data): - self.sha1.update(data) - self.f.write(data) + def write(self, data): + self.sha1.update(data) + self.f.write(data) - def write_sha(self): - sha = self.sha1.digest() - self.f.write(sha) - return sha + def write_sha(self): + sha = self.sha1.digest() + self.f.write(sha) + return sha - def close(self): - sha = self.write_sha() - self.f.close() - return sha + def close(self): + sha = self.write_sha() + self.f.close() + return sha - def tell(self): - return self.f.tell() + def tell(self): + return self.f.tell() class LockFile(object): @@ -98,7 +131,7 @@ class LockFile(object): lock_file = self._lock_file_path() try: - fp = open(lock_file, "r") + fp = open(lock_file, "rb") pid = int(fp.read()) fp.close() except IOError: @@ -123,7 +156,7 @@ class LockFile(object): if os.path.exists(lock_file): raise IOError("Lock for file %r did already exist, delete %r in case the lock is illegal" % (self._file_path, lock_file)) - fp = open(lock_file, "w") + fp = open(lock_file, "wb") fp.write(str(os.getpid())) fp.close() @@ -179,7 +212,7 @@ class ConcurrentWriteOperation(LockFile): self._obtain_lock_or_raise() dirname, basename = os.path.split(self._file_path) - self._temp_write_fp = open(tempfile.mktemp(basename, '', dirname), "w") + self._temp_write_fp = open(tempfile.mktemp(basename, '', dirname), "wb") return self._temp_write_fp def _is_writing(self): diff --git a/test/git/test_commit.py b/test/git/test_commit.py index be6d1a28..2e3f131e 100644 --- a/test/git/test_commit.py +++ b/test/git/test_commit.py @@ -75,7 +75,7 @@ class TestCommit(TestBase): assert_equal(sha1, commit.sha) def test_count(self): - assert self.rorepo.tag('refs/tags/0.1.5').commit.count( ) == 141 + assert self.rorepo.tag('refs/tags/0.1.5').commit.count( ) == 143 def test_list(self): assert isinstance(Commit.list_items(self.rorepo, '0.1.5', max_count=5)['5117c9c8a4d3af19a9958677e45cda9269de1541'], Commit) diff --git a/test/git/test_git.py b/test/git/test_git.py index c4a39e85..6e4ab394 100644 --- a/test/git/test_git.py +++ b/test/git/test_git.py @@ -45,13 +45,6 @@ class TestGit(TestCase): self.git.hash_object(istream=fh, stdin=True)) fh.close() - def test_it_handles_large_input(self): - if sys.platform == 'win32': - output = self.git.execute(["type", "C:\WINDOWS\system32\cmd.exe"]) - else: - output = self.git.execute(["cat", "/bin/bash"]) - assert_true(len(output) > 4096) # at least 4k - @patch_object(Git, 'execute') def test_it_ignores_false_kwargs(self, git): # this_should_not_be_ignored=False implies it *should* be ignored diff --git a/test/git/test_index.py b/test/git/test_index.py index e9541232..1a543f82 100644 --- a/test/git/test_index.py +++ b/test/git/test_index.py @@ -42,7 +42,7 @@ class TestTree(TestBase): # write the data - it must match the original tmpfile = tempfile.mktemp() index_merge.write(tmpfile) - fp = open(tmpfile, 'r') + fp = open(tmpfile, 'rb') assert fp.read() == fixture("index_merge") fp.close() os.remove(tmpfile) @@ -164,14 +164,14 @@ 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") - fp = open(file_path, "w") + fp = open(file_path, "wb") fp.write(new_data) fp.close() index.reset(rev_head_parent, working_tree=True) assert not index.diff(None) assert cur_branch == rw_repo.active_branch assert cur_commit == rw_repo.head.commit - fp = open(file_path) + fp = open(file_path,'rb') try: assert fp.read() != new_data finally: @@ -332,7 +332,8 @@ class TestTree(TestBase): # add fake symlink and assure it checks-our as symlink fake_symlink_relapath = "my_fake_symlink" - fake_symlink_path = self._make_file(fake_symlink_relapath, "/etc/that", rw_repo) + link_target = "/etc/that" + fake_symlink_path = self._make_file(fake_symlink_relapath, link_target, rw_repo) fake_entry = BaseIndexEntry((0120000, null_sha, 0, fake_symlink_relapath)) entries = index.reset(new_commit).add([fake_entry]) assert len(entries) == 1 and S_ISLNK(entries[0].mode) @@ -341,5 +342,12 @@ class TestTree(TestBase): assert not S_ISLNK(os.stat(fake_symlink_path)[ST_MODE]) os.remove(fake_symlink_path) index.checkout(fake_symlink_path) - assert S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE]) + + # on windows we will never get symlinks + if os.name == 'nt': + # simlinks should contain the link as text ( which is what a + # symlink actually is ) + open(fake_symlink_path,'rb').read() == link_target + else: + assert S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE]) diff --git a/test/git/test_remote.py b/test/git/test_remote.py index 0af9f0cf..700798dd 100644 --- a/test/git/test_remote.py +++ b/test/git/test_remote.py @@ -240,7 +240,13 @@ class TestRemote(TestBase): lhead = rw_repo.head lindex = rw_repo.index # assure we are on master and it is checked out where the remote is - lhead.reference = rw_repo.heads.master + try: + lhead.reference = rw_repo.heads.master + except AttributeError: + # if the author is on a non-master branch, the clones might not have + # a local master yet. We simply create it + lhead.reference = rw_repo.create_head('master') + # END master handling lhead.reset(remote.refs.master, working_tree=True) # push without spec should fail ( without further configuration ) diff --git a/test/git/test_repo.py b/test/git/test_repo.py index 0b196a1f..93ab7a90 100644 --- a/test/git/test_repo.py +++ b/test/git/test_repo.py @@ -7,15 +7,14 @@ import os, sys from test.testlib import * from git import * +from git.utils import join_path_native +import tempfile class TestRepo(TestBase): @raises(InvalidGitRepositoryError) def test_new_should_raise_on_invalid_repo_location(self): - if sys.platform == "win32": - Repo("C:\\WINDOWS\\Temp") - else: - Repo("/tmp") + Repo(tempfile.gettempdir()) @raises(NoSuchPathError) def test_new_should_raise_on_non_existant_path(self): @@ -220,7 +219,8 @@ class TestRepo(TestBase): def test_untracked_files(self): base = self.rorepo.git.git_dir - files = (base+"/__test_myfile", base+"/__test_other_file") + files = ( join_path_native(base, "__test_myfile"), + join_path_native(base, "__test_other_file") ) num_recently_untracked = 0 try: for fpath in files: @@ -233,7 +233,7 @@ class TestRepo(TestBase): # assure we have all names - they are relative to the git-dir num_test_untracked = 0 for utfile in untracked_files: - num_test_untracked += os.path.join(base, utfile) in files + num_test_untracked += join_path_native(base, utfile) in files assert len(files) == num_test_untracked finally: for fpath in files: diff --git a/test/git/test_utils.py b/test/git/test_utils.py index 029d2054..69a9297d 100644 --- a/test/git/test_utils.py +++ b/test/git/test_utils.py @@ -56,7 +56,7 @@ class TestUtils(TestCase): def _cmp_contents(self, file_path, data): # raise if data from file at file_path # does not match data string - fp = open(file_path, "r") + fp = open(file_path, "rb") try: assert fp.read() == data finally: @@ -66,7 +66,7 @@ class TestUtils(TestCase): my_file = tempfile.mktemp() orig_data = "hello" new_data = "world" - my_file_fp = open(my_file, "w") + my_file_fp = open(my_file, "wb") my_file_fp.write(orig_data) my_file_fp.close() diff --git a/test/testlib/helper.py b/test/testlib/helper.py index 27c2b3d9..e33961a7 100644 --- a/test/testlib/helper.py +++ b/test/testlib/helper.py @@ -17,7 +17,7 @@ def fixture_path(name): return os.path.join(test_dir, "fixtures", name) def fixture(name): - return open(fixture_path(name)).read() + return open(fixture_path(name), 'rb').read() def absolute_project_path(): return os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) @@ -63,7 +63,18 @@ class ListProcessAdapter(object): poll = wait - + +def _rmtree_onerror(osremove, fullpath, exec_info): + """ + Handle the case on windows that read-only files cannot be deleted by + os.remove by setting it to mode 777, then retry deletion. + """ + if os.name != 'nt' or osremove is not os.remove: + raise + + os.chmod(fullpath, 0777) + os.remove(fullpath) + def with_bare_rw_repo(func): """ Decorator providing a specially made read-write repository to the test case @@ -82,7 +93,8 @@ def with_bare_rw_repo(func): try: return func(self, rw_repo) finally: - shutil.rmtree(repo_dir) + rw_repo.git.clear_cache() + shutil.rmtree(repo_dir, onerror=_rmtree_onerror) # END cleanup # END bare repo creator bare_repo_creator.__name__ = func.__name__ @@ -107,7 +119,8 @@ def with_rw_repo(working_tree_ref): try: return func(self, rw_repo) finally: - shutil.rmtree(repo_dir) + rw_repo.git.clear_cache() + shutil.rmtree(repo_dir, onerror=_rmtree_onerror) # END cleanup # END rw repo creator repo_creator.__name__ = func.__name__ @@ -174,13 +187,18 @@ def with_rw_and_rw_remote_repo(working_tree_ref): rw_repo.git.ls_remote(d_remote) except GitCommandError,e: print str(e) - raise AssertionError('Please start a git-daemon to run this test, execute: git-daemon "%s"'%tempfile.gettempdir()) + if os.name == 'nt': + raise AssertionError('git-daemon needs to run this test, but windows does not have one. Otherwise, run: git-daemon "%s"'%tempfile.gettempdir()) + else: + raise AssertionError('Please start a git-daemon to run this test, execute: git-daemon "%s"'%tempfile.gettempdir()) try: return func(self, rw_repo, rw_remote_repo) finally: - shutil.rmtree(repo_dir) - shutil.rmtree(remote_repo_dir) + rw_repo.git.clear_cache() + rw_remote_repo.git.clear_cache() + shutil.rmtree(repo_dir, onerror=_rmtree_onerror) + shutil.rmtree(remote_repo_dir, onerror=_rmtree_onerror) # END cleanup # END bare repo creator remote_repo_creator.__name__ = func.__name__ |