From 00641196696a47a3b3ddba0bcc33fcff47cfb9c2 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 25 Mar 2020 03:11:34 +0200 Subject: [PATCH 1/2] protect: fix bug in mode verification --- dvc/remote/local.py | 4 ++-- tests/unit/remote/test_local.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 25d409f67d..c8cb0d9905 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -453,8 +453,8 @@ def protect(self, path_info): if exc.errno not in [errno.EPERM, errno.EACCES]: raise - actual = os.stat(path).st_mode - if actual & mode != mode: + actual = stat.S_IMODE(os.stat(path).st_mode) + if actual != mode: raise def _get_unpacked_dir_path_info(self, checksum): diff --git a/tests/unit/remote/test_local.py b/tests/unit/remote/test_local.py index d18e3b4fb7..28317cdd14 100644 --- a/tests/unit/remote/test_local.py +++ b/tests/unit/remote/test_local.py @@ -1,4 +1,5 @@ import os +import errno import pytest @@ -58,3 +59,18 @@ def test_is_protected(tmp_dir, link_name): assert remote.is_protected(foo) else: assert not remote.is_protected(foo) + + +@pytest.mark.parametrize("err", [errno.EPERM, errno.EACCES]) +def test_protect_ignore_errors(tmp_dir, mocker, err): + tmp_dir.gen("foo", "foo") + foo = PathInfo("foo") + remote = RemoteLOCAL(None, {}) + + remote.protect(foo) + + mock_chmod = mocker.patch( + "os.chmod", side_effect=OSError(err, "something") + ) + remote.protect(foo) + assert mock_chmod.called From 15dc06e0ebd635c8f6a1c0eb31556cd968ed118a Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 26 Mar 2020 01:36:44 +0200 Subject: [PATCH 2/2] protect: ignore EROFS Fixes #3510 --- dvc/remote/local.py | 6 +++++- tests/unit/remote/test_local.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dvc/remote/local.py b/dvc/remote/local.py index c8cb0d9905..7a93434f3d 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -448,7 +448,11 @@ def protect(self, path_info): try: os.chmod(path, mode) except OSError as exc: - # In share cache scenario, we might not own the cache file, so we + # There is nothing we need to do in case of a read-only file system + if exc.errno == errno.EROFS: + return + + # In shared cache scenario, we might not own the cache file, so we # need to check if cache file is already protected. if exc.errno not in [errno.EPERM, errno.EACCES]: raise diff --git a/tests/unit/remote/test_local.py b/tests/unit/remote/test_local.py index 28317cdd14..04e216bde0 100644 --- a/tests/unit/remote/test_local.py +++ b/tests/unit/remote/test_local.py @@ -74,3 +74,15 @@ def test_protect_ignore_errors(tmp_dir, mocker, err): ) remote.protect(foo) assert mock_chmod.called + + +def test_protect_ignore_erofs(tmp_dir, mocker): + tmp_dir.gen("foo", "foo") + foo = PathInfo("foo") + remote = RemoteLOCAL(None, {}) + + mock_chmod = mocker.patch( + "os.chmod", side_effect=OSError(errno.EROFS, "read-only fs") + ) + remote.protect(foo) + assert mock_chmod.called