| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 76150aa commit e0f75ac
3 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1270,8 +1270,8 @@ class BlockingLockFile(LockFile): | |||
| 1270 | 1270 | specified timeout. | |
| 1271 | 1271 | ||
| 1272 | 1272 | :note: | |
| 1273 | - If the directory containing the lock was removed, an exception will be raised | ||
| 1274 | - during the blocking period, preventing hangs as the lock can never be obtained. | ||
| 1273 | + Only an existing lock causes retries. Other errors, including a removed or | ||
| 1274 | + unwritable parent directory, are raised immediately. | ||
| 1275 | 1275 | """ | |
| 1276 | 1276 | ||
| 1277 | 1277 | __slots__ = ("_check_interval", "_max_block_time") | |
@@ -1297,7 +1297,7 @@ def __init__( | |||
| 1297 | 1297 | ||
| 1298 | 1298 | def _obtain_lock(self) -> None: | |
| 1299 | 1299 | """This method blocks until it obtained the lock, or raises :exc:`IOError` if it | |
| 1300 | - ran out of time or if the parent directory was not available anymore. | ||
| 1300 | + ran out of time or lock creation failed for a reason other than an existing lock. | ||
| 1301 | 1301 | ||
| 1302 | 1302 | If this method returns, you are guaranteed to own the lock. | |
| 1303 | 1303 | """ | |
@@ -1307,8 +1307,12 @@ def _obtain_lock(self) -> None: | |||
| 1307 | 1307 | try: | |
| 1308 | 1308 | super()._obtain_lock() | |
| 1309 | 1309 | except OSError as e: | |
| 1310 | - # synity check: if the directory leading to the lockfile is not | ||
| 1311 | - # readable anymore, raise an exception | ||
| 1310 | + # LockFile retains the creation error as the cause of its OSError. | ||
| 1311 | + # Only an existing lock is contention; other errors must not be retried. | ||
| 1312 | + if not isinstance(e.__cause__, FileExistsError): | ||
| 1313 | + raise | ||
| 1314 | + | ||
| 1315 | + # Abort if the directory containing the existing lock has disappeared. | ||
| 1312 | 1316 | curtime = time.time() | |
| 1313 | 1317 | if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir): | |
| 1314 | 1318 | msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % ( | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -3,6 +3,7 @@ | |||
| 3 | 3 | # This module is part of GitPython and is released under the | |
| 4 | 4 | # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ | |
| 5 | 5 | ||
| 6 | + import errno | ||
| 6 | 7 | import glob | |
| 7 | 8 | import io | |
| 8 | 9 | import os | |
@@ -17,7 +18,7 @@ | |||
| 17 | 18 | from git import GitConfigParser, Repo | |
| 18 | 19 | from git.compat import defenc | |
| 19 | 20 | from git.config import _OMD, cp | |
| 20 | - from git.util import cwd, rmfile | ||
| 21 | + from git.util import BlockingLockFile, cwd, rmfile | ||
| 21 | 22 | from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory | |
| 22 | 23 | ||
| 23 | 24 | _tc_lock_fpaths = osp.join(osp.dirname(__file__), "fixtures/*.lock") | |
@@ -28,6 +29,23 @@ def _rm_lock_files(): | |||
| 28 | 29 | rmfile(lfp) | |
| 29 | 30 | ||
| 30 | 31 | ||
| 32 | + @pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows") | ||
| 33 | + @pytest.mark.parametrize("error_number", [errno.EACCES, errno.ENOENT]) | ||
| 34 | + def test_blocking_config_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number): | ||
| 35 | + monkeypatch.chdir(tmp_path) | ||
| 36 | + monkeypatch.setattr(GitConfigParser, "t_lock", BlockingLockFile) | ||
| 37 | + error = OSError(error_number, os.strerror(error_number), "config.lock") | ||
| 38 | + with mock.patch.object(os, "open", side_effect=error) as open_lock: | ||
| 39 | + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: | ||
| 40 | + with pytest.raises(OSError) as exc_info: | ||
| 41 | + GitConfigParser("config", read_only=False) | ||
| 42 | + sleep.assert_not_called() | ||
| 43 | + open_lock.assert_called_once() | ||
| 44 | + assert exc_info.value.__cause__ is error | ||
| 45 | + assert str(exc_info.value) == str(error) | ||
| 46 | + assert not (tmp_path / "config.lock").exists() | ||
| 47 | + | ||
| 48 | + | ||
| 31 | 49 | class TestBase(TestCase): | |
| 32 | 50 | def setUp(self): | |
| 33 | 51 | _rm_lock_files() | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,7 +4,7 @@ | |||
| 4 | 4 | # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ | |
| 5 | 5 | ||
| 6 | 6 | import ast | |
| 7 | - from datetime import datetime | ||
| 7 | + import errno | ||
| 8 | 8 | import os | |
| 9 | 9 | import pathlib | |
| 10 | 10 | import pickle | |
@@ -14,6 +14,7 @@ | |||
| 14 | 14 | import tempfile | |
| 15 | 15 | import threading | |
| 16 | 16 | import time | |
| 17 | + from datetime import datetime | ||
| 17 | 18 | from unittest import SkipTest, mock | |
| 18 | 19 | ||
| 19 | 20 | import ddt | |
@@ -37,12 +38,11 @@ | |||
| 37 | 38 | LockFile, | |
| 38 | 39 | cygpath, | |
| 39 | 40 | decygpath, | |
| 40 | - is_cygwin_git, | ||
| 41 | 41 | get_user_id, | |
| 42 | + is_cygwin_git, | ||
| 42 | 43 | remove_password_if_present, | |
| 43 | 44 | rmtree, | |
| 44 | 45 | ) | |
| 45 | - | ||
| 46 | 46 | from test.lib import TestBase, requires_symlinks, with_rw_repo | |
| 47 | 47 | ||
| 48 | 48 | ||
@@ -917,5 +917,68 @@ def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type | |||
| 917 | 917 | ||
| 918 | 918 | def test_blocking_lock_still_rejects_missing_directory(tmp_path): | |
| 919 | 919 | waiter = BlockingLockFile(tmp_path / "missing" / "resource") | |
| 920 | - with pytest.raises(OSError, match="Directory containing the lockfile"): | ||
| 921 | - waiter._obtain_lock() | ||
| 920 | + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: | ||
| 921 | + with pytest.raises(OSError) as exc_info: | ||
| 922 | + waiter._obtain_lock() | ||
| 923 | + sleep.assert_not_called() | ||
| 924 | + assert isinstance(exc_info.value.__cause__, FileNotFoundError) | ||
| 925 | + assert not waiter._has_lock() | ||
| 926 | + | ||
| 927 | + | ||
| 928 | + @pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows") | ||
| 929 | + @pytest.mark.parametrize( | ||
| 930 | + "error_number", [errno.EACCES, errno.ENOENT, errno.ENOTDIR, errno.EROFS, errno.ENOSPC, errno.EIO] | ||
| 931 | + ) | ||
| 932 | + @pytest.mark.parametrize("path_type", [str, pathlib.Path]) | ||
| 933 | + @pytest.mark.parametrize("relative", [True, False]) | ||
| 934 | + def test_blocking_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number, path_type, relative): | ||
| 935 | + monkeypatch.chdir(tmp_path) | ||
| 936 | + path = path_type("resource" if relative else tmp_path / "resource") | ||
| 937 | + waiter = BlockingLockFile(path) | ||
| 938 | + error = OSError(error_number, os.strerror(error_number), waiter._lock_file_path()) | ||
| 939 | + with mock.patch.object(os, "open", side_effect=error) as open_lock: | ||
| 940 | + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: | ||
| 941 | + with pytest.raises(OSError) as exc_info: | ||
| 942 | + waiter._obtain_lock() | ||
| 943 | + sleep.assert_not_called() | ||
| 944 | + open_lock.assert_called_once() | ||
| 945 | + assert exc_info.value.__cause__ is error | ||
| 946 | + assert str(exc_info.value) == str(error) | ||
| 947 | + assert not waiter._has_lock() | ||
| 948 | + assert not os.path.lexists(waiter._lock_file_path()) | ||
| 949 | + | ||
| 950 | + | ||
| 951 | + @pytest.mark.skipif(os.name != "posix", reason="Directory write permissions require POSIX") | ||
| 952 | + @pytest.mark.parametrize("path_type", [str, pathlib.Path]) | ||
| 953 | + def test_blocking_relative_lock_rejects_unwritable_cwd(tmp_path, monkeypatch, path_type): | ||
| 954 | + monkeypatch.chdir(tmp_path) | ||
| 955 | + original_mode = tmp_path.stat().st_mode | ||
| 956 | + tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR) | ||
| 957 | + try: | ||
| 958 | + if os.access(".", os.W_OK): | ||
| 959 | + pytest.skip("Current user can write to read-only directories") | ||
| 960 | + assert os.path.isdir(".") | ||
| 961 | + waiter = BlockingLockFile(path_type("resource")) | ||
| 962 | + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: | ||
| 963 | + with pytest.raises(OSError) as exc_info: | ||
| 964 | + waiter._obtain_lock() | ||
| 965 | + sleep.assert_not_called() | ||
| 966 | + assert isinstance(exc_info.value.__cause__, PermissionError) | ||
| 967 | + assert not waiter._has_lock() | ||
| 968 | + finally: | ||
| 969 | + tmp_path.chmod(original_mode) | ||
| 970 | + | ||
| 971 | + | ||
| 972 | + @pytest.mark.skipif(os.name != "posix", reason="Removing the current directory requires POSIX") | ||
| 973 | + @pytest.mark.parametrize("path_type", [str, pathlib.Path]) | ||
| 974 | + def test_blocking_relative_lock_rejects_removed_cwd(tmp_path, monkeypatch, path_type): | ||
| 975 | + monkeypatch.chdir(tmp_path) | ||
| 976 | + tmp_path.rmdir() | ||
| 977 | + assert os.path.isdir(".") | ||
| 978 | + waiter = BlockingLockFile(path_type("resource")) | ||
| 979 | + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: | ||
| 980 | + with pytest.raises(OSError) as exc_info: | ||
| 981 | + waiter._obtain_lock() | ||
| 982 | + sleep.assert_not_called() | ||
| 983 | + assert isinstance(exc_info.value.__cause__, FileNotFoundError) | ||
| 984 | + assert not waiter._has_lock() | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments