From 2b3107f72c33c0ea8345bde8bf701a89574e2ec3 Mon Sep 17 00:00:00 2001 From: pared Date: Fri, 22 Mar 2019 15:23:22 +0100 Subject: [PATCH] Check whether outputs in graphs do not overlap --- dvc/exceptions.py | 14 ++++++++++++++ dvc/repo/__init__.py | 11 ++++++++++- tests/test_repro.py | 10 ++++++---- tests/test_run.py | 37 +++++++++++++++++++++++++++++++------ 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 1729f4a0d..0a1df2a6e 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -227,3 +227,17 @@ class RecursiveAddingWhileUsingFilename(DvcException): super(RecursiveAddingWhileUsingFilename, self).__init__( "using fname with recursive is not allowed." ) + + +class OverlappingOutputPathsError(DvcException): + def __init__(self, out_1, out_2): + super(OverlappingOutputPathsError, self).__init__( + "Paths for outs:\n'{}'('{}')\n'{}'('{}')\noverlap. To avoid " + "unpredictable behaviour, rerun command with non overlapping outs " + "paths.".format( + str(out_1), + out_1.stage.relpath, + str(out_2), + out_2.stage.relpath, + ) + ) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index f758609fe..5fe31d1aa 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -283,6 +283,7 @@ class Repo(object): from dvc.exceptions import ( OutputDuplicationError, StagePathAsOutputError, + OverlappingOutputPathsError, ) G = nx.DiGraph() @@ -293,7 +294,15 @@ class Repo(object): for stage in stages: for out in stage.outs: - existing = [o.stage for o in outs if o.path == out.path] + existing = [] + for o in outs: + if o.path == out.path: + existing.append(o.stage) + + in_o_dir = out.path.startswith(o.path + o.sep) + in_out_dir = o.path.startswith(out.path + out.sep) + if in_o_dir or in_out_dir: + raise OverlappingOutputPathsError(o, out) if existing: stages = [stage.relpath, existing[0].relpath] diff --git a/tests/test_repro.py b/tests/test_repro.py index beb825567..cd5109737 100644 --- a/tests/test_repro.py +++ b/tests/test_repro.py @@ -115,9 +115,10 @@ class TestReproWorkingDirectoryAsOutput(TestDvc): faulty_stage_path = os.path.join("dir2", "something.dvc") with open(faulty_stage_path, "w") as fd: + output = os.path.join("..", "something") stage_dump = { - "cmd": "echo something > something", - "outs": [{"path": "something"}], + "cmd": "echo something > {}".format(output), + "outs": [{"path": output}], } yaml.safe_dump(stage_dump, fd, default_flow_style=False) @@ -156,9 +157,10 @@ class TestReproWorkingDirectoryAsOutput(TestDvc): error_stage_path = os.path.join(nested_dir, "dir", "error.dvc") with open(error_stage_path, "w") as fd: + output = os.path.join("..", "..", "something") stage_dump = { - "cmd": "echo something > something", - "outs": [{"path": "something"}], + "cmd": "echo something > {}".format(output), + "outs": [{"path": output}], } yaml.safe_dump(stage_dump, fd, default_flow_style=False) diff --git a/tests/test_run.py b/tests/test_run.py index 8c4e21a2a..86ae2f0dd 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -20,6 +20,7 @@ from dvc.exceptions import ( CyclicGraphError, ArgumentDuplicationError, StagePathAsOutputError, + OverlappingOutputPathsError, ) from tests.basic_env import TestDvc @@ -198,12 +199,7 @@ class TestRunStageInsideOutput(TestDvc): self.dvc.run(cmd="", deps=[], outs=[self.DATA_DIR]) with self.assertRaises(StagePathAsOutputError): - self.dvc.run( - cmd="", - cwd=self.DATA_DIR, - outs=[self.FOO], - fname="inside-cwd.dvc", - ) + self.dvc.run(cmd="", cwd=self.DATA_DIR, fname="inside-cwd.dvc") def test_file_name(self): self.dvc.run(cmd="", deps=[], outs=[self.DATA_DIR]) @@ -820,3 +816,32 @@ class TestRunPersistOutsNoCache(TestRunPersist): def test(self): self._test() + + +class TestShouldRaiseOnOverlappingOutputPaths(TestDvc): + def test(self): + ret = main(["add", self.DATA_DIR]) + self.assertEqual(0, ret) + + with self.assertRaises(OverlappingOutputPathsError) as err: + self.dvc.run( + outs=[self.DATA], cmd="echo data >> {}".format(self.DATA) + ) + error_output = str(err.exception) + + data_dir_stage = self.DATA_DIR + Stage.STAGE_FILE_SUFFIX + data_stage = os.path.basename(self.DATA) + Stage.STAGE_FILE_SUFFIX + + self.assertIn("Paths for outs:\n", error_output) + self.assertIn( + "\n'{}'('{}')\n".format(self.DATA_DIR, data_dir_stage), + error_output, + ) + self.assertIn( + "\n'{}'('{}')\n".format(self.DATA, data_stage), error_output + ) + self.assertIn( + "\noverlap. To avoid unpredictable behaviour, rerun " + "command with non overlapping outs paths.", + error_output, + )