Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-02 07:18:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

dvc: move protect from tree to cache (#5271)

Browse Source
This commit is contained in:
Ruslan Kuprieiev
2021-01-14 21:10:53 +02:00
committed by GitHub
parent a2cac8b423
commit b027760e12
10 changed files with 108 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/cache/base.py
Vendored
+15 -4
View File
@@ -221,7 +221,7 @@ class CloudCache:
path_info
):
# Default relink procedure involves unneeded copy
self.tree.unprotect(path_info)
self.unprotect(path_info)
else:
self.tree.remove(path_info)
self.link(cache_info, path_info)
@@ -281,7 +281,9 @@ class CloudCache:
from_info = PathInfo(tmp)
to_info = self.tree.path_info / tmp_fname("")
self.tree.upload(from_info, to_info, no_progress_bar=True)
self.tree.upload(
from_info, to_info, no_progress_bar=True, file_mode=self.CACHE_MODE
)
hash_info = self.tree.get_file_hash(to_info)
hash_info.value += self.tree.CHECKSUM_DIR_SUFFIX
@@ -373,6 +375,15 @@ class CloudCache:
def hash_to_path(self, hash_):
return self.tree.hash_to_path_info(hash_)
def protect(self, path_info): # pylint: disable=unused-argument
pass
def is_protected(self, path_info): # pylint: disable=unused-argument
return False
def unprotect(self, path_info): # pylint: disable=unused-argument
pass
def changed_cache_file(self, hash_info):
"""Compare the given hash with the (corresponding) actual one.
@@ -387,7 +398,7 @@ class CloudCache:
"""
# Prefer string path over PathInfo when possible due to performance
cache_info = self.hash_to_path(hash_info.value)
if self.tree.is_protected(cache_info):
if self.is_protected(cache_info):
logger.trace(
"Assuming '%s' is unchanged since it is read-only", cache_info
)
@@ -408,7 +419,7 @@ class CloudCache:
if actual.value.split(".")[0] == hash_info.value.split(".")[0]:
# making cache file read-only so we don't need to check it
# next time
self.tree.protect(cache_info)
self.protect(cache_info)
return False
if self.tree.exists(cache_info):
dvc/cache/local.py
Vendored
+57 -3
View File
@@ -2,13 +2,14 @@ import logging
import os
from funcy import cached_property
from shortuuid import uuid
from dvc.hash_info import HashInfo
from dvc.path_info import PathInfo
from dvc.progress import Tqdm
from ..tree.local import LocalTree
from ..utils.fs import walk_files
from ..utils import relpath
from ..utils.fs import copyfile, remove, walk_files
from .base import CloudCache
logger = logging.getLogger(__name__)
@@ -16,7 +17,7 @@ logger = logging.getLogger(__name__)
class LocalCache(CloudCache):
DEFAULT_CACHE_TYPES = ["reflink", "copy"]
CACHE_MODE = LocalTree.CACHE_MODE
CACHE_MODE = 0o444
UNPACKED_DIR_SUFFIX = ".unpacked"
def __init__(self, tree):
@@ -89,3 +90,56 @@ class LocalCache(CloudCache):
info = self.tree.hash_to_path_info(hash_)
path_info = info.with_name(info.name + self.UNPACKED_DIR_SUFFIX)
self.tree.remove(path_info)
def _unprotect_file(self, path):
if self.tree.is_symlink(path) or self.tree.is_hardlink(path):
logger.debug("Unprotecting '%s'", path)
tmp = os.path.join(os.path.dirname(path), "." + uuid())
# The operations order is important here - if some application
# would access the file during the process of copyfile then it
# would get only the part of file. So, at first, the file should be
# copied with the temporary name, and then original file should be
# replaced by new.
copyfile(path, tmp, name="Unprotecting '{}'".format(relpath(path)))
remove(path)
os.rename(tmp, path)
else:
logger.debug(
"Skipping copying for '%s', since it is not "
"a symlink or a hardlink.",
path,
)
os.chmod(path, self.tree.file_mode)
def _unprotect_dir(self, path):
for fname in self.tree.walk_files(path):
self._unprotect_file(fname)
def unprotect(self, path_info):
if not os.path.exists(path_info):
from dvc.exceptions import DvcException
raise DvcException(
f"can't unprotect non-existing data '{path_info}'"
)
if os.path.isdir(path_info):
self._unprotect_dir(path_info)
else:
self._unprotect_file(path_info)
def protect(self, path_info):
self.tree.chmod(path_info, self.CACHE_MODE)
def is_protected(self, path_info):
import stat
try:
mode = os.stat(path_info).st_mode
except FileNotFoundError:
return False
return stat.S_IMODE(mode) == self.CACHE_MODE
dvc/output/base.py
+1 -1
View File
@@ -430,7 +430,7 @@ class BaseOutput:
def unprotect(self):
if self.exists:
self.tree.unprotect(self.path_info)
self.cache.unprotect(self.path_info)
def get_dir_cache(self, **kwargs):
if not self.is_dir_checksum:
dvc/remote/base.py
+11 -2
View File
@@ -309,15 +309,24 @@ class Remote:
)
if download:
# NOTE: don't set CACHE_MODE if we don't trust the remote, so that
# cache could verify these files by-hand later.
if self.tree.verify:
mode = self.tree.file_mode
else:
mode = cache.CACHE_MODE
func = partial(
_log_exceptions(self.tree.download, "download"),
dir_mode=cache.tree.dir_mode,
file_mode=cache.tree.file_mode,
file_mode=mode,
)
status = STATUS_DELETED
desc = "Downloading"
else:
func = _log_exceptions(self.tree.upload, "upload")
func = partial(
_log_exceptions(self.tree.upload, "upload"),
file_mode=self.cache.CACHE_MODE,
)
status = STATUS_NEW
desc = "Uploading"
dvc/repo/__init__.py
+1 -1
View File
@@ -267,7 +267,7 @@ class Repo:
return Repo(root_dir)
def unprotect(self, target):
return self.cache.local.tree.unprotect(PathInfo(target))
return self.cache.local.unprotect(PathInfo(target))
def _ignore(self):
flist = [
dvc/tree/base.py
-12
View File
@@ -60,7 +60,6 @@ class BaseTree:
TRAVERSE_THRESHOLD_SIZE = 500000
CAN_TRAVERSE = True
CACHE_MODE: Optional[int] = None
SHARED_MODE_MAP = {None: (None, None), "group": (None, None)}
PARAM_CHECKSUM: ClassVar[Optional[str]] = None
@@ -251,19 +250,8 @@ class BaseTree:
def reflink(self, from_info, to_info):
raise RemoteActionNotImplemented("reflink", self.scheme)
@staticmethod
def protect(path_info):
pass
def is_protected(self, path_info):
return False
# pylint: enable=unused-argument
@staticmethod
def unprotect(path_info):
pass
@classmethod
def is_dir_hash(cls, hash_):
if not hash_:
dvc/tree/local.py
+1 -55
View File
@@ -5,14 +5,12 @@ import stat
from typing import Any, Dict
from funcy import cached_property
from shortuuid import uuid
from dvc.exceptions import DvcException
from dvc.hash_info import HashInfo
from dvc.path_info import PathInfo
from dvc.scheme import Schemes
from dvc.system import System
from dvc.utils import file_md5, is_exec, relpath, tmp_fname
from dvc.utils import file_md5, is_exec, tmp_fname
from dvc.utils.fs import copy_fobj_to_file, copyfile, makedirs, move, remove
from .base import BaseTree
@@ -27,7 +25,6 @@ class LocalTree(BaseTree):
PARAM_PATH = "path"
TRAVERSE_PREFIX_LEN = 2
CACHE_MODE = 0o444
SHARED_MODE_MAP: Dict[Any, Any] = {
None: (0o644, 0o755),
"group": (0o664, 0o775),
@@ -253,55 +250,6 @@ class LocalTree(BaseTree):
if actual != mode:
raise
def _unprotect_file(self, path):
if System.is_symlink(path) or System.is_hardlink(path):
logger.debug("Unprotecting '%s'", path)
tmp = os.path.join(os.path.dirname(path), "." + uuid())
# The operations order is important here - if some application
# would access the file during the process of copyfile then it
# would get only the part of file. So, at first, the file should be
# copied with the temporary name, and then original file should be
# replaced by new.
copyfile(path, tmp, name="Unprotecting '{}'".format(relpath(path)))
remove(path)
os.rename(tmp, path)
else:
logger.debug(
"Skipping copying for '%s', since it is not "
"a symlink or a hardlink.",
path,
)
os.chmod(path, self.file_mode)
def _unprotect_dir(self, path):
for fname in self.walk_files(path):
self._unprotect_file(fname)
def unprotect(self, path_info):
if not os.path.exists(path_info):
raise DvcException(
f"can't unprotect non-existing data '{path_info}'"
)
if os.path.isdir(path_info):
self._unprotect_dir(path_info)
else:
self._unprotect_file(path_info)
def protect(self, path_info):
self.chmod(path_info, self.CACHE_MODE)
def is_protected(self, path_info):
try:
mode = os.stat(path_info).st_mode
except FileNotFoundError:
return False
return stat.S_IMODE(mode) == self.CACHE_MODE
def get_file_hash(self, path_info):
hash_info = HashInfo(self.PARAM_CHECKSUM, file_md5(path_info)[0],)
@@ -332,8 +280,6 @@ class LocalTree(BaseTree):
if file_mode is not None:
self.chmod(tmp_file, file_mode)
else:
self.protect(tmp_file)
os.replace(tmp_file, to_info)
@staticmethod
tests/func/test_add.py
+1 -1
View File
@@ -714,7 +714,7 @@ def test_readding_dir_should_not_unprotect_all(tmp_dir, dvc, mocker):
dvc.add("dir")
tmp_dir.gen("dir/new_file", "new_file_content")
unprotect_spy = mocker.spy(LocalTree, "unprotect")
unprotect_spy = mocker.spy(dvc.cache.local, "unprotect")
dvc.add("dir")
assert not unprotect_spy.mock.called
tests/unit/remote/test_local.py → tests/unit/cache/test_local.py
Vendored
+16 -17
View File
@@ -36,7 +36,8 @@ def test_status_download_optimization(mocker, dvc):
@pytest.mark.parametrize("link_name", ["hardlink", "symlink"])
def test_is_protected(tmp_dir, dvc, link_name):
tree = LocalTree(dvc, {})
cache = dvc.cache.local
tree = cache.tree
link_method = getattr(tree, link_name)
(tmp_dir / "foo").write_text("foo")
@@ -46,47 +47,45 @@ def test_is_protected(tmp_dir, dvc, link_name):
link_method(foo, link)
assert not tree.is_protected(foo)
assert not tree.is_protected(link)
assert not cache.is_protected(foo)
assert not cache.is_protected(link)
tree.protect(foo)
cache.protect(foo)
assert tree.is_protected(foo)
assert tree.is_protected(link)
assert cache.is_protected(foo)
assert cache.is_protected(link)
tree.unprotect(link)
cache.unprotect(link)
assert not tree.is_protected(link)
assert not cache.is_protected(link)
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 tree.is_protected(foo)
assert not cache.is_protected(foo)
else:
assert tree.is_protected(foo)
assert cache.is_protected(foo)
@pytest.mark.parametrize("err", [errno.EPERM, errno.EACCES])
def test_protect_ignore_errors(tmp_dir, mocker, err):
def test_protect_ignore_errors(tmp_dir, dvc, mocker, err):
tmp_dir.gen("foo", "foo")
foo = PathInfo("foo")
tree = LocalTree(None, {})
tree.protect(foo)
dvc.cache.local.protect(foo)
mock_chmod = mocker.patch(
"os.chmod", side_effect=OSError(err, "something")
)
tree.protect(foo)
dvc.cache.local.protect(foo)
assert mock_chmod.called
def test_protect_ignore_erofs(tmp_dir, mocker):
def test_protect_ignore_erofs(tmp_dir, dvc, mocker):
tmp_dir.gen("foo", "foo")
foo = PathInfo("foo")
tree = LocalTree(None, {})
mock_chmod = mocker.patch(
"os.chmod", side_effect=OSError(errno.EROFS, "read-only fs")
)
tree.protect(foo)
dvc.cache.local.protect(foo)
assert mock_chmod.called
tests/unit/tree/test_tree.py
+5 -1
View File
@@ -53,11 +53,15 @@ def test_get_cloud_tree_validate(tmp_dir, dvc):
@pytest.mark.skipif(os.name == "nt", reason="Not supported for Windows.")
@pytest.mark.parametrize(
"mode, expected", [(None, LocalTree.CACHE_MODE), (0o777, 0o777)],
"mode, expected", [(None, None), (0o777, 0o777)],
)
def test_upload_file_mode(tmp_dir, mode, expected):
tmp_dir.gen("src", "foo")
src = PathInfo(tmp_dir / "src")
if not expected:
expected = stat.S_IMODE((tmp_dir / "src").stat().st_mode)
dest = PathInfo(tmp_dir / "dest")
tree = LocalTree(None, {"url": os.fspath(tmp_dir)})
tree.upload(src, dest, file_mode=mode)