From 547e6f0e2e9ace80e6d6b08db3c15fe91efe880f Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Mon, 12 Sep 2016 17:19:20 +0100 Subject: [PATCH 01/16] Replaced lock file code with flock() --- git/util.py | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/git/util.py b/git/util.py index f5c692315..fff8ddc0a 100644 --- a/git/util.py +++ b/git/util.py @@ -4,6 +4,7 @@ # This module is part of GitPython and is released under # the BSD License: http://www.opensource.org/licenses/bsd-license.php +from fcntl import flock, LOCK_UN, LOCK_EX, LOCK_NB import os import re import sys @@ -324,12 +325,12 @@ def update(self, op_code, cur_count, max_count=None, message=''): You may read the contents of the current line in self._cur_line""" pass - + class CallableRemoteProgress(RemoteProgress): """An implementation forwarding updates to any callable""" __slots__ = ('_callable') - + def __init__(self, fn): self._callable = fn super(CallableRemoteProgress, self).__init__() @@ -535,9 +536,10 @@ class LockFile(object): As we are a utility class to be derived from, we only use protected methods. Locks will automatically be released on destruction""" - __slots__ = ("_file_path", "_owns_lock") + __slots__ = ("_file_path", "_owns_lock", "_file_descriptor") def __init__(self, file_path): + self._file_descriptor = None self._file_path = file_path self._owns_lock = False @@ -559,17 +561,24 @@ def _obtain_lock_or_raise(self): :raise IOError: if a lock was already present or a lock file could not be written""" if self._has_lock(): return + lock_file = self._lock_file_path() - if os.path.isfile(lock_file): - raise IOError("Lock for file %r did already exist, delete %r in case the lock is illegal" % - (self._file_path, lock_file)) + + # Create lock file + try: + open(lock_file, 'a').close() + except OSError as e: + # Silence error only if file exists + if e.errno != 17: # 17 -> File exists + raise try: - fd = os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0) - os.close(fd) + fd = os.open(lock_file, os.O_WRONLY, 0) + flock(fd, LOCK_EX | LOCK_NB) except OSError as e: raise IOError(str(e)) + self._file_descriptor = fd self._owns_lock = True def _obtain_lock(self): @@ -582,19 +591,15 @@ def _release_lock(self): if not self._has_lock(): return - # if someone removed our file beforhand, lets just flag this issue - # instead of failing, to make it more usable. - lfp = self._lock_file_path() - try: - # on bloody windows, the file needs write permissions to be removable. - # Why ... - if os.name == 'nt': - os.chmod(lfp, 0o777) - # END handle win32 - os.remove(lfp) - except OSError: - pass + fd = self._file_descriptor + lock_file = self._lock_file_path() + + flock(fd, LOCK_UN) + os.close(fd) + os.remove(lock_file) + self._owns_lock = False + self._file_descriptor = None class BlockingLockFile(LockFile): @@ -629,7 +634,7 @@ def _obtain_lock(self): try: super(BlockingLockFile, self)._obtain_lock() except IOError: - # synity check: if the directory leading to the lockfile is not + # sanity check: if the directory leading to the lockfile is not # readable anymore, raise an execption curtime = time.time() if not os.path.isdir(os.path.dirname(self._lock_file_path())): From 6dfc870cffcfa7ca29e30301fff256e4e5b181f3 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Wed, 14 Sep 2016 10:23:34 +0100 Subject: [PATCH 02/16] No need to create the lock file beforehand --- git/util.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/git/util.py b/git/util.py index fff8ddc0a..8d97242c7 100644 --- a/git/util.py +++ b/git/util.py @@ -564,20 +564,14 @@ def _obtain_lock_or_raise(self): lock_file = self._lock_file_path() - # Create lock file + # Create file and lock try: - open(lock_file, 'a').close() - except OSError as e: - # Silence error only if file exists - if e.errno != 17: # 17 -> File exists - raise - - try: - fd = os.open(lock_file, os.O_WRONLY, 0) - flock(fd, LOCK_EX | LOCK_NB) + fd = os.open(lock_file, os.O_CREAT, 0) except OSError as e: raise IOError(str(e)) + flock(fd, LOCK_EX | LOCK_NB) + self._file_descriptor = fd self._owns_lock = True From 1e4eff1d7ff660077eb0e05f39f9466c7e00d34e Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Wed, 14 Sep 2016 10:26:00 +0100 Subject: [PATCH 03/16] Ignore cache dir of Py.test --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index d35cddebd..a672c3f16 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,4 @@ nbproject .DS_Store /*egg-info /.tox +.cache From ae19557f8b6e5387b7275d46621535b24ace00d8 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Wed, 14 Sep 2016 10:31:52 +0100 Subject: [PATCH 04/16] Updated AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index e2c3293cc..b06c5b46f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,5 +15,6 @@ Contributors are: -Jonathan Chu -Vincent Driessen -Phil Elson +-Daniele Esposti Portions derived from other open source works and are clearly marked. From f0fcc072b37d6e773e18ea561ebce8ccba2ba19c Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Mon, 19 Sep 2016 23:32:39 +0100 Subject: [PATCH 05/16] Force GC collection --- git/test/test_util.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git/test/test_util.py b/git/test/test_util.py index c6ca6920b..3a67c04ba 100644 --- a/git/test/test_util.py +++ b/git/test/test_util.py @@ -5,6 +5,7 @@ # the BSD License: http://www.opensource.org/licenses/bsd-license.php import tempfile +import gc from git.test.lib import ( TestBase, @@ -76,6 +77,7 @@ def test_lock_file(self): # auto-release on destruction del(other_lock_file) + gc.collect() lock_file._obtain_lock_or_raise() lock_file._release_lock() From d437f8893f60bfdc703b0d16335f28ad2e799974 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Thu, 6 Oct 2016 23:54:32 +0100 Subject: [PATCH 06/16] Fixed typo after merge --- git/util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/git/util.py b/git/util.py index 9f8ccea58..fabfffb8f 100644 --- a/git/util.py +++ b/git/util.py @@ -616,9 +616,8 @@ def _release_lock(self): # if someone removed our file beforhand, lets just flag this issue # instead of failing, to make it more usable. - lfp = self._lock_file_path() try: - rmfile(lfp) + rmfile(lock_file) except OSError: pass self._owns_lock = False From ab7a87290149114e080037f8da0871939b1ed7fc Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Fri, 7 Oct 2016 22:45:36 +0100 Subject: [PATCH 07/16] Updated file locking for Windows platform --- git/util.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/git/util.py b/git/util.py index fabfffb8f..75c455b62 100644 --- a/git/util.py +++ b/git/util.py @@ -5,7 +5,6 @@ # the BSD License: http://www.opensource.org/licenses/bsd-license.php from __future__ import unicode_literals -from fcntl import flock, LOCK_UN, LOCK_EX, LOCK_NB import getpass import logging import os @@ -49,6 +48,57 @@ #{ Utility Methods +if platform.system() == 'Windows': + # This code is a derivative work of Portalocker http://code.activestate.com/recipes/65203/ + import win32con + import win32file + import pywintypes + + LOCK_EX = win32con.LOCKFILE_EXCLUSIVE_LOCK + LOCK_SH = 0 # the default + LOCK_NB = win32con.LOCKFILE_FAIL_IMMEDIATELY + LOCK_UN = 1 << 2 + + __overlapped = pywintypes.OVERLAPPED() + + def flock(fd, flags=0): + hfile = win32file._get_osfhandle(fd) + + if flags & LOCK_UN != 0: + # Unlock file descriptor + try: + win32file.UnlockFileEx(hfile, 0, -0x10000, __overlapped) + except pywintypes.error, exc_value: + # error: (158, 'UnlockFileEx', 'The segment is already unlocked.') + # To match the 'posix' implementation, silently ignore this error + if exc_value[0] == 158: + pass + else: + # Q: Are there exceptions/codes we should be dealing with here? + raise + + elif flags & LOCK_EX != 0: + # Lock file + try: + win32file.LockFileEx(hfile, flags, 0, -0x10000, __overlapped) + except pywintypes.error, exc_value: + if exc_value[0] == 33: + # error: (33, 'LockFileEx', + # 'The process cannot access the file because another process has locked + # a portion of the file.') + raise IOError(33, exc_value[2]) + else: + # Q: Are there exceptions/codes we should be dealing with here? + raise + + else: + raise NotImplementedError("Unsupported set of bitflags {}".format(bin(flags))) + + +else: + # from fcntl import flock, LOCK_UN, LOCK_EX, LOCK_NB + pass + def unbare_repo(func): """Methods with this decorator raise InvalidGitRepositoryError if they @@ -620,6 +670,7 @@ def _release_lock(self): rmfile(lock_file) except OSError: pass + self._owns_lock = False self._file_descriptor = None From c0b45fa508a8826a70a0593f3808c8c053b91787 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Fri, 7 Oct 2016 23:08:53 +0100 Subject: [PATCH 08/16] Updated setup to include Windows requirements --- setup.py | 7 +++++++ win32-requirements.txt | 2 ++ 2 files changed, 9 insertions(+) create mode 100644 win32-requirements.txt diff --git a/setup.py b/setup.py index c7dd25fcc..43b0505bd 100755 --- a/setup.py +++ b/setup.py @@ -13,6 +13,7 @@ import logging import os import sys +import platform from os import path with open(path.join(path.dirname(__file__), 'VERSION')) as v: @@ -21,6 +22,10 @@ with open('requirements.txt') as reqs_file: requirements = reqs_file.read().splitlines() +if platform.system() == 'Windows': + with open('win32-requirements.txt') as reqs_file: + requirements += reqs_file.read().splitlines() + class build_py(_build_py): @@ -65,6 +70,8 @@ def _stamp_version(filename): print("WARNING: Couldn't find version line in file %s" % filename, file=sys.stderr) install_requires = ['gitdb >= 0.6.4'] +if platform.system() == 'Windows': + install_requires.append("pypiwin32 >= 219") extras_require = { ':python_version == "2.6"': ['ordereddict'], } diff --git a/win32-requirements.txt b/win32-requirements.txt new file mode 100644 index 000000000..0008dcffd --- /dev/null +++ b/win32-requirements.txt @@ -0,0 +1,2 @@ +-r requirements.txt +pypiwin32 >= 219 From 4c9dd7fac214bb0f85077cab40d4455cf3113b17 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Fri, 7 Oct 2016 23:12:39 +0100 Subject: [PATCH 09/16] Forgot to uncomment conditional import --- git/util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/git/util.py b/git/util.py index 75c455b62..ab27dadfb 100644 --- a/git/util.py +++ b/git/util.py @@ -96,8 +96,7 @@ def flock(fd, flags=0): else: - # from fcntl import flock, LOCK_UN, LOCK_EX, LOCK_NB - pass + from fcntl import flock, LOCK_UN, LOCK_EX, LOCK_NB def unbare_repo(func): From 20b4f49b4e71c90581c68ea7f4cf56160a6bdc79 Mon Sep 17 00:00:00 2001 From: Daniele Esposti Date: Fri, 7 Oct 2016 23:42:32 +0100 Subject: [PATCH 10/16] Fix syntax for Python 3.x --- git/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git/util.py b/git/util.py index ab27dadfb..037fd91b5 100644 --- a/git/util.py +++ b/git/util.py @@ -68,7 +68,7 @@ def flock(fd, flags=0): # Unlock file descriptor try: win32file.UnlockFileEx(hfile, 0, -0x10000, __overlapped) - except pywintypes.error, exc_value: + except pywintypes.error as exc_value: # error: (158, 'UnlockFileEx', 'The segment is already unlocked.') # To match the 'posix' implementation, silently ignore this error if exc_value[0] == 158: @@ -81,7 +81,7 @@ def flock(fd, flags=0): # Lock file try: win32file.LockFileEx(hfile, flags, 0, -0x10000, __overlapped) - except pywintypes.error, exc_value: + except pywintypes.error as exc_value: if exc_value[0] == 33: # error: (33, 'LockFileEx', # 'The process cannot access the file because another process has locked From e118818e61ec393451e9b597a10c645220484644 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 10 Oct 2016 10:02:16 +0200 Subject: [PATCH 11/16] imp(performance): execute performance tests on travis Fixes #524 --- git/test/performance/lib.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/git/test/performance/lib.py b/git/test/performance/lib.py index 0c4c20a47..b57b9b714 100644 --- a/git/test/performance/lib.py +++ b/git/test/performance/lib.py @@ -3,7 +3,6 @@ from git.test.lib import ( TestBase ) -from gitdb.test.lib import skip_on_travis_ci import tempfile import logging @@ -43,8 +42,6 @@ class TestBigRepoR(TestBase): #} END invariants def setUp(self): - # This will raise on travis, which is what we want to happen early as to prevent us to do any work - skip_on_travis_ci(lambda *args: None)(self) try: super(TestBigRepoR, self).setUp() except AttributeError: From 955750852912f31b0c2a60597ef58443e7a78a30 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 10 Oct 2016 10:16:35 +0200 Subject: [PATCH 12/16] fix(travis): increase ulimit Now that performance tests are run, it appears we run into one particular failure on travis, possibly indicating a bug in python 3.3. Just bluntly increason the amount of handles might silence it... . Related to #524 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2d6764054..2691d87c4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -29,7 +29,7 @@ install: - cat git/test/fixtures/.gitconfig >> ~/.gitconfig script: # Make sure we limit open handles to see if we are leaking them - - ulimit -n 110 + - ulimit -n 128 - ulimit -n - nosetests -v --with-coverage - if [ "$TRAVIS_PYTHON_VERSION" == '3.4' ]; then flake8; fi From 7642479871b5f40ee02e3d481bfe111cd68457e1 Mon Sep 17 00:00:00 2001 From: Guyzmo Date: Tue, 11 Oct 2016 14:25:23 +0200 Subject: [PATCH 13/16] Improved way of listing URLs in remote instead of using git remote show that can trigger a connection to the remote repository, use git remote get-url --all that works by only reading the .git/config. change should have no functional impact, so no test needed. Signed-off-by: Guyzmo --- git/remote.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/git/remote.py b/git/remote.py index d35e1fad1..a28917a52 100644 --- a/git/remote.py +++ b/git/remote.py @@ -496,10 +496,9 @@ def delete_url(self, url, **kwargs): def urls(self): """:return: Iterator yielding all configured URL targets on a remote as strings""" - remote_details = self.repo.git.remote("show", self.name) + remote_details = self.repo.git.remote("get-url", "--all", self.name) for line in remote_details.split('\n'): - if ' Push URL:' in line: - yield line.split(': ')[-1] + yield line @property def refs(self): From 282824d79563f2f1349ce1212ee37d439916537a Mon Sep 17 00:00:00 2001 From: Guyzmo Date: Wed, 12 Oct 2016 06:58:29 +0200 Subject: [PATCH 14/16] Updated AUTHORS file --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index b06c5b46f..44ac4e2ff 100644 --- a/AUTHORS +++ b/AUTHORS @@ -16,5 +16,6 @@ Contributors are: -Vincent Driessen -Phil Elson -Daniele Esposti +-Bernard `Guyzmo` Pratz Portions derived from other open source works and are clearly marked. From 32ca6864a9528cc483e9eaeb7ebd25c5e95dc376 Mon Sep 17 00:00:00 2001 From: Kostis Anagnostopoulos Date: Tue, 11 Oct 2016 19:12:33 +0200 Subject: [PATCH 15/16] remote, #528: fix prev cmt, Git<2.7 miss `get-url` --- git/remote.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/git/remote.py b/git/remote.py index a28917a52..7c0e64ed6 100644 --- a/git/remote.py +++ b/git/remote.py @@ -33,6 +33,7 @@ from gitdb.util import join from git.compat import (defenc, force_text, is_win) import logging +from git.exc import GitCommandError log = logging.getLogger('git.remote') @@ -494,11 +495,22 @@ def delete_url(self, url, **kwargs): @property def urls(self): - """:return: Iterator yielding all configured URL targets on a remote - as strings""" - remote_details = self.repo.git.remote("get-url", "--all", self.name) - for line in remote_details.split('\n'): - yield line + """:return: Iterator yielding all configured URL targets on a remote as strings""" + try: + remote_details = self.repo.git.remote("get-url", "--all", self.name) + for line in remote_details.split('\n'): + yield line + except GitCommandError as ex: + ## We are on git < 2.7 (i.e TravisCI as of Oct-2016), + # so `get-utl` command does not exist yet! + # see: https://github.com/gitpython-developers/GitPython/pull/528#issuecomment-252976319 + # and: http://stackoverflow.com/a/32991784/548792 + # + if 'Unknown subcommand: get-url' in str(ex): + remote_details = self.repo.git.remote("show", self.name) + for line in remote_details.split('\n'): + if ' Push URL:' in line: + yield line.split(': ')[-1] @property def refs(self): From 621c5e721a9c19342eccad54d065d6a6f6133d08 Mon Sep 17 00:00:00 2001 From: Guyzmo Date: Wed, 12 Oct 2016 07:13:05 +0200 Subject: [PATCH 16/16] Fixed shadowing of potential exceptions. --- git/remote.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/git/remote.py b/git/remote.py index 7c0e64ed6..e8cfe5161 100644 --- a/git/remote.py +++ b/git/remote.py @@ -511,6 +511,8 @@ def urls(self): for line in remote_details.split('\n'): if ' Push URL:' in line: yield line.split(': ')[-1] + else: + raise ex @property def refs(self):