diff --git a/dvc/output.py b/dvc/output.py index 9164c23bd..0f7622b5c 100644 --- a/dvc/output.py +++ b/dvc/output.py @@ -997,8 +997,9 @@ class Output: self.save() self.commit() - if self.protocol == "local" and self.use_scm_ignore: - self.repo.scm_context.ignore(self.fspath) + # should already be ignored in self.save() + # if self.protocol == "local" and self.use_scm_ignore: + # self.repo.scm_context.ignore(self.fspath) def transfer( self, source, odb=None, jobs=None, update=False, no_progress_bar=False diff --git a/dvc/repo/move.py b/dvc/repo/move.py index 299858037..b196228ed 100644 --- a/dvc/repo/move.py +++ b/dvc/repo/move.py @@ -32,9 +32,10 @@ def move(self, from_path, to_path): It only works with outputs generated by `add` or `import`, also known as data sources. """ - from dvc import output + from dvc import dependency, output from dvc.dvcfile import DVC_FILE_SUFFIX from dvc.stage import Stage + from dvc_objects.fs.local import LocalFileSystem from_out = output.loads_from(Stage(self), [from_path])[0] assert from_out.protocol == "local" @@ -45,6 +46,7 @@ def move(self, from_path, to_path): assert len(outs) == 1 out = outs[0] stage = out.stage + deps = stage.deps if not stage.is_data_source: raise MoveNotDataSourceError(stage.addressing) @@ -64,6 +66,9 @@ def move(self, from_path, to_path): wdir=new_wdir, outs=[to_path], meta=stage.meta, + frozen=stage.frozen, + always_changed=stage.always_changed, + desc=stage.desc, ) os.unlink(stage.path) @@ -71,8 +76,18 @@ def move(self, from_path, to_path): else: to_path = os.path.relpath(to_path, stage.wdir) - to_out = output.loads_from(stage, [to_path], out.use_cache, out.metric)[0] + def with_dep_path_adjusted(dep: dependency.Dependency): + d = dep.dumpd() + if isinstance(dep.fs, LocalFileSystem) and not os.path.isabs(dep.def_path): + path = os.path.relpath(os.path.abspath(dep.def_path), stage.wdir) + return d | {"path": path} + return d - out.move(to_out) - stage.save() + stage.outs = output.loadd_from(stage, [out.dumpd() | {"path": to_path}]) + stage.deps = dependency.loadd_from( + stage, [with_dep_path_adjusted(dep) for dep in deps] + ) + + out.move(stage.outs[0]) + stage.md5 = stage.compute_md5() stage.dump() diff --git a/tests/func/test_move.py b/tests/func/test_move.py index 8fb110c40..4182b5812 100644 --- a/tests/func/test_move.py +++ b/tests/func/test_move.py @@ -8,12 +8,14 @@ from dvc.cli import main from dvc.exceptions import MoveNotDataSourceError, OutputNotFoundError -def test_move(tmp_dir, dvc): - tmp_dir.dvc_gen("foo", "foo") - dvc.move("foo", "foo1") +def test_move(tmp_dir, dvc, scm): + tmp_dir.dvc_gen("foo", "bar") + dvc.move("foo", "bar") assert not (tmp_dir / "foo").is_file() - assert (tmp_dir / "foo1").is_file() + assert (tmp_dir / "bar").is_file() + # should only have the new path in the .gitignore, and only once + assert (tmp_dir / ".gitignore").read_text().splitlines() == ["/bar"] def test_move_non_existent_file(dvc): @@ -195,3 +197,167 @@ def test_move_meta(tmp_dir, dvc): custom_key: 42 """ ) + + +def test_import(tmp_dir, dvc, scm): + tmp_dir.dvc_gen("foo", "foo", commit="add foo") + imp_stage = dvc.imp(os.curdir, "foo", "foo_imported") + + dvc.move("foo_imported", "foo_moved") + + (stage,) = dvc.stage.collect("foo_moved.dvc") + assert imp_stage.md5 != stage.md5 + res = (tmp_dir / "foo_moved.dvc").read_text() + assert res == textwrap.dedent( + f"""\ + md5: {stage.md5} + frozen: true + deps: + - path: foo + repo: + url: {os.curdir} + rev_lock: {scm.get_rev()} + outs: + - md5: acbd18db4cc2f85cedef654fccc4a4d8 + size: 3 + hash: md5 + path: foo_moved + """ + ) + + +@pytest.mark.parametrize( + "path_func", + [pytest.param(os.path.abspath, id="abs"), pytest.param(os.path.relpath, id="rel")], +) +def test_import_url_in_repo(tmp_dir, dvc, path_func): + tmp_dir.gen("foo", "foo") + imp_stage = dvc.imp_url(path_func(tmp_dir / "foo"), "foo_imported") + (tmp_dir / "data").mkdir() + + dvc.move("foo_imported", os.path.join("data", "foo_moved")) + + (stage,) = dvc.stage.collect(os.path.join("data", "foo_moved.dvc")) + assert imp_stage.md5 != stage.md5 + res = (tmp_dir / "data" / "foo_moved.dvc").read_text() + assert res == textwrap.dedent( + f"""\ + md5: {stage.md5} + frozen: true + deps: + - md5: acbd18db4cc2f85cedef654fccc4a4d8 + size: 3 + hash: md5 + path: ../foo + outs: + - md5: acbd18db4cc2f85cedef654fccc4a4d8 + size: 3 + hash: md5 + path: foo_moved + """ + ) + + +@pytest.mark.parametrize( + "path_func", + [pytest.param(os.path.abspath, id="abs"), pytest.param(os.path.relpath, id="rel")], +) +def test_import_url_out_of_repo(tmp_dir, dvc, scm, path_func, make_tmp_dir): + external = make_tmp_dir("external") + external.gen("foo", "foo") + + imp_stage = dvc.imp_url(path_func(external / "foo"), "foo_imported") + + data_dir = tmp_dir / "data" + data_dir.mkdir() + + new_path = data_dir / "foo_moved" + new_dvcfile = new_path.with_suffix(".dvc") + dvc.move("foo_imported", os.fspath(new_path)) + + (stage,) = dvc.stage.collect(os.fspath(new_dvcfile)) + assert imp_stage.md5 != stage.md5 + + with data_dir.chdir(): + expected_path = path_func(external / "foo") + + assert new_dvcfile.parse() == { + "md5": stage.md5, + "frozen": True, + "deps": [ + { + "md5": "acbd18db4cc2f85cedef654fccc4a4d8", + "size": 3, + "hash": "md5", + "path": expected_path, + } + ], + "outs": [ + { + "md5": "acbd18db4cc2f85cedef654fccc4a4d8", + "size": 3, + "hash": "md5", + "path": "foo_moved", + } + ], + } + + +@pytest.mark.parametrize( + "path_func", + [pytest.param(os.path.abspath, id="abs"), pytest.param(os.path.relpath, id="rel")], +) +def test_all_metadata_are_preserved(tmp_dir, dvc, make_tmp_dir, path_func): + external = make_tmp_dir("external") + external.gen("foo", "foo") + + contents = { + "md5": "bad", # placeholder, does not matter for the test + "frozen": True, + "desc": "this is a stage description", + "always_changed": True, + "meta": {"custom_key": 42}, + "deps": [ + { + "md5": "acbd18db4cc2f85cedef654fccc4a4d8", + "size": 3, + "hash": "md5", + "path": path_func(external / "foo"), + } + ], + "outs": [ + { + "md5": "acbd18db4cc2f85cedef654fccc4a4d8", + "path": "foo_imported", + "persist": True, + "hash": "md5", + "size": 3, + "desc": "this is a description", + "type": "model", + "labels": ["label1", "label2"], + "meta": {"custom_key": 42}, + "cache": False, + "remote": "myremote", + "push": False, + } + ], + } + (tmp_dir / "foo_imported.dvc").dump(contents) + (tmp_dir / "foo_imported").write_text("foo") + + data_dir = tmp_dir / "data" + data_dir.mkdir() + + new_path = data_dir / "foo_moved" + new_dvcfile = new_path.with_suffix(".dvc") + dvc.move("foo_imported", os.fspath(new_path)) + + (stage,) = dvc.stage.collect(os.fspath(new_dvcfile)) + + with data_dir.chdir(): + expected_path = path_func(external / "foo") + + contents["outs"][0] |= {"path": "foo_moved"} + contents["deps"][0] |= {"path": expected_path} + contents |= {"md5": stage.md5} + assert new_dvcfile.parse() == contents