Skip to content

Commit

Permalink
outputs: validate that output is not ignored (iterative#4386)
Browse files Browse the repository at this point in the history
  • Loading branch information
pared authored Aug 14, 2020
1 parent 0092502 commit 35f125f
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 5 deletions.
10 changes: 7 additions & 3 deletions dvc/ignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ def __bool__(self):
)


def _no_match(path):
return CheckIgnoreResult(path, False, ["::"])


class DvcIgnoreFilterNoop:
def __init__(self, tree, root_dir):
pass
Expand All @@ -165,8 +169,8 @@ def is_ignored_dir(self, _):
def is_ignored_file(self, _):
return False

def check_ignore(self, _):
return []
def check_ignore(self, path):
return _no_match(path)


class DvcIgnoreFilter:
Expand Down Expand Up @@ -325,7 +329,7 @@ def check_ignore(self, target):

if matches:
return CheckIgnoreResult(target, True, matches)
return CheckIgnoreResult(target, False, ["::"])
return _no_match(target)


def init(path):
Expand Down
16 changes: 14 additions & 2 deletions dvc/output/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ def __init__(self, path):
super().__init__(f"Stage file '{path}' cannot be an output.")


class OutputIsIgnoredError(DvcException):
def __init__(self, match):
lines = "\n".join(match.patterns)
super().__init__(f"Path '{match.file}' is ignored by\n{lines}")


class BaseOutput:
IS_DEPENDENCY = False

Expand Down Expand Up @@ -77,6 +83,7 @@ class BaseOutput:
DoesNotExistError = OutputDoesNotExistError
IsNotFileOrDirError = OutputIsNotFileOrDirError
IsStageFileError = OutputIsStageFileError
IsIgnoredError = OutputIsIgnoredError

sep = "/"

Expand All @@ -91,7 +98,7 @@ def __init__(
plot=False,
persist=False,
):
self._validate_output_path(path)
self._validate_output_path(path, stage)
# This output (and dependency) objects have too many paths/urls
# here is a list and comments:
#
Expand Down Expand Up @@ -499,8 +506,13 @@ def get_used_cache(self, **kwargs):
return ret

@classmethod
def _validate_output_path(cls, path):
def _validate_output_path(cls, path, stage=None):
from dvc.dvcfile import is_valid_filename

if is_valid_filename(path):
raise cls.IsStageFileError(path)

if stage:
check = stage.repo.tree.dvcignore.check_ignore(path)
if check.match:
raise cls.IsIgnoredError(check)
8 changes: 8 additions & 0 deletions tests/func/test_ignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from dvc.exceptions import DvcIgnoreInCollectedDirError
from dvc.ignore import DvcIgnore, DvcIgnorePatterns
from dvc.output.base import OutputIsIgnoredError
from dvc.path_info import PathInfo
from dvc.pathspec_math import PatternInfo, merge_patterns
from dvc.repo import Repo
Expand Down Expand Up @@ -400,3 +401,10 @@ def test_ignore_in_added_dir(tmp_dir, dvc):
dvc.checkout()

assert not ignored_path.exists()


def test_ignored_output(tmp_dir, scm, dvc, run_copy):
tmp_dir.gen({".dvcignore": "*.log", "foo": "foo content"})

with pytest.raises(OutputIsIgnoredError):
run_copy("foo", "foo.log", name="copy")
6 changes: 6 additions & 0 deletions tests/unit/output/test_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from voluptuous import MultipleInvalid, Schema

from dvc.cache import NamedCache
from dvc.ignore import _no_match
from dvc.output import CHECKSUM_SCHEMA, BaseOutput


Expand Down Expand Up @@ -72,6 +73,11 @@ def test_get_used_cache(exists, expected_message, mocker, caplog):
stage = mocker.MagicMock()
mocker.patch.object(stage, "__str__", return_value="stage: 'stage.dvc'")
mocker.patch.object(stage, "addressing", "stage.dvc")
mocker.patch.object(
stage.repo.tree.dvcignore,
"check_ignore",
return_value=_no_match("path"),
)

output = BaseOutput(stage, "path")

Expand Down

0 comments on commit 35f125f

Please sign in to comment.