diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 833f59bb4..b5e84a5cc 100644 --- a/dvc/remote/local.py +++ b/dvc/remote/local.py @@ -129,7 +129,7 @@ class RemoteLOCAL(RemoteBase): def exists(self, path_info): assert not isinstance(path_info, list) assert path_info["scheme"] == "local" - return os.path.exists(path_info["path"]) + return os.path.lexists(path_info["path"]) def changed_cache(self, md5): cache = self.get(md5) @@ -316,7 +316,7 @@ class RemoteLOCAL(RemoteBase): cache = self.get(checksum) if not self.is_dir_cache(cache): - if os.path.exists(path): + if self.exists(path_info): self.safe_remove(path_info, force=force) self.link(cache, path) @@ -347,7 +347,7 @@ class RemoteLOCAL(RemoteBase): entry_checksum_info = {self.PARAM_CHECKSUM: m} if self.changed(entry_info, entry_checksum_info): - if os.path.exists(p): + if self.exists(entry_info): self.safe_remove(entry_info, force=force) self.link(c, p) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 553f503eb..83faa59e7 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -10,6 +10,7 @@ import sys import stat import math import json +import errno import shutil import hashlib import nanotime @@ -156,22 +157,30 @@ def move(src, dst): shutil.move(src, dst) -def remove(path): - if not os.path.exists(path): - return - - logger.debug("Removing '{}'".format(os.path.relpath(path))) - - def _chmod(func, p, excinfo): +def _chmod(func, p, excinfo): + try: perm = os.stat(p).st_mode perm |= stat.S_IWRITE os.chmod(p, perm) - func(p) + except OSError as exc: + # NOTE: broken symlink case. + if exc.errno != errno.ENOENT: + raise - if os.path.isfile(path): - _chmod(os.unlink, path, None) - else: - shutil.rmtree(path, onerror=_chmod) + func(p) + + +def remove(path): + logger.debug("Removing '{}'".format(os.path.relpath(path))) + + try: + if os.path.isdir(path): + shutil.rmtree(path, onerror=_chmod) + else: + _chmod(os.unlink, path, None) + except OSError as exc: + if exc.errno != errno.ENOENT: + raise def to_chunks(l, jobs): diff --git a/dvc/utils/fs.py b/dvc/utils/fs.py index 822ef6401..7b3c67101 100644 --- a/dvc/utils/fs.py +++ b/dvc/utils/fs.py @@ -1,7 +1,8 @@ from __future__ import unicode_literals -import nanotime import os +import errno +import nanotime import logging from dvc.exceptions import DvcException @@ -26,7 +27,13 @@ def get_mtime_and_size(path): for root, dirs, files in os.walk(str(path)): for name in dirs + files: entry = os.path.join(root, name) - stat = os.stat(entry) + try: + stat = os.stat(entry) + except OSError as exc: + # NOTE: broken symlink case. + if exc.errno != errno.ENOENT: + raise + continue size += stat.st_size entry_mtime = stat.st_mtime if entry_mtime > mtime: diff --git a/tests/requirements.txt b/tests/requirements.txt index 71ad79e5b..fade9c8b1 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -13,3 +13,4 @@ collective.checkdocs black==19.3b0 ; python_version >= '3.6' flake8 flake8-docstrings +jaraco.windows==3.9.2 diff --git a/tests/test_checkout.py b/tests/test_checkout.py index e263c1938..f987f1376 100644 --- a/tests/test_checkout.py +++ b/tests/test_checkout.py @@ -517,3 +517,52 @@ class TestCheckoutRecursiveNotDirectory(TestDvc): with self.assertRaises(TargetNotDirectoryError): self.dvc.checkout(target=self.FOO, recursive=True) + + +class TestCheckoutMovedCacheDirWithSymlinks(TestDvc): + def test(self): + ret = main(["config", "cache.type", "symlink"]) + self.assertEqual(ret, 0) + + ret = main(["add", self.FOO]) + self.assertEqual(ret, 0) + + ret = main(["add", self.DATA_DIR]) + self.assertEqual(ret, 0) + + if os.name == "nt": + from jaraco.windows.filesystem import readlink + else: + readlink = os.readlink + + self.assertTrue(System.is_symlink(self.FOO)) + old_foo_link = readlink(self.FOO) + + self.assertTrue(System.is_symlink(self.DATA)) + old_data_link = readlink(self.DATA) + + old_cache_dir = self.dvc.cache.local.cache_dir + new_cache_dir = old_cache_dir + "_new" + os.rename(old_cache_dir, new_cache_dir) + + ret = main(["cache", "dir", new_cache_dir]) + self.assertEqual(ret, 0) + + ret = main(["checkout", "-f"]) + self.assertEqual(ret, 0) + + self.assertTrue(System.is_symlink(self.FOO)) + new_foo_link = readlink(self.FOO) + + self.assertTrue(System.is_symlink(self.DATA)) + new_data_link = readlink(self.DATA) + + self.assertEqual( + os.path.relpath(old_foo_link, old_cache_dir), + os.path.relpath(new_foo_link, new_cache_dir), + ) + + self.assertEqual( + os.path.relpath(old_data_link, old_cache_dir), + os.path.relpath(new_data_link, new_cache_dir), + ) diff --git a/tests/test_remove.py b/tests/test_remove.py index d0511e1e6..beab5c2b5 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -1,6 +1,8 @@ import os +import shutil from dvc.main import main +from dvc.system import System from dvc.stage import Stage, StageFileDoesNotExistError from dvc.exceptions import DvcException @@ -33,6 +35,24 @@ class TestRemoveNonExistentFile(TestDvc): self.dvc.remove("non_existent_dvc_file") +class TestRemoveBrokenSymlink(TestDvc): + def test(self): + ret = main(["config", "cache.type", "symlink"]) + self.assertEqual(ret, 0) + + ret = main(["add", self.FOO]) + self.assertEqual(ret, 0) + + shutil.rmtree(self.dvc.cache.local.cache_dir) + + self.assertTrue(System.is_symlink(self.FOO)) + + ret = main(["remove", self.FOO + ".dvc"]) + self.assertEqual(ret, 0) + + self.assertFalse(os.path.lexists(self.FOO)) + + class TestRemoveDirectory(TestDvc): def test(self): stages = self.dvc.add(self.DATA_DIR)