Skip to content

outputs: validate that output is not ignored #4386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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