From 0935930f46c3399ad7b1c4b61a5fd7296a3b4ba5 Mon Sep 17 00:00:00 2001 From: pawel Date: Thu, 12 Sep 2019 15:32:32 +0200 Subject: [PATCH 1/3] output: prevent stage file from being used as output --- dvc/dependency/base.py | 8 ++++++++ dvc/output/base.py | 16 ++++++++++++++++ tests/func/test_run.py | 24 ++++++++++++++++++++++++ tests/unit/test_stage.py | 2 +- 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/dvc/dependency/base.py b/dvc/dependency/base.py index f76f4e022e..3fba3960d5 100644 --- a/dvc/dependency/base.py +++ b/dvc/dependency/base.py @@ -15,11 +15,19 @@ def __init__(self, path): super(DependencyIsNotFileOrDirError, self).__init__(msg) +class DependencyIsStageFileError(DvcException): + def __init__(self, path): + super(DependencyIsStageFileError, self).__init__( + "Stage file '{}' cannot be dependency.".format(path) + ) + + class DependencyBase(object): IS_DEPENDENCY = True DoesNotExistError = DependencyDoesNotExistError IsNotFileOrDirError = DependencyIsNotFileOrDirError + IsStageFileError = DependencyIsStageFileError def update(self): pass diff --git a/dvc/output/base.py b/dvc/output/base.py index 48fd5bebbd..5a66efb99e 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -32,6 +32,13 @@ def __init__(self, path): super(OutputAlreadyTrackedError, self).__init__(msg) +class OutputIsStageFileError(DvcException): + def __init__(self, path): + super(OutputIsStageFileError, self).__init__( + "Stage file '{}' cannot be output.".format(path) + ) + + class OutputBase(object): IS_DEPENDENCY = False @@ -57,6 +64,7 @@ class OutputBase(object): DoesNotExistError = OutputDoesNotExistError IsNotFileOrDirError = OutputIsNotFileOrDirError + IsStageFileError = OutputIsStageFileError sep = "/" @@ -71,6 +79,7 @@ def __init__( persist=False, tags=None, ): + self._validate_output_path(path) # This output (and dependency) objects have too many paths/urls # here is a list and comments: # @@ -415,3 +424,10 @@ def get_used_cache(self, **kwargs): ret.extend(self._collect_used_dir_cache(**kwargs)) return ret + + @classmethod + def _validate_output_path(cls, path): + from dvc.stage import Stage + + if Stage.is_valid_filename(path): + raise cls.IsStageFileError(path) diff --git a/tests/func/test_run.py b/tests/func/test_run.py index 567f30be04..d4abed8948 100644 --- a/tests/func/test_run.py +++ b/tests/func/test_run.py @@ -7,8 +7,10 @@ import filecmp import pytest +from dvc.dependency.base import DependencyIsStageFileError from dvc.main import main from dvc.output import OutputBase +from dvc.output.base import OutputIsStageFileError from dvc.repo import Repo as DvcRepo from dvc.utils import file_md5 from dvc.utils.stage import load_stage_file @@ -941,3 +943,25 @@ def test_bad_stage_fname(repo_dir, dvc_repo): # Check that command hasn't been run assert not os.path.exists("out") + + +def test_should_raise_on_stage_dependency(repo_dir, dvc_repo): + stage_file_name = "name.dvc" + + with pytest.raises(DependencyIsStageFileError): + dvc_repo.run( + cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"), + deps=[repo_dir.FOO, stage_file_name], + outs=["out"], + ) + + +def test_should_raise_on_stage_output(repo_dir, dvc_repo): + stage_file_name = "name.dvc" + + with pytest.raises(OutputIsStageFileError): + dvc_repo.run( + cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"), + deps=[repo_dir.FOO], + outs=[stage_file_name], + ) diff --git a/tests/unit/test_stage.py b/tests/unit/test_stage.py index f87fbe0f66..d805a5d6f6 100644 --- a/tests/unit/test_stage.py +++ b/tests/unit/test_stage.py @@ -70,7 +70,7 @@ def test_stage_fname(add): def test_stage_update(mocker): - dep = DependencyREPO({"url": "example.com"}, None, None) + dep = DependencyREPO({"url": "example.com"}, None, "dep_path") mocker.patch.object(dep, "update", return_value=None) stage = Stage(None, "path", deps=[dep]) From 74a0cef3e52360059f2d77a16a5379fd29c97ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Fri, 13 Sep 2019 16:02:49 +0200 Subject: [PATCH 2/3] Update dvc/output/base.py Co-Authored-By: Ruslan Kuprieiev --- dvc/output/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/output/base.py b/dvc/output/base.py index 5a66efb99e..c478f6190f 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -35,7 +35,7 @@ def __init__(self, path): class OutputIsStageFileError(DvcException): def __init__(self, path): super(OutputIsStageFileError, self).__init__( - "Stage file '{}' cannot be output.".format(path) + "Stage file '{}' cannot be an output.".format(path) ) From 53f6aead951735a92451f08f5543e65960f416ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Fri, 13 Sep 2019 16:03:00 +0200 Subject: [PATCH 3/3] Update dvc/dependency/base.py Co-Authored-By: Ruslan Kuprieiev --- dvc/dependency/base.py | 2 +- tests/func/test_run.py | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/dvc/dependency/base.py b/dvc/dependency/base.py index 3fba3960d5..4f52bf5d05 100644 --- a/dvc/dependency/base.py +++ b/dvc/dependency/base.py @@ -18,7 +18,7 @@ def __init__(self, path): class DependencyIsStageFileError(DvcException): def __init__(self, path): super(DependencyIsStageFileError, self).__init__( - "Stage file '{}' cannot be dependency.".format(path) + "Stage file '{}' cannot be a dependency.".format(path) ) diff --git a/tests/func/test_run.py b/tests/func/test_run.py index d4abed8948..148bb88535 100644 --- a/tests/func/test_run.py +++ b/tests/func/test_run.py @@ -946,22 +946,18 @@ def test_bad_stage_fname(repo_dir, dvc_repo): def test_should_raise_on_stage_dependency(repo_dir, dvc_repo): - stage_file_name = "name.dvc" - with pytest.raises(DependencyIsStageFileError): dvc_repo.run( cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"), - deps=[repo_dir.FOO, stage_file_name], + deps=[repo_dir.FOO, "name.dvc"], outs=["out"], ) def test_should_raise_on_stage_output(repo_dir, dvc_repo): - stage_file_name = "name.dvc" - with pytest.raises(OutputIsStageFileError): dvc_repo.run( cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"), deps=[repo_dir.FOO], - outs=[stage_file_name], + outs=["name.dvc"], )