From ed45313a2b231fb151bfde9c7d83a932aec6c46b Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Wed, 18 Dec 2019 15:34:33 +0000 Subject: [PATCH 01/15] add: auto convert absolute_path => relative - fixes #2955 --- dvc/repo/add.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index e469119b93..bedae0ddf6 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -79,6 +79,12 @@ def add(repo, targets, recursive=False, no_commit=False, fname=None): def _find_all_targets(repo, target, recursive): + target = os.path.normpath(target) + if os.path.commonprefix( + [os.path.abspath(target), repo.root_dir] + ).startswith(repo.root_dir): + target = os.path.relpath(target, repo.root_dir) + if os.path.isdir(target) and recursive: return [ fname From 8b6bec3feb1b092ebdbfadbdfea088202e01a38e Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 16 Jan 2020 22:01:56 +0000 Subject: [PATCH 02/15] use path_isin --- dvc/repo/add.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index bedae0ddf6..708045ad6a 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -10,6 +10,7 @@ from ..repo.scm_context import scm_context from ..stage import Stage from ..utils import LARGE_DIR_SIZE +from ..utils.fs import path_isin logger = logging.getLogger(__name__) @@ -80,9 +81,7 @@ def add(repo, targets, recursive=False, no_commit=False, fname=None): def _find_all_targets(repo, target, recursive): target = os.path.normpath(target) - if os.path.commonprefix( - [os.path.abspath(target), repo.root_dir] - ).startswith(repo.root_dir): + if path_isin(target, repo.root_dir): target = os.path.relpath(target, repo.root_dir) if os.path.isdir(target) and recursive: From 0026bb546dde1a7d77ef8ad40ccf79f64776ff15 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 16 Jan 2020 22:17:18 +0000 Subject: [PATCH 03/15] catch-all absolute paths in repo --- dvc/output/base.py | 2 ++ dvc/repo/add.py | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/output/base.py b/dvc/output/base.py index 7770c30cce..6a8839b731 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -90,6 +90,8 @@ def __init__( # should be absolute and don't contain remote:// refs. self.stage = stage self.repo = stage.repo if stage else None + if self.repo is not None and path_isin(path, self.repo.root_dir): + path = os.path.relpath(path, self.repo.root_dir) self.def_path = path self.info = info self.remote = remote or self.REMOTE(self.repo, {}) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 708045ad6a..7cde77fe0c 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -81,8 +81,6 @@ def add(repo, targets, recursive=False, no_commit=False, fname=None): def _find_all_targets(repo, target, recursive): target = os.path.normpath(target) - if path_isin(target, repo.root_dir): - target = os.path.relpath(target, repo.root_dir) if os.path.isdir(target) and recursive: return [ From e03fdd8b6ba7f668ba32c7f80c814676f8636f0f Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sun, 19 Jan 2020 19:28:44 +0000 Subject: [PATCH 04/15] unneeded leftover --- dvc/repo/add.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index 7cde77fe0c..e469119b93 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -10,7 +10,6 @@ from ..repo.scm_context import scm_context from ..stage import Stage from ..utils import LARGE_DIR_SIZE -from ..utils.fs import path_isin logger = logging.getLogger(__name__) @@ -80,8 +79,6 @@ def add(repo, targets, recursive=False, no_commit=False, fname=None): def _find_all_targets(repo, target, recursive): - target = os.path.normpath(target) - if os.path.isdir(target) and recursive: return [ fname From d11cbd9ff2d2aebb4efa0791a3b9496ee23af4ae Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 20 Jan 2020 14:15:16 +0000 Subject: [PATCH 05/15] revert realtive imports for later --- dvc/output/base.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dvc/output/base.py b/dvc/output/base.py index 6a8839b731..f285bdff67 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -9,6 +9,7 @@ from dvc.exceptions import CollectCacheError, RemoteCacheRequiredError from dvc.exceptions import DvcException from dvc.remote.base import RemoteBASE +from dvc.utils.fs import path_isin logger = logging.getLogger(__name__) From ce13d120f808efa83fc677579b58cbd0928c989d Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 27 Jan 2020 00:29:42 +0000 Subject: [PATCH 06/15] output: move abs2relpath from Base to Local --- dvc/output/base.py | 3 --- dvc/output/local.py | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dvc/output/base.py b/dvc/output/base.py index f285bdff67..7770c30cce 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -9,7 +9,6 @@ from dvc.exceptions import CollectCacheError, RemoteCacheRequiredError from dvc.exceptions import DvcException from dvc.remote.base import RemoteBASE -from dvc.utils.fs import path_isin logger = logging.getLogger(__name__) @@ -91,8 +90,6 @@ def __init__( # should be absolute and don't contain remote:// refs. self.stage = stage self.repo = stage.repo if stage else None - if self.repo is not None and path_isin(path, self.repo.root_dir): - path = os.path.relpath(path, self.repo.root_dir) self.def_path = path self.info = info self.remote = remote or self.REMOTE(self.repo, {}) diff --git a/dvc/output/local.py b/dvc/output/local.py index 0e1b72dc97..5f13011066 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -18,6 +18,11 @@ class OutputLOCAL(OutputBase): REMOTE = RemoteLOCAL sep = os.sep + def __init__(self, stage, path, **kwargs): + if stage and stage.repo and path_isin(path, stage.repo.root_dir): + path = os.path.relpath(path, self.repo.root_dir) + super().__init__(stage, path, **kwargs) + def _parse_path(self, remote, path): parsed = urlparse(path) if parsed.scheme == "remote": From 2e64c5e3004774004132a460cd27b410e604e4c1 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 27 Jan 2020 00:39:19 +0000 Subject: [PATCH 07/15] output:local:fix parse_path --- dvc/output/local.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 5f13011066..75c376be8b 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -18,10 +18,12 @@ class OutputLOCAL(OutputBase): REMOTE = RemoteLOCAL sep = os.sep - def __init__(self, stage, path, **kwargs): - if stage and stage.repo and path_isin(path, stage.repo.root_dir): - path = os.path.relpath(path, self.repo.root_dir) - super().__init__(stage, path, **kwargs) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + if self.repo and path_isin(self.def_path, self.repo.root_dir): + self.def_path = relpath(self.def_path, self.repo.root_dir) + self.path_info = self._parse_path(self.remote, self.def_path) def _parse_path(self, remote, path): parsed = urlparse(path) From 010114ec510f9e763a6b9aeb7b312afeab973a5a Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 30 Jan 2020 00:28:55 +0000 Subject: [PATCH 08/15] remove path_info --- dvc/output/local.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 75c376be8b..028b8d53ba 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -23,7 +23,6 @@ def __init__(self, *args, **kwargs): if self.repo and path_isin(self.def_path, self.repo.root_dir): self.def_path = relpath(self.def_path, self.repo.root_dir) - self.path_info = self._parse_path(self.remote, self.def_path) def _parse_path(self, remote, path): parsed = urlparse(path) From 25f499976a598b9ac93d201f85e5ff7b8f008060 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 30 Jan 2020 20:55:15 +0000 Subject: [PATCH 09/15] test:local versus base abs2relpath --- tests/unit/output/test_local.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/output/test_local.py b/tests/unit/output/test_local.py index 50059e6eae..e0b0494d9b 100644 --- a/tests/unit/output/test_local.py +++ b/tests/unit/output/test_local.py @@ -2,6 +2,7 @@ from mock import patch from dvc.output import OutputLOCAL +from dvc.output.base import OutputBase from dvc.remote.local import RemoteLOCAL from dvc.stage import Stage from dvc.utils import relpath @@ -46,10 +47,15 @@ def test_str_on_absolute_path(dvc): stage = Stage(dvc) path = os.path.abspath(os.path.join("path", "to", "file")) - output = OutputLOCAL(stage, path, cache=False) + output = OutputBase(stage, path, cache=False) assert path == str(output) + output = OutputLOCAL(stage, path, cache=False) + + assert path != str(output) + assert path == os.path.abspath(str(output)) + class TestGetFilesNumber(TestDvc): def _get_output(self): From 309b0d3d33f0fcb46c2eb2370479fffe3b42520b Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 30 Jan 2020 20:59:20 +0000 Subject: [PATCH 10/15] test:fix remaining abs2relpath test --- tests/func/test_add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 1ce44a0b0b..002722f52b 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -216,7 +216,7 @@ def test(self): self.assertEqual(ret, 0) d = load_stage_file("bar.dvc") - self.assertEqual(d["outs"][0]["path"], bar) + self.assertEqual(d["outs"][0]["path"], self.BAR) class TestCmdAdd(TestDvc): From 1de5fd798e29b69b7ba62f5b333e84837efccb0b Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Thu, 6 Feb 2020 19:28:48 +0000 Subject: [PATCH 11/15] output: local path relative to wdir --- dvc/output/local.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 028b8d53ba..2b3cd908a1 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -22,7 +22,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if self.repo and path_isin(self.def_path, self.repo.root_dir): - self.def_path = relpath(self.def_path, self.repo.root_dir) + self.def_path = relpath(self.def_path, self.stage.wdir) def _parse_path(self, remote, path): parsed = urlparse(path) From 75fa84e8c3bc4c103c183a1ba04ebfc92691ff4a Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 8 Feb 2020 16:41:36 +0000 Subject: [PATCH 12/15] output: local: modify path before inheritance --- dvc/output/local.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 2b3cd908a1..591f894929 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -18,11 +18,11 @@ class OutputLOCAL(OutputBase): REMOTE = RemoteLOCAL sep = os.sep - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, stage, path, **kwargs): + if stage and path_isin(path, stage.repo.root_dir): + path = relpath(path, stage.wdir) - if self.repo and path_isin(self.def_path, self.repo.root_dir): - self.def_path = relpath(self.def_path, self.stage.wdir) + super().__init__(stage, path, **kwargs) def _parse_path(self, remote, path): parsed = urlparse(path) From 73269805f6de9bab2d0416617502696a0316e9c6 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sat, 8 Feb 2020 16:52:16 +0000 Subject: [PATCH 13/15] output: local: minor inheritance fix --- dvc/output/local.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/output/local.py b/dvc/output/local.py index 591f894929..6e4123d1e3 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -18,11 +18,11 @@ class OutputLOCAL(OutputBase): REMOTE = RemoteLOCAL sep = os.sep - def __init__(self, stage, path, **kwargs): + def __init__(self, stage, path, *args, **kwargs): if stage and path_isin(path, stage.repo.root_dir): path = relpath(path, stage.wdir) - super().__init__(stage, path, **kwargs) + super().__init__(stage, path, *args, **kwargs) def _parse_path(self, remote, path): parsed = urlparse(path) From 5af83d1ac07edb3118ea849a90aa8dd852db60d5 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Sun, 9 Feb 2020 21:39:40 +0000 Subject: [PATCH 14/15] test: local: split output test --- tests/unit/output/test_local.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/unit/output/test_local.py b/tests/unit/output/test_local.py index e0b0494d9b..bc7c3b937b 100644 --- a/tests/unit/output/test_local.py +++ b/tests/unit/output/test_local.py @@ -47,16 +47,21 @@ def test_str_on_absolute_path(dvc): stage = Stage(dvc) path = os.path.abspath(os.path.join("path", "to", "file")) - output = OutputBase(stage, path, cache=False) - - assert path == str(output) - output = OutputLOCAL(stage, path, cache=False) assert path != str(output) assert path == os.path.abspath(str(output)) +def test_base_str_on_absolute_path(dvc): + stage = Stage(dvc) + + path = os.path.abspath(os.path.join("path", "to", "file")) + output = OutputBase(stage, path, cache=False) + + assert path == str(output) + + class TestGetFilesNumber(TestDvc): def _get_output(self): stage = Stage(self.dvc) From d937cdf636364f18305e1834900b680f616b3ec6 Mon Sep 17 00:00:00 2001 From: Casper da Costa-Luis Date: Mon, 10 Feb 2020 13:52:06 +0000 Subject: [PATCH 15/15] test: local: positively explicit --- tests/unit/output/test_local.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/unit/output/test_local.py b/tests/unit/output/test_local.py index bc7c3b937b..ef029d2940 100644 --- a/tests/unit/output/test_local.py +++ b/tests/unit/output/test_local.py @@ -2,7 +2,6 @@ from mock import patch from dvc.output import OutputLOCAL -from dvc.output.base import OutputBase from dvc.remote.local import RemoteLOCAL from dvc.stage import Stage from dvc.utils import relpath @@ -43,23 +42,28 @@ def test_str_workdir_inside_repo(dvc): assert os.path.join("some_folder", "path") == str(output) -def test_str_on_absolute_path(dvc): +def test_str_on_local_absolute_path(dvc): stage = Stage(dvc) - path = os.path.abspath(os.path.join("path", "to", "file")) - output = OutputLOCAL(stage, path, cache=False) + rel_path = os.path.join("path", "to", "file") + abs_path = os.path.abspath(rel_path) + output = OutputLOCAL(stage, abs_path, cache=False) - assert path != str(output) - assert path == os.path.abspath(str(output)) + assert output.def_path == rel_path + assert output.path_info.fspath == abs_path + assert str(output) == rel_path -def test_base_str_on_absolute_path(dvc): +def test_str_on_external_absolute_path(dvc): stage = Stage(dvc) - path = os.path.abspath(os.path.join("path", "to", "file")) - output = OutputBase(stage, path, cache=False) + rel_path = os.path.join("..", "path", "to", "file") + abs_path = os.path.abspath(rel_path) + output = OutputLOCAL(stage, abs_path, cache=False) - assert path == str(output) + assert output.def_path == abs_path + assert output.path_info.fspath == abs_path + assert str(output) == abs_path class TestGetFilesNumber(TestDvc):