Merge pull request #2493 from pared/2345

output: prevent stage file from being used as output

dvc/dependency/base.py

@@ -15,11 +15,19 @@ class DependencyIsNotFileOrDirError(DvcException):
         super(DependencyIsNotFileOrDirError, self).__init__(msg)
 
 
+class DependencyIsStageFileError(DvcException):
+    def __init__(self, path):
+        super(DependencyIsStageFileError, self).__init__(
+            "Stage file '{}' cannot be a dependency.".format(path)
+        )
+
+
 class DependencyBase(object):
     IS_DEPENDENCY = True
 
     DoesNotExistError = DependencyDoesNotExistError
     IsNotFileOrDirError = DependencyIsNotFileOrDirError
+    IsStageFileError = DependencyIsStageFileError
 
     def update(self):
         pass

dvc/output/base.py

@@ -32,6 +32,13 @@ class OutputAlreadyTrackedError(DvcException):
         super(OutputAlreadyTrackedError, self).__init__(msg)
 
 
+class OutputIsStageFileError(DvcException):
+    def __init__(self, path):
+        super(OutputIsStageFileError, self).__init__(
+            "Stage file '{}' cannot be an 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 @@ class OutputBase(object):
         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 @@ class OutputBase(object):
         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)

tests/func/test_run.py

@@ -7,8 +7,10 @@ import shutil
 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,21 @@ 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):
+    with pytest.raises(DependencyIsStageFileError):
+        dvc_repo.run(
+            cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"),
+            deps=[repo_dir.FOO, "name.dvc"],
+            outs=["out"],
+        )
+
+
+def test_should_raise_on_stage_output(repo_dir, dvc_repo):
+    with pytest.raises(OutputIsStageFileError):
+        dvc_repo.run(
+            cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "out"),
+            deps=[repo_dir.FOO],
+            outs=["name.dvc"],
+        )

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])