From 5a86586f3a2604ae71a557b0c61c46089ee554e8 Mon Sep 17 00:00:00 2001 From: skshetry <18718008+skshetry@users.noreply.github.com> Date: Mon, 18 Aug 2025 21:23:01 +0545 Subject: [PATCH] feat(data status): detect file moves/renames by default (#10846) --- dvc/commands/data.py | 27 ++-- dvc/repo/data.py | 115 +++++++++++--- tests/func/test_data_status.py | 204 +++++++++++++++++++++++++ tests/unit/command/test_data_status.py | 4 + 4 files changed, 316 insertions(+), 34 deletions(-) diff --git a/dvc/commands/data.py b/dvc/commands/data.py index 4e8e398fd..fb5fd5958 100644 --- a/dvc/commands/data.py +++ b/dvc/commands/data.py @@ -10,6 +10,7 @@ from dvc.ui import ui from dvc.utils import colorize if TYPE_CHECKING: + from dvc.repo.data import GitInfo from dvc.repo.data import Status as DataStatus @@ -55,20 +56,15 @@ class CmdDataStatus(CmdBase): def _process_status(status: "DataStatus"): """Flatten stage status, and filter empty stage status contents.""" for stage, stage_status in status.items(): - items = stage_status - if isinstance(stage_status, dict): - items = { - file: state - for state, files in stage_status.items() - for file in files - } - if not items: + if not stage_status or ( + isinstance(stage_status, dict) and not any(stage_status.values()) + ): continue - yield stage, items + yield stage, stage_status @classmethod def _show_status(cls, status: "DataStatus") -> int: # noqa: C901 - git_info = status.pop("git") # type: ignore[misc] + git_info: GitInfo = status.pop("git") # type: ignore[misc] result = dict(cls._process_status(status)) if not result: no_changes = "No changes" @@ -90,7 +86,16 @@ class CmdDataStatus(CmdBase): ui.write(f" ({hint})") if isinstance(stage_status, dict): - items = [f"{state}: {file}" for file, state in stage_status.items()] + items = [ + f"{state}: " + + ( + " -> ".join(change.values()) + if isinstance(change, dict) + else change + ) + for state, changes in stage_status.items() + for change in changes + ] else: items = stage_status diff --git a/dvc/repo/data.py b/dvc/repo/data.py index c209f17a3..80f678b5e 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -41,6 +41,14 @@ def _adapt_path(change: "Change") -> str: return os.path.sep.join(key) +def _adapt_path_from_entry(entry: "DataIndexEntry") -> str: + key = entry.key + assert key + if entry.meta and entry.meta.isdir: + key = (*key, "") + return os.sep.join(key) + + def _get_missing_paths( to_check: Mapping["FileSystem", Mapping[str, Iterable["DataIndexEntry"]]], batch_size: Optional[int] = None, @@ -59,11 +67,7 @@ def _get_missing_paths( continue for entry in paths_map[cache_path]: - key = entry.key - assert key - if entry.meta and entry.meta.isdir: - key = (*key, "") - yield os.path.sep.join(key) + yield _adapt_path_from_entry(entry) class StorageCallback(Callback): @@ -82,6 +86,21 @@ class StorageCallback(Callback): self.parent_cb.relative_update(value - self.value) +class Rename(TypedDict): + old: str + new: str + + +class DiffResult(TypedDict, total=False): + modified: list[str] + added: list[str] + deleted: list[str] + renamed: list[Rename] + unchanged: list[str] + unknown: list[str] + not_in_cache: list[str] + + def _diff( old: "BaseDataIndex", new: "BaseDataIndex", @@ -91,11 +110,25 @@ def _diff( not_in_cache: bool = False, batch_size: Optional[int] = None, callback: "Callback" = DEFAULT_CALLBACK, -) -> dict[str, list[str]]: - from dvc_data.index.diff import ADD, DELETE, MODIFY, UNCHANGED, UNKNOWN, diff + with_renames: bool = False, +) -> DiffResult: + from dvc_data.index.diff import ( + ADD, + DELETE, + MODIFY, + RENAME, + UNCHANGED, + UNKNOWN, + diff, + ) - ret: dict[str, list[str]] = defaultdict(list) - change_types = {MODIFY: "modified", ADD: "added", DELETE: "deleted"} + ret: DiffResult = defaultdict(list) # type: ignore[assignment] + change_types = { + MODIFY: "modified", + ADD: "added", + DELETE: "deleted", + RENAME: "renamed", + } to_check: dict[FileSystem, dict[str, list[DataIndexEntry]]] = defaultdict( lambda: defaultdict(list) @@ -108,25 +141,38 @@ def _diff( shallow=not granular, hash_only=True, with_unknown=True, + with_renames=with_renames, callback=callback, ): + typ = change.typ + # The index is a trie, so even when we filter by a specific path # like `dir/file`, all parent nodes leading to that path (e.g., `dir/`) # still appear in the view. As a result, keys like `dir/` will be present # even if only `dir/file` matches the filter. # We need to skip such entries to avoid showing root of tracked directories. - if filter_keys and not any(change.key[: len(fk)] == fk for fk in filter_keys): - continue + if filter_keys: + # RENAME does not have a `change.key` + if typ == RENAME: + assert change.new + key = change.new.key + # match with "new" key only + assert key + else: + key = change.key + + if not any(key[: len(fk)] == fk for fk in filter_keys): + continue if ( - change.typ == UNCHANGED + typ == UNCHANGED and (not change.old or not change.old.hash_info) and (not change.new or not change.new.hash_info) ): # NOTE: emulating previous behaviour continue - if change.typ == UNKNOWN and not change.new: + if typ == UNKNOWN and not change.new: # NOTE: emulating previous behaviour continue @@ -136,8 +182,18 @@ def _diff( # check later in batches to_check[cache_fs][cache_path].append(old_entry) - change_typ = change_types.get(change.typ, change.typ) - ret[change_typ].append(_adapt_path(change)) + change_typ = change_types.get(typ, typ) + if typ == RENAME: + assert change.old is not None + assert change.new is not None + ret["renamed"].append( + { + "old": _adapt_path_from_entry(change.old), + "new": _adapt_path_from_entry(change.new), + } + ) + else: + ret[change_typ].append(_adapt_path(change)) # type: ignore[literal-required] total_items = sum( len(entries) for paths in to_check.values() for entries in paths.values() @@ -150,7 +206,7 @@ def _diff( ) if missing_items: ret["not_in_cache"] = missing_items - return dict(ret) + return dict(ret) # type: ignore[return-value] class GitInfo(TypedDict, total=False): @@ -230,7 +286,8 @@ def _diff_index_to_wtree( filter_keys: Optional[Iterable["DataIndexKey"]] = None, granular: bool = False, batch_size: Optional[int] = None, -) -> dict[str, list[str]]: + with_renames: bool = False, +) -> DiffResult: from .index import build_data_index with ui.progress(desc="Building workspace index", unit="entry") as pb: @@ -255,6 +312,7 @@ def _diff_index_to_wtree( filter_keys=filter_keys, granular=granular, not_in_cache=True, + with_renames=with_renames, batch_size=batch_size, callback=pb.as_callback(), ) @@ -265,7 +323,8 @@ def _diff_head_to_index( head: str = "HEAD", filter_keys: Optional[Iterable["DataIndexKey"]] = None, granular: bool = False, -) -> dict[str, list[str]]: + with_renames: bool = False, +) -> DiffResult: from dvc.scm import RevError from dvc_data.index import DataIndex @@ -286,6 +345,7 @@ def _diff_head_to_index( index_view, filter_keys=filter_keys, granular=granular, + with_renames=with_renames, callback=pb.as_callback(), ) @@ -293,8 +353,8 @@ def _diff_head_to_index( class Status(TypedDict): not_in_cache: list[str] not_in_remote: list[str] - committed: dict[str, list[str]] - uncommitted: dict[str, list[str]] + committed: DiffResult + uncommitted: DiffResult untracked: list[str] unchanged: list[str] git: GitInfo @@ -398,7 +458,7 @@ def _prune_keys(filter_keys: Iterable["DataIndexKey"]) -> list["DataIndexKey"]: return result -def status( +def status( # noqa: PLR0913 repo: "Repo", targets: Optional[Iterable[Union[os.PathLike[str], str]]] = None, *, @@ -410,6 +470,7 @@ def status( config: Optional[dict] = None, batch_size: Optional[int] = None, head: str = "HEAD", + with_renames: bool = True, ) -> Status: from dvc.scm import NoSCMError, SCMError @@ -426,7 +487,11 @@ def status( filter_keys = _prune_keys(filter_keys) uncommitted_diff = _diff_index_to_wtree( - repo, filter_keys=filter_keys, granular=granular, batch_size=batch_size + repo, + filter_keys=filter_keys, + granular=granular, + batch_size=batch_size, + with_renames=with_renames, ) unchanged = set(uncommitted_diff.pop("unchanged", [])) @@ -441,7 +506,11 @@ def status( try: committed_diff = _diff_head_to_index( - repo, filter_keys=filter_keys, head=head, granular=granular + repo, + filter_keys=filter_keys, + head=head, + granular=granular, + with_renames=with_renames, ) except (SCMError, NoSCMError): committed_diff = {} diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index bfaed9a82..844574795 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -1,3 +1,4 @@ +import shutil from collections.abc import Iterable from functools import partial from os import fspath @@ -1000,3 +1001,206 @@ def test_missing_cache_remote_check(M, tmp_dir, dvc, scm, local_remote): "git": M.dict(), "not_in_remote": [join("dir", "")], } + + +@pytest.mark.parametrize( + "targets,expected_non_granular,expected_granular", + [ + param( + None, + { + "committed": { + "renamed": [ + {"old": join("dir", ""), "new": join("dir2", "")}, + {"old": "file", "new": "file2"}, + ], + }, + "uncommitted": {"modified": [join("dir2", "")]}, + }, + { + "committed": { + "renamed": [ + {"old": join("dir", ""), "new": join("dir2", "")}, + {"old": join("dir", "bar"), "new": join("dir2", "bar")}, + {"old": join("dir", "foo"), "new": join("dir2", "foo")}, + {"old": "file", "new": "file2"}, + ], + }, + "uncommitted": { + "modified": [join("dir2", "")], + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + param( + ["dir"], + {"committed": {"deleted": [join("dir", "")]}}, + { + "committed": { + "deleted": m.unordered( + join("dir", ""), join("dir", "bar"), join("dir", "foo") + ) + } + }, + ), + param( + ["dir2"], + { + "committed": {"added": [join("dir2", "")]}, + "uncommitted": {"modified": [join("dir2", "")]}, + }, + { + "committed": { + "added": m.unordered( + join("dir2", ""), + join("dir2", "bar"), + join("dir2", "foo"), + ), + }, + "uncommitted": { + "modified": [join("dir2", "")], + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + param( + [join("dir", "bar")], {}, {"committed": {"deleted": [join("dir", "bar")]}} + ), + param( + [join("dir", "foo")], {}, {"committed": {"deleted": [join("dir", "foo")]}} + ), + param( + [join("dir2", "bar")], {}, {"committed": {"added": [join("dir2", "bar")]}} + ), + param( + [join("dir2", "foobar")], + {}, + {"uncommitted": {"added": [join("dir2", "foobar")]}}, + ), + param( + ["file"], + {"committed": {"deleted": ["file"]}}, + {"committed": {"deleted": ["file"]}}, + ), + param( + ["file2"], + {"committed": {"added": ["file2"]}}, + {"committed": {"added": ["file2"]}}, + ), + param( + ["dir", "dir2"], + { + "committed": { + "renamed": [{"old": join("dir", ""), "new": join("dir2", "")}], + }, + "uncommitted": {"modified": [join("dir2", "")]}, + }, + { + "committed": { + "renamed": [ + {"old": join("dir", ""), "new": join("dir2", "")}, + {"old": join("dir", "bar"), "new": join("dir2", "bar")}, + {"old": join("dir", "foo"), "new": join("dir2", "foo")}, + ] + }, + "uncommitted": { + "modified": [join("dir2", "")], + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + param( + ["file", "file2"], + {"committed": {"renamed": [{"old": "file", "new": "file2"}]}}, + {"committed": {"renamed": [{"old": "file", "new": "file2"}]}}, + ), + param( + [join("dir", "foo"), join("dir2", "foobar")], + {}, + { + "committed": {"deleted": [join("dir", "foo")]}, + "uncommitted": {"added": [join("dir2", "foobar")]}, + }, + ), + param( + [join("dir2", "foo"), join("dir2", "foobar")], + {}, + { + "committed": {"added": [join("dir2", "foo")]}, + "uncommitted": { + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + param( + ["dir2", "file2"], + { + "committed": {"added": m.unordered(join("dir2", ""), "file2")}, + "uncommitted": {"modified": [join("dir2", "")]}, + }, + { + "committed": { + "added": m.unordered( + join("dir2", ""), + join("dir2", "bar"), + join("dir2", "foo"), + "file2", + ) + }, + "uncommitted": { + "modified": [join("dir2", "")], + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + param( + ["dir2", join("dir2", "foobar"), "file"], + { + "uncommitted": {"modified": [join("dir2", "")]}, + "committed": {"added": [join("dir2", "")], "deleted": ["file"]}, + }, + { + "committed": { + "added": m.unordered( + join("dir2", ""), join("dir2", "bar"), join("dir2", "foo") + ), + "deleted": ["file"], + }, + "uncommitted": { + "modified": [join("dir2", "")], + "renamed": [ + {"old": join("dir2", "foo"), "new": join("dir2", "foobar")} + ], + }, + }, + ), + ], +) +def test_renames( + M, tmp_dir, scm, dvc, targets, expected_non_granular, expected_granular +): + tmp_dir.dvc_gen( + {"dir": {"foo": "foo", "bar": "bar"}, "file": "file"}, commit="add dir and file" + ) + dvc.move("dir", "dir2") + dvc.move("file", "file2") + shutil.move(tmp_dir / "dir2" / "foo", tmp_dir / "dir2" / "foobar") + + assert ( + dvc.data_status(with_renames=True, targets=targets) + == EMPTY_STATUS | {"git": M.dict()} | expected_non_granular + ) + assert ( + dvc.data_status(granular=True, with_renames=True, targets=targets) + == EMPTY_STATUS | {"git": M.dict()} | expected_granular + ) diff --git a/tests/unit/command/test_data_status.py b/tests/unit/command/test_data_status.py index 04c529881..9436716ea 100644 --- a/tests/unit/command/test_data_status.py +++ b/tests/unit/command/test_data_status.py @@ -19,12 +19,14 @@ def mocked_status(): "deleted": ["dir/baz"], "modified": ["dir/foobar"], "unknown": ["dir/unknown1"], + "renamed": [{"old": "dir/", "new": "dir2/"}], }, uncommitted={ "added": ["dir/baz"], "modified": ["dir/bar"], "deleted": ["dir/foobar"], "unknown": ["dir2/unknown2"], + "renamed": [{"old": "dir2/file-old", "new": "dir2/file-new"}], }, untracked=["untracked"], unchanged=["dir/foo"], @@ -124,6 +126,7 @@ DVC committed changes: deleted: dir/baz modified: dir/foobar unknown: dir/unknown1 + renamed: dir/ -> dir2/ DVC uncommitted changes: (use "dvc commit ..." to track changes) @@ -132,6 +135,7 @@ DVC uncommitted changes: modified: dir/bar deleted: dir/foobar unknown: dir2/unknown2 + renamed: dir2/file-old -> dir2/file-new """ if "--untracked-files" in args: expected_out += """