diff --git a/dvc/utils/fs.py b/dvc/utils/fs.py index 8bbc097c00..f3bd706c61 100644 --- a/dvc/utils/fs.py +++ b/dvc/utils/fs.py @@ -1,3 +1,4 @@ +import sys import errno import logging import os @@ -129,6 +130,13 @@ def _chmod(func, p, excinfo): func(p) +def _unlink(path, onerror): + try: + os.unlink(path) + except OSError: + onerror(os.unlink, path, sys.exc_info()) + + def remove(path): logger.debug("Removing '%s'", path) @@ -137,7 +145,7 @@ def remove(path): if os.path.isdir(path): shutil.rmtree(path, onerror=_chmod) else: - _chmod(os.unlink, path, None) + _unlink(path, _chmod) except OSError as exc: if exc.errno != errno.ENOENT: raise diff --git a/tests/func/test_unprotect.py b/tests/func/test_unprotect.py index 782ee7c5ca..8aeed22870 100644 --- a/tests/func/test_unprotect.py +++ b/tests/func/test_unprotect.py @@ -24,11 +24,14 @@ def test(self): self.assertTrue(os.access(self.FOO, os.W_OK)) - # NOTE: cache is now unprotected, bceause we try to set write perms - # on files that we try to delete, as some filesystems require that - # (e.g. NTFS). But it should be restored after the next cache check, - # hence why we call `dvc status` here. - self.assertTrue(os.access(cache, os.W_OK)) - ret = main(["status"]) - self.assertEqual(ret, 0) + if os.name == "nt": + # NOTE: cache is now unprotected, because NTFS doesn't allow + # deleting read-only files, so we have to try to set write perms + # on files that we try to delete, which propagates to the cache + # file. But it should be restored after the next cache check, hence + # why we call `dvc status` here. + self.assertTrue(os.access(cache, os.W_OK)) + ret = main(["status"]) + self.assertEqual(ret, 0) + self.assertFalse(os.access(cache, os.W_OK)) diff --git a/tests/unit/remote/test_local.py b/tests/unit/remote/test_local.py index 2ce4b5d8d8..a5d653e971 100644 --- a/tests/unit/remote/test_local.py +++ b/tests/unit/remote/test_local.py @@ -54,11 +54,12 @@ def test_is_protected(tmp_dir, dvc, link_name): remote.unprotect(link) assert not remote.is_protected(link) - if link_name == "symlink" and os.name == "nt": - # NOTE: Windows symlink perms don't propagate to the target - assert remote.is_protected(foo) - else: + if os.name == "nt" and link_name == "hardlink": + # NOTE: NTFS doesn't allow deleting read-only files, which forces us to + # set write perms on the link, which propagates to the source. assert not remote.is_protected(foo) + else: + assert remote.is_protected(foo) @pytest.mark.parametrize("err", [errno.EPERM, errno.EACCES])