project: use one status snapshot for dirty checks On Git 2.11 and newer, replace update-index, diff-index, diff-files, and ls-files fan-out with one porcelain-v2 snapshot for IsDirty, UncommittedFiles, and HasChanges. Preserve category ordering and duplicate staged-plus-unstaged paths, and keep the exact legacy path for older Git or a failed snapshot. Bug: 543851900 Bug: 553599402 Change-Id: I309e39977e1f1d4ee6115f488d6aabb83ec3d83c Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/632143 Tested-by: Gavin Mak <gavinmak@google.com> Reviewed-by: Brian Gan <brgan@google.com> Commit-Queue: Gavin Mak <gavinmak@google.com>
diff --git a/project.py b/project.py index 68986d4..0cf1d37 100644 --- a/project.py +++ b/project.py
@@ -57,6 +57,7 @@ from git_refs import R_PUB from git_refs import R_TAGS from git_refs import R_WORKTREE_M +import git_status import git_superproject from git_trace2_event_log import EventLog import platform_utils @@ -868,8 +869,46 @@ if e.git_rc != 1: raise - def IsDirty(self, consider_untracked=True): + def IsDirty(self, consider_untracked: bool = True) -> bool: """Is the working directory modified in some way?""" + status = self._GetStatusSnapshot( + untracked_files="normal" if consider_untracked else "no" + ) + if status is not None: + return status.is_dirty(consider_untracked=consider_untracked) + + return self._IsDirtyLegacy(consider_untracked=consider_untracked) + + def _GetStatusSnapshot( + self, + untracked_files: str = "all", + branch: bool = False, + ahead_behind: bool = False, + show_stash: bool = False, + ) -> Optional[git_status.StatusSnapshot]: + """Read one porcelain-v2 snapshot, or select the legacy path.""" + if not git_require((2, 11, 0)): + return None + try: + return git_status.GetStatus( + self, + self.gitdir, + untracked_files=untracked_files, + branch=branch, + ahead_behind=ahead_behind, + show_stash=show_stash, + ) + except (GitError, ValueError, git_status.UnsupportedStatusError) as e: + logger.warning( + "project %s: porcelain v2 status failed; using legacy " + "status: %s", + self.RelPath(local=False), + e, + ) + return None + + def _IsDirtyLegacy(self, consider_untracked: bool = True) -> bool: + """Check dirty state with plumbing supported by older Git.""" self._RefreshIndexStatCache() if self.work_git.DiffZ("diff-index", "-M", "--cached", HEAD): return True @@ -994,6 +1033,46 @@ uncommitted files. If False - return as soon as any kind of uncommitted files is detected. """ + status = self._GetStatusSnapshot(untracked_files="all") + if status is not None: + return self._UncommittedFilesFromStatus(status, get_all=get_all) + + return self._UncommittedFilesLegacy(get_all=get_all) + + def _UncommittedFilesFromStatus( + self, status: git_status.StatusSnapshot, get_all: bool = True + ) -> List[str]: + """Format uncommitted paths from a porcelain-v2 snapshot.""" + details = [] + if self.IsRebaseInProgress(): + details.append("rebase in progress") + if not get_all: + return details + + changes = [] + for path, entry in status.index_changes.items(): + changes.append(path) + # The legacy diff-index call did not enable rename detection, so + # it reported a staged rename as delete(source) plus add(target). + if entry.status == "R" and entry.src_path: + changes.append(entry.src_path) + changes.sort() + if changes: + details.extend(changes) + if not get_all: + return details + + changes = list(status.worktree_changes) + if changes: + details.extend(changes) + if not get_all: + return details + + details.extend(status.untracked) + return details + + def _UncommittedFilesLegacy(self, get_all: bool = True) -> List[str]: + """List uncommitted paths with plumbing supported by older Git.""" details = [] self._RefreshIndexStatCache() if self.IsRebaseInProgress():
diff --git a/tests/test_project.py b/tests/test_project.py index f82e1f6..2a5aebf 100644 --- a/tests/test_project.py +++ b/tests/test_project.py
@@ -30,6 +30,7 @@ import error import git_command import git_config +import git_status import git_trace2_event_log import manifest_xml import platform_utils @@ -602,6 +603,84 @@ proj.bare_git.SetHead.assert_called_once_with("refs/heads/manifest") proj.bare_git.DetachHead.assert_called_once_with(revision) + def test_dirty_status_snapshot(self) -> None: + """Dirty checks cover staged, unstaged, and optional untracked files.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + Path(tempdir, "tracked").write_text("initial") + proj.work_git.add("tracked") + proj.work_git.commit("-m", "initial") + + self.assertFalse(proj.IsDirty()) + + Path(tempdir, "tracked").write_text("staged") + proj.work_git.add("tracked") + self.assertTrue(proj.IsDirty(consider_untracked=False)) + + Path(tempdir, "tracked").write_text("unstaged") + self.assertEqual( + ["tracked", "tracked"], proj.UncommittedFiles(get_all=True) + ) + + proj.work_git.reset("--hard", "HEAD") + Path(tempdir, "untracked").write_text("new") + self.assertTrue(proj.IsDirty()) + self.assertFalse(proj.IsDirty(consider_untracked=False)) + + def test_uncommitted_files_preserve_staged_rename_paths(self) -> None: + """The snapshot returns both paths reported by legacy diff-index.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + Path(tempdir, "old").write_text("tracked") + proj.work_git.add("old") + proj.work_git.commit("-m", "initial") + proj.work_git.mv("old", "new") + + self.assertEqual(["new", "old"], proj.UncommittedFiles()) + + def test_has_changes_includes_rebase_from_status_snapshot(self) -> None: + """HasChanges keeps treating an in-progress rebase as a change.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + status = git_status.StatusSnapshot() + proj._GetStatusSnapshot = mock.MagicMock(return_value=status) + proj.IsRebaseInProgress = mock.MagicMock(return_value=True) + + self.assertTrue(proj.HasChanges()) + self.assertEqual( + ["rebase in progress"], proj.UncommittedFiles(get_all=False) + ) + + def test_get_status_snapshot_handles_unsupported_status_error(self) -> None: + """UnsupportedStatusError triggers the legacy status fallback.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + with mock.patch.object( + git_status, + "GetStatus", + side_effect=git_status.UnsupportedStatusError, + ): + self.assertIsNone(proj._GetStatusSnapshot()) + + def test_old_git_dirty_check_uses_legacy_plumbing(self) -> None: + """Git clients before 2.11 retain the existing dirty-check path.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + proj.work_git = mock.MagicMock() + proj.work_git.DiffZ.side_effect = [{}, {"tracked": mock.sentinel}] + + with mock.patch.object(project, "git_require", return_value=False): + self.assertTrue(proj.IsDirty()) + + proj.work_git.update_index.assert_called_once_with( + "-q", + "--unmerged", + "--ignore-missing", + "--refresh", + log_as_error=False, + ) + self.assertEqual(2, proj.work_git.DiffZ.call_count) + @unittest.skipUnless( utils_for_test.supports_reftable(), "git reftable support is required for this test",