From 35f125f3dfd23aebd499f8b8b9de123293a44ccf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= <pawelredzynski@gmail.com>
Date: Fri, 14 Aug 2020 13:30:12 +0200
Subject: [PATCH] outputs: validate that output is not ignored (#4386)

---
 dvc/ignore.py                    | 10 +++++++---
 dvc/output/base.py               | 16 ++++++++++++++--
 tests/func/test_ignore.py        |  8 ++++++++
 tests/unit/output/test_output.py |  6 ++++++
 4 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/dvc/ignore.py b/dvc/ignore.py
index ea93db805..1c7f731ba 100644
--- a/dvc/ignore.py
+++ b/dvc/ignore.py
@@ -152,6 +152,10 @@ CheckIgnoreResult = namedtuple(
 )
 
 
+def _no_match(path):
+    return CheckIgnoreResult(path, False, ["::"])
+
+
 class DvcIgnoreFilterNoop:
     def __init__(self, tree, root_dir):
         pass
@@ -165,8 +169,8 @@ class DvcIgnoreFilterNoop:
     def is_ignored_file(self, _):
         return False
 
-    def check_ignore(self, _):
-        return []
+    def check_ignore(self, path):
+        return _no_match(path)
 
 
 class DvcIgnoreFilter:
@@ -325,7 +329,7 @@ class DvcIgnoreFilter:
 
                 if matches:
                     return CheckIgnoreResult(target, True, matches)
-        return CheckIgnoreResult(target, False, ["::"])
+        return _no_match(target)
 
 
 def init(path):
diff --git a/dvc/output/base.py b/dvc/output/base.py
index f17e3df9e..bdaded877 100644
--- a/dvc/output/base.py
+++ b/dvc/output/base.py
@@ -45,6 +45,12 @@ class OutputIsStageFileError(DvcException):
         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
 
@@ -77,6 +83,7 @@ class BaseOutput:
     DoesNotExistError = OutputDoesNotExistError
     IsNotFileOrDirError = OutputIsNotFileOrDirError
     IsStageFileError = OutputIsStageFileError
+    IsIgnoredError = OutputIsIgnoredError
 
     sep = "/"
 
@@ -91,7 +98,7 @@ class BaseOutput:
         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:
         #
@@ -499,8 +506,13 @@ class BaseOutput:
         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)
diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py
index b68f0f4f7..131295203 100644
--- a/tests/func/test_ignore.py
+++ b/tests/func/test_ignore.py
@@ -5,6 +5,7 @@ import pytest
 
 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
@@ -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")
diff --git a/tests/unit/output/test_output.py b/tests/unit/output/test_output.py
index c8ffc50f7..4f2e69e88 100644
--- a/tests/unit/output/test_output.py
+++ b/tests/unit/output/test_output.py
@@ -5,6 +5,7 @@ from funcy import first
 from voluptuous import MultipleInvalid, Schema
 
 from dvc.cache import NamedCache
+from dvc.ignore import _no_match
 from dvc.output import CHECKSUM_SCHEMA, BaseOutput
 
 
@@ -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")