From 46c65fdba21127d8a350dd9c103e5015aa578442 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Tue, 17 Oct 2023 16:30:19 +0300 Subject: [PATCH] worktree: don't modify index entries in place (#10024) --- dvc/repo/worktree.py | 6 +++ dvc/testing/fixtures.py | 22 +++++++++++ dvc/testing/remote_tests.py | 43 +++++++++++++++++++--- dvc/testing/scripts.py | 28 ++++++++++++++ tests/dir_helpers.py | 19 ---------- tests/func/experiments/test_experiments.py | 2 +- tests/scripts.py | 27 +------------- 7 files changed, 96 insertions(+), 51 deletions(-) create mode 100644 dvc/testing/scripts.py diff --git a/dvc/repo/worktree.py b/dvc/repo/worktree.py index 319259d6b..f67327db0 100644 --- a/dvc/repo/worktree.py +++ b/dvc/repo/worktree.py @@ -210,7 +210,9 @@ def _merge_push_meta( assert isinstance(old_tree, Tree) entry.hash_info = old_tree.hash_info entry.meta = out.meta + entries = [entry] for subkey, entry in index.iteritems(key): + entries.append(entry) if entry.meta is not None and entry.meta.isdir: continue fs_path = repo.fs.path.join(repo.root_dir, *subkey) @@ -225,6 +227,10 @@ def _merge_push_meta( # this dir (entry will have the latest remote version # ID after checkout) entry.meta = meta + + for entry in entries: + index.add(entry) + tree_meta, new_tree = build_tree(index, key) out.obj = new_tree out.hash_info = new_tree.hash_info diff --git a/dvc/testing/fixtures.py b/dvc/testing/fixtures.py index 0161cfd65..8f03e1950 100644 --- a/dvc/testing/fixtures.py +++ b/dvc/testing/fixtures.py @@ -1,9 +1,12 @@ import os +import pathlib import subprocess # nosec B404 from typing import Dict, Tuple import pytest +from .scripts import copy_script + # pylint: disable=redefined-outer-name,unused-argument __all__ = [ @@ -28,6 +31,8 @@ __all__ = [ "local_workspace", "docker_compose_project_name", "docker_services", + "copy_script", + "run_copy", ] CACHE: Dict[Tuple[bool, bool, bool], str] = {} @@ -261,3 +266,20 @@ def docker_services( executor.execute(docker_setup) # note: we are not tearing down the containers here return Services(executor) + + +@pytest.fixture +def run_copy(tmp_dir, copy_script, dvc): # noqa: ARG001 + def run_copy(src, dst, **run_kwargs): + wdir = pathlib.Path(run_kwargs.get("wdir", ".")) + wdir = pathlib.Path("../" * len(wdir.parts)) + script_path = wdir / "copy.py" + + return dvc.run( + cmd=f"python {script_path} {src} {dst}", + outs=[dst], + deps=[src, f"{script_path}"], + **run_kwargs, + ) + + return run_copy diff --git a/dvc/testing/remote_tests.py b/dvc/testing/remote_tests.py index 067e9e4aa..e72d3ad9e 100644 --- a/dvc/testing/remote_tests.py +++ b/dvc/testing/remote_tests.py @@ -169,22 +169,40 @@ class TestRemote: class TestRemoteVersionAware: - def test_file(self, tmp_dir, dvc, remote_version_aware): # pylint: disable=W0613 + def test_file( + self, tmp_dir, dvc, run_copy, remote_version_aware + ): # pylint: disable=W0613 (stage,) = tmp_dir.dvc_gen("foo", "foo") + run_copy("foo", "foo_copy", name="copy") dvc.push() - assert "version_id" in (tmp_dir / "foo.dvc").read_text() + foo_dvc = (tmp_dir / "foo.dvc").read_text() + assert "version_id" in foo_dvc stage = stage.reload() out = stage.outs[0] assert out.meta.version_id + dvc_lock = (tmp_dir / "dvc.lock").read_text() remove(dvc.cache.local.path) remove(tmp_dir / "foo") dvc.pull() assert (tmp_dir / "foo").read_text() == "foo" + assert (tmp_dir / "foo.dvc").read_text() == foo_dvc + assert (tmp_dir / "dvc.lock").read_text() == dvc_lock - def test_dir(self, tmp_dir, dvc, remote_version_aware): # pylint: disable=W0613 + dvc.push() + assert (tmp_dir / "foo.dvc").read_text() == foo_dvc + assert (tmp_dir / "dvc.lock").read_text() == dvc_lock + + dvc.reproduce() + dvc.push() + assert (tmp_dir / "foo.dvc").read_text() == foo_dvc + assert (tmp_dir / "dvc.lock").read_text() == dvc_lock + + def test_dir( + self, tmp_dir, dvc, run_copy, remote_version_aware + ): # pylint: disable=W0613 (stage,) = tmp_dir.dvc_gen( { "data_dir": { @@ -196,8 +214,10 @@ class TestRemoteVersionAware: ) dvc.push() - assert "files" in (tmp_dir / "data_dir.dvc").read_text() - assert "version_id" in (tmp_dir / "data_dir.dvc").read_text() + + data_dir_dvc = (tmp_dir / "data_dir.dvc").read_text() + assert "files" in data_dir_dvc + assert "version_id" in data_dir_dvc stage = stage.reload() out = stage.outs[0] assert out.files @@ -213,6 +233,19 @@ class TestRemoteVersionAware: assert ( tmp_dir / "data_dir" / "data_sub_dir" / "data_sub" ).read_text() == "data_sub" + assert (tmp_dir / "data_dir.dvc").read_text() == data_dir_dvc + + run_copy("data_dir", "data_dir_copy", name="copy") + dvc_lock = (tmp_dir / "dvc.lock").read_text() + + dvc.push() + assert (tmp_dir / "data_dir.dvc").read_text() == data_dir_dvc + assert (tmp_dir / "dvc.lock").read_text() != dvc_lock + dvc_lock = (tmp_dir / "dvc.lock").read_text() + + dvc.push() + assert (tmp_dir / "data_dir.dvc").read_text() == data_dir_dvc + assert (tmp_dir / "dvc.lock").read_text() == dvc_lock class TestRemoteWorktree: diff --git a/dvc/testing/scripts.py b/dvc/testing/scripts.py new file mode 100644 index 000000000..072ecd5af --- /dev/null +++ b/dvc/testing/scripts.py @@ -0,0 +1,28 @@ +import pytest + +COPY_SCRIPT = """ +import os +import shutil +import sys + +if os.path.isfile(sys.argv[1]): + shutil.copyfile(sys.argv[1], sys.argv[2]) +else: + shutil.copytree(sys.argv[1], sys.argv[2]) +""".replace( + "\r\n", "\n" +) + + +def _add_script(tmp_dir, path, contents=""): + script, *_ = tmp_dir.gen(path, contents.encode("utf-8")) + if hasattr(tmp_dir, "scm"): + tmp_dir.scm_add(path, commit=f"add {path}") + return script.fs_path + + +@pytest.fixture +def copy_script( + tmp_dir, +): + return _add_script(tmp_dir, "copy.py", COPY_SCRIPT) diff --git a/tests/dir_helpers.py b/tests/dir_helpers.py index b96671676..b340ab89c 100644 --- a/tests/dir_helpers.py +++ b/tests/dir_helpers.py @@ -45,12 +45,10 @@ from global repo template to creating everything inplace, which: import os -import pathlib import pytest __all__ = [ - "run_copy", "run_head", "erepo_dir", "git_dir", @@ -59,23 +57,6 @@ __all__ = [ ] -@pytest.fixture -def run_copy(tmp_dir, copy_script, dvc): - def run_copy(src, dst, **run_kwargs): - wdir = pathlib.Path(run_kwargs.get("wdir", ".")) - wdir = pathlib.Path("../" * len(wdir.parts)) - script_path = wdir / "copy.py" - - return dvc.run( - cmd=f"python {script_path} {src} {dst}", - outs=[dst], - deps=[src, f"{script_path}"], - **run_kwargs, - ) - - return run_copy - - @pytest.fixture def run_head(tmp_dir, head_script, dvc): script = os.path.abspath(tmp_dir / "head.py") diff --git a/tests/func/experiments/test_experiments.py b/tests/func/experiments/test_experiments.py index 29259ac3e..0b729674a 100644 --- a/tests/func/experiments/test_experiments.py +++ b/tests/func/experiments/test_experiments.py @@ -25,8 +25,8 @@ from dvc.repo.experiments.refs import CELERY_STASH from dvc.repo.experiments.utils import exp_refs_by_rev from dvc.scm import SCMError, resolve_rev from dvc.stage.exceptions import StageFileDoesNotExistError +from dvc.testing.scripts import COPY_SCRIPT from dvc.utils.serialize import PythonFileCorruptedError -from tests.scripts import COPY_SCRIPT @pytest.mark.parametrize("name", [None, "foo"]) diff --git a/tests/scripts.py b/tests/scripts.py index 257ebaed3..5b2887f8f 100644 --- a/tests/scripts.py +++ b/tests/scripts.py @@ -1,24 +1,6 @@ import pytest -COPY_SCRIPT = """ -import os -import shutil -import sys - -if os.path.isfile(sys.argv[1]): - shutil.copyfile(sys.argv[1], sys.argv[2]) -else: - shutil.copytree(sys.argv[1], sys.argv[2]) -""".replace( - "\r\n", "\n" -) - - -def _add_script(tmp_dir, path, contents=""): - script, *_ = tmp_dir.gen(path, contents.encode("utf-8")) - if hasattr(tmp_dir, "scm"): - tmp_dir.scm_add(path, commit=f"add {path}") - return script.fs_path +from dvc.testing.scripts import _add_script @pytest.fixture @@ -38,13 +20,6 @@ with Path(sys.argv[1]).open("a+", encoding="utf-8") as f: ) -@pytest.fixture -def copy_script( - tmp_dir, -): - return _add_script(tmp_dir, "copy.py", COPY_SCRIPT) - - @pytest.fixture def head_script(tmp_dir): """Output first line of each file to different file with '-1' appended.