From aca6d229da0f7fa5246e5ca0fa00d250e298e715 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 27 Jan 2022 18:16:17 +0200 Subject: [PATCH] output: move isexec to meta With our staging process automatically capturing file metadata, we no longer need to set isexec ourselves. In followup PRs we will also use meta during checkout, so that we won't need to use Output.set_exec by hand. --- dvc/data/meta.py | 9 ++++++++- dvc/data/stage.py | 4 ++-- dvc/fs/base.py | 6 ------ dvc/fs/dvc.py | 3 --- dvc/fs/git.py | 8 -------- dvc/fs/local.py | 6 +----- dvc/fs/repo.py | 6 ------ dvc/output.py | 15 ++------------- dvc/schema.py | 2 +- dvc/stage/loader.py | 1 - dvc/stage/serialize.py | 3 --- dvc/stage/utils.py | 2 +- tests/unit/stage/test_loader_pipeline_file.py | 4 ++-- tests/unit/stage/test_serialize_pipeline_lock.py | 2 +- 14 files changed, 18 insertions(+), 53 deletions(-) diff --git a/dvc/data/meta.py b/dvc/data/meta.py index ef3930e08..0284286a6 100644 --- a/dvc/data/meta.py +++ b/dvc/data/meta.py @@ -7,9 +7,11 @@ from typing import Optional class Meta: PARAM_SIZE = "size" PARAM_NFILES = "nfiles" + PARAM_ISEXEC = "isexec" size: Optional[int] = field(default=None) nfiles: Optional[int] = field(default=None) + isexec: Optional[bool] = field(default=False) @classmethod def from_dict(cls, d): @@ -18,7 +20,9 @@ class Meta: size = d.pop(cls.PARAM_SIZE, None) nfiles = d.pop(cls.PARAM_NFILES, None) - return cls(size=size, nfiles=nfiles) + isexec = d.pop(cls.PARAM_ISEXEC, False) + + return cls(size=size, nfiles=nfiles, isexec=isexec) def to_dict(self): ret = OrderedDict() @@ -29,4 +33,7 @@ class Meta: if self.nfiles is not None: ret[self.PARAM_NFILES] = self.nfiles + if self.isexec: + ret[self.PARAM_ISEXEC] = self.isexec + return ret diff --git a/dvc/data/stage.py b/dvc/data/stage.py index a34ce6250..992ad8437 100644 --- a/dvc/data/stage.py +++ b/dvc/data/stage.py @@ -10,7 +10,7 @@ from dvc.hash_info import HashInfo from dvc.ignore import DvcIgnore from dvc.objects.file import HashFile from dvc.progress import Tqdm -from dvc.utils import file_md5 +from dvc.utils import file_md5, is_exec from .db.reference import ReferenceObjectDB from .meta import Meta @@ -72,7 +72,7 @@ def _get_file_hash(fs_path, fs, name): else: raise NotImplementedError - meta = Meta(size=info["size"]) + meta = Meta(size=info["size"], isexec=is_exec(info.get("mode", 0))) hash_info = HashInfo(name, hash_value) return meta, hash_info diff --git a/dvc/fs/base.py b/dvc/fs/base.py index edcf1207d..3e7e6dfe0 100644 --- a/dvc/fs/base.py +++ b/dvc/fs/base.py @@ -150,12 +150,6 @@ class FileSystem: """ return True - def isexec(self, path): - """Optional: Overwrite only if the remote has a way to distinguish - between executable and non-executable file. - """ - return False - def iscopy(self, path): """Check if this file is an independent copy.""" return False # We can't be sure by default diff --git a/dvc/fs/dvc.py b/dvc/fs/dvc.py index 9d0c229af..832e490c1 100644 --- a/dvc/fs/dvc.py +++ b/dvc/fs/dvc.py @@ -227,9 +227,6 @@ class DvcFileSystem(FileSystem): # pylint:disable=abstract-method recurse = recursive or not strict return meta.output_exists if recurse else meta.is_output - def isexec(self, path): # pylint: disable=unused-argument - return False - def metadata(self, fs_path): abspath = os.path.abspath(fs_path) diff --git a/dvc/fs/git.py b/dvc/fs/git.py index 9443ce8d8..1326fa1fc 100644 --- a/dvc/fs/git.py +++ b/dvc/fs/git.py @@ -51,14 +51,6 @@ class GitFileSystem(FSSpecWrapper): # pylint:disable=abstract-method def rev(self) -> str: return self.fs.rev - def isexec(self, path: AnyFSPath) -> bool: - from dvc.utils import is_exec - - try: - return is_exec(self.info(path)["mode"]) - except FileNotFoundError: - return False - def isfile(self, path: AnyFSPath) -> bool: return self.fs.isfile(path) diff --git a/dvc/fs/local.py b/dvc/fs/local.py index 9c7a7cb90..7024aed2a 100644 --- a/dvc/fs/local.py +++ b/dvc/fs/local.py @@ -3,7 +3,7 @@ import os from dvc.scheme import Schemes from dvc.system import System -from dvc.utils import is_exec, tmp_fname +from dvc.utils import tmp_fname from dvc.utils.fs import copy_fobj_to_file, copyfile, makedirs, move, remove from ._callback import DEFAULT_CALLBACK @@ -83,10 +83,6 @@ class LocalFileSystem(FileSystem): def makedirs(self, path, **kwargs): makedirs(path, exist_ok=kwargs.pop("exist_ok", True)) - def isexec(self, path): - mode = self.info(path)["mode"] - return is_exec(mode) - def move(self, from_info, to_info): self.makedirs(self.path.parent(to_info)) move(from_info, to_info) diff --git a/dvc/fs/repo.py b/dvc/fs/repo.py index 9b2770676..2f38b2cbe 100644 --- a/dvc/fs/repo.py +++ b/dvc/fs/repo.py @@ -302,12 +302,6 @@ class RepoFileSystem(FileSystem): # pylint:disable=abstract-method return False return meta.isfile - def isexec(self, path): - fs, dvc_fs = self._get_fs_pair(path) - if dvc_fs and dvc_fs.exists(path): - return dvc_fs.isexec(path) - return fs.isexec(path) - def _dvc_walk(self, walk): try: root, dirs, files = next(walk) diff --git a/dvc/output.py b/dvc/output.py index 87bd4fa47..13fd8e737 100644 --- a/dvc/output.py +++ b/dvc/output.py @@ -81,7 +81,6 @@ def loadd_from(stage, d_list): persist = d.pop(Output.PARAM_PERSIST, False) checkpoint = d.pop(Output.PARAM_CHECKPOINT, False) desc = d.pop(Output.PARAM_DESC, False) - isexec = d.pop(Output.PARAM_ISEXEC, False) live = d.pop(Output.PARAM_LIVE, False) remote = d.pop(Output.PARAM_REMOTE, None) ret.append( @@ -95,7 +94,6 @@ def loadd_from(stage, d_list): persist=persist, checkpoint=checkpoint, desc=desc, - isexec=isexec, live=live, remote=remote, ) @@ -111,7 +109,6 @@ def loads_from( plot=False, persist=False, checkpoint=False, - isexec=False, live=False, remote=None, ): @@ -125,7 +122,6 @@ def loads_from( plot=plot, persist=persist, checkpoint=checkpoint, - isexec=isexec, live=live, remote=remote, ) @@ -257,7 +253,6 @@ class Output: PARAM_PLOT_HEADER = "header" PARAM_PERSIST = "persist" PARAM_DESC = "desc" - PARAM_ISEXEC = "isexec" PARAM_LIVE = "live" PARAM_LIVE_SUMMARY = "summary" PARAM_LIVE_HTML = "html" @@ -289,7 +284,6 @@ class Output: checkpoint=False, live=False, desc=None, - isexec=False, remote=None, repo=None, ): @@ -329,7 +323,6 @@ class Output: self.fs_path = self._parse_path(self.fs, fs_path) self.obj = None - self.isexec = False if self.IS_DEPENDENCY else isexec self.remote = remote @@ -566,10 +559,9 @@ class Output: dvcignore=self.dvcignore, ) self.hash_info = self.obj.hash_info - self.isexec = self.isfile() and self.fs.isexec(self.fs_path) def set_exec(self): - if self.isfile() and self.isexec: + if self.isfile() and self.meta.isexec: self.odb.set_exec(self.fs_path) def commit(self, filter_info=None): @@ -674,9 +666,6 @@ class Output: if self.checkpoint: ret[self.PARAM_CHECKPOINT] = self.checkpoint - if self.isexec: - ret[self.PARAM_ISEXEC] = self.isexec - if self.live: ret[self.PARAM_LIVE] = self.live @@ -1063,7 +1052,7 @@ ARTIFACT_SCHEMA = { Output.PARAM_CHECKPOINT: bool, Meta.PARAM_SIZE: int, Meta.PARAM_NFILES: int, - Output.PARAM_ISEXEC: bool, + Meta.PARAM_ISEXEC: bool, } SCHEMA = { diff --git a/dvc/schema.py b/dvc/schema.py index 5f437374f..dd3e37cdb 100644 --- a/dvc/schema.py +++ b/dvc/schema.py @@ -28,7 +28,7 @@ DATA_SCHEMA = { Required("path"): str, Meta.PARAM_SIZE: int, Meta.PARAM_NFILES: int, - Output.PARAM_ISEXEC: bool, + Meta.PARAM_ISEXEC: bool, } LOCK_FILE_STAGE_SCHEMA = { Required(StageParams.PARAM_CMD): Any(str, list), diff --git a/dvc/stage/loader.py b/dvc/stage/loader.py index f5959c35c..2743d720a 100644 --- a/dvc/stage/loader.py +++ b/dvc/stage/loader.py @@ -68,7 +68,6 @@ class StageLoader(Mapping): info = get_in(checksums, [key, path], {}) info = info.copy() info.pop("path", None) - item.isexec = info.pop("isexec", None) item.meta = Meta.from_dict(info) item.hash_info = HashInfo.from_dict(info) diff --git a/dvc/stage/serialize.py b/dvc/stage/serialize.py index 606d5943c..bb6d556df 100644 --- a/dvc/stage/serialize.py +++ b/dvc/stage/serialize.py @@ -167,9 +167,6 @@ def to_single_stage_lockfile(stage: "Stage") -> dict: *item.meta.to_dict().items(), ] - if item.isexec: - ret.append((item.PARAM_ISEXEC, True)) - return OrderedDict(ret) res = OrderedDict([("cmd", stage.cmd)]) diff --git a/dvc/stage/utils.py b/dvc/stage/utils.py index fbba5fb28..a48b8ef9d 100644 --- a/dvc/stage/utils.py +++ b/dvc/stage/utils.py @@ -196,7 +196,7 @@ def compute_md5(stage): Output.PARAM_METRIC, Output.PARAM_PERSIST, Output.PARAM_CHECKPOINT, - Output.PARAM_ISEXEC, + Meta.PARAM_ISEXEC, Meta.PARAM_SIZE, Meta.PARAM_NFILES, ], diff --git a/tests/unit/stage/test_loader_pipeline_file.py b/tests/unit/stage/test_loader_pipeline_file.py index 2538cbbdc..84847ed2d 100644 --- a/tests/unit/stage/test_loader_pipeline_file.py +++ b/tests/unit/stage/test_loader_pipeline_file.py @@ -42,7 +42,7 @@ def test_fill_from_lock_deps_outs(dvc, lock_data): def test_fill_from_lock_outs_isexec(dvc): stage = create_stage(PipelineStage, dvc, PIPELINE_FILE, outs=["foo"]) - assert not stage.outs[0].isexec + assert not stage.outs[0].meta.isexec StageLoader.fill_from_lock( stage, @@ -54,7 +54,7 @@ def test_fill_from_lock_outs_isexec(dvc): assert stage.outs[0].def_path == "foo" assert stage.outs[0].hash_info == HashInfo("md5", "foo_checksum") - assert stage.outs[0].isexec + assert stage.outs[0].meta.isexec def test_fill_from_lock_params(dvc, lock_data): diff --git a/tests/unit/stage/test_serialize_pipeline_lock.py b/tests/unit/stage/test_serialize_pipeline_lock.py index 18d14a9e7..968b3183f 100644 --- a/tests/unit/stage/test_serialize_pipeline_lock.py +++ b/tests/unit/stage/test_serialize_pipeline_lock.py @@ -136,7 +136,7 @@ def test_lock_outs(dvc, typ): def test_lock_outs_isexec(dvc, typ): stage = create_stage(PipelineStage, dvc, **{typ: ["input"]}, **kwargs) stage.outs[0].hash_info = HashInfo("md5", "md-five") - stage.outs[0].isexec = True + stage.outs[0].meta.isexec = True assert to_single_stage_lockfile(stage) == OrderedDict( [ ("cmd", "command"),