Fix #350 Hash directory also hashes untracked files

[spoorcc]

#350

Summary by CodeRabbit

• New Features

  • Update/check operations now respect per-project ignore rules so ignored files don't block updates.

• Bug Fixes

  • Ignored files are excluded from local-change detection, preventing unnecessary skipped updates.

• Tests

  • Acceptance scenarios and unit tests added/updated to cover overwrite protection, force behavior, and ignored-file handling.

• Documentation

  • Docs updated with Git/SVN tabbed examples and changelog entry for the upcoming release.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR makes dfetch ignore VCS-ignored files when detecting local changes: it adds per-repo ignored-files helpers, threads ignore lists through check/update commands, updates VCS change detection to respect ignore lists, and updates tests and BDD steps accordingly.

Changes

Cohort / File(s)	Summary
Changelog CHANGELOG.rst	Added Release 0.11.0 entry: "Don't consider ignored files for determining local changes (#350)".
Command layer dfetch/commands/check.py, dfetch/commands/update.py, dfetch/commands/common.py	Compute per-project ignore lists via files_to_ignore(path) and pass files_to_ignore= into check_for_update / update calls.
VCS base & checks dfetch/project/vcs.py	update(..., files_to_ignore: Optional[Sequence[str]] = None) and check_for_update(..., files_to_ignore) usages added; _are_there_local_changes(files_to_ignore) now excludes ignored files when computing hashes.
Git implementation dfetch/vcs/git.py	Added GitLocalRepo.ignored_files(path: str) -> Sequence[str] using git ls-files --ignored --others --exclude-standard.
SVN implementation dfetch/project/svn.py	Added SvnRepo.ignored_files(path: str) -> Sequence[str] parsing svn status --no-ignore for "I" lines.
Common helper dfetch/commands/common.py	Added public files_to_ignore(path: str) -> Sequence[str] that delegates to Git/SVN helpers or returns empty list.
BDD feature tests features/guard-against-overwriting-git.feature, features/guard-against-overwriting-svn.feature, features/check-git-repo.feature, features/check-svn-repo.feature	New scenarios verifying skip-on-local-change, force override, and ignored-files behavior per VCS; added "ignored files are not reported" check scenarios.
Removed feature features/guard-against-overwriting.feature	Deleted generic feature (replaced by VCS-specific variants).
Behavioral test steps features/steps/generic_steps.py, features/steps/git_steps.py, features/steps/svn_steps.py	Added replace_in_file() and new steps to replace text and mark files changed locally; removed one Git step and added SVN tag creation step.
Unit tests tests/test_update.py, tests/test_vcs.py	Updated assertions/calls to include files_to_ignore=[] where local-change checks or update mocks are invoked.
Docs & tooling dfetch/commands/diff.py, dfetch/manifest/project.py, doc/conf.py, pyproject.toml	Documentation reorganized into Git/SVN tabs; added sphinx-tabs dependency and enabled extension in Sphinx config.

Sequence Diagram(s)

sequenceDiagram
participant CLI as dfetch CLI
participant Cmd as Command (update/check)
participant Common as files_to_ignore()
participant Repo as Repo (Git/SVN)
participant VCS as VCS layer
participant FS as Local FS
CLI->>Cmd: run update/check [--force]
Cmd->>Common: files_to_ignore(project.destination)
Common->>Repo: repo.ignored_files(project.destination)
Repo->>FS: query ignored files (git/svn)
FS-->>Repo: ignored files list
Repo-->>Common: ignored files
Common-->>Cmd: files_to_ignore list
Cmd->>VCS: update/check(force, files_to_ignore)
VCS->>VCS: _are_there_local_changes(files_to_ignore)
VCS->>FS: compute current tree hash (exclude files_to_ignore)
FS-->>VCS: computed hash
alt local changes detected and not forced
VCS-->>Cmd: report skipped (local changes)
Cmd-->>CLI: skipped message
else no local changes or forced
VCS->>Repo: fetch/update remote
Repo-->>VCS: fetched
VCS-->>Cmd: updated
Cmd-->>CLI: success message
end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• New files not part of dfetch diff generated patch #888: Similar work propagating ignore-list handling into VCS/diff/update flows and touching Git/SVN ignored-file logic.

Poem

🐰 I hopped through .git and svn with care,

Skipped the crumbs they hide, left them there,
Updates now mind what ignores say,
Force may stomp if you shout "OK!",
A joyful nibble, safe and fair.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 59.26% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: preventing ignored/untracked files from being included in the directory hash calculation when checking for local changes.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63d8e76 and f6978d1.

📒 Files selected for processing (21)

• CHANGELOG.rst
• dfetch/commands/check.py
• dfetch/commands/common.py
• dfetch/commands/diff.py
• dfetch/commands/update.py
• dfetch/manifest/project.py
• dfetch/project/svn.py
• dfetch/project/vcs.py
• dfetch/vcs/git.py
• doc/conf.py
• features/check-git-repo.feature
• features/check-svn-repo.feature
• features/guard-against-overwriting-git.feature
• features/guard-against-overwriting-svn.feature
• features/guard-against-overwriting.feature
• features/steps/generic_steps.py
• features/steps/git_steps.py
• features/steps/svn_steps.py
• pyproject.toml
• tests/test_update.py
• tests/test_vcs.py

💤 Files with no reviewable changes (2)

• features/guard-against-overwriting.feature
• features/steps/git_steps.py

✅ Files skipped from review due to trivial changes (1)

• dfetch/manifest/project.py

🚧 Files skipped from review as they are similar to previous changes (7)

• CHANGELOG.rst
• dfetch/vcs/git.py
• dfetch/project/svn.py
• tests/test_vcs.py
• features/guard-against-overwriting-svn.feature
• features/guard-against-overwriting-git.feature
• dfetch/commands/common.py

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 895
File: dfetch/commands/check.py:133-142
Timestamp: 2025-12-31T10:55:20.972Z
Learning: In dfetch, when determining ignored files for a project, the VCS detection should use the superproject's VCS type (where the manifest is located), not the fetched project's VCS type. The ignored files are then queried from the project's destination path using the superproject's VCS system.

📚 Learning: 2025-12-31T10:55:20.972Z

Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 895
File: dfetch/commands/check.py:133-142
Timestamp: 2025-12-31T10:55:20.972Z
Learning: In dfetch, when determining ignored files for a project, the VCS detection should use the superproject's VCS type (where the manifest is located), not the fetched project's VCS type. The ignored files are then queried from the project's destination path using the superproject's VCS system.

Applied to files:

• features/check-git-repo.feature
• features/check-svn-repo.feature
• dfetch/commands/update.py
• dfetch/commands/check.py
• dfetch/project/vcs.py

🧬 Code graph analysis (5)

tests/test_update.py (1)

dfetch/commands/common.py (1)

• files_to_ignore (71-79)

dfetch/commands/update.py (2)

dfetch/commands/common.py (1)

• files_to_ignore (71-79)

dfetch/project/vcs.py (1)

• update (95-146)

dfetch/commands/check.py (3)

dfetch/commands/common.py (1)

• files_to_ignore (71-79)

dfetch/project/vcs.py (1)

• check_for_update (159-193)

dfetch/manifest/project.py (1)

• destination (405-407)

dfetch/project/vcs.py (2)

dfetch/commands/common.py (1)

• files_to_ignore (71-79)

dfetch/project/abstract_check_reporter.py (1)

• AbstractCheckReporter (9-91)

features/steps/svn_steps.py (1)

dfetch/util/util.py (1)

• in_directory (66-75)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: test (windows-latest, 3.9)
• GitHub Check: test (windows-latest, 3.12)
• GitHub Check: test (windows-latest, 3.14)
• GitHub Check: test (windows-latest, 3.10)
• GitHub Check: test (windows-latest, 3.13)
• GitHub Check: test (windows-latest, 3.11)
• GitHub Check: test (macos-latest, 3.11)
• GitHub Check: test (macos-latest, 3.10)
• GitHub Check: test-cygwin
• GitHub Check: test
• GitHub Check: build (windows-latest)
• GitHub Check: build (macos-latest)
• GitHub Check: build (ubuntu-latest)
• GitHub Check: DevContainer Build & Test

🔇 Additional comments (17)

dfetch/commands/diff.py (1)

22-30: LGTM! Documentation presentation improved.

The tabbed interface for Git and SVN scenarios enhances the documentation by allowing users to view relevant examples side-by-side. This is a purely presentational improvement with no functional changes.

Also applies to: 40-52

features/steps/svn_steps.py (1)

93-96: LGTM! Test step follows established patterns.

The new step definition for adding SVN tags reuses the existing create_tag helper and follows the same pattern as other step definitions in this file.

tests/test_update.py (1)

68-68: LGTM! Test updated for new API.

The test correctly verifies that the update method is called with the new files_to_ignore parameter, reflecting the API changes introduced in this PR.

features/check-svn-repo.feature (1)

162-180: LGTM! Scenario validates ignore functionality.

This scenario properly tests that SVN-ignored files don't affect the up-to-date status of a project, which is the core fix for issue #350.

features/check-git-repo.feature (1)

140-157: LGTM! Git scenario mirrors SVN behavior.

This scenario validates that Git-ignored files don't affect project status, paralleling the SVN scenario and ensuring consistent behavior across VCS types.

dfetch/project/vcs.py (3)

95-112: LGTM! Clean API extension with proper defaults.

The update method now accepts an optional files_to_ignore parameter that defaults to an empty list, maintaining backward compatibility while enabling the new ignore functionality. The parameter is properly propagated to _are_there_local_changes.

---

159-187: LGTM! Consistent propagation through check flow.

The check_for_update method correctly accepts and propagates the files_to_ignore parameter to the local changes detection, ensuring ignored files don't trigger false positives during checks.

---

349-361: LGTM! Correct implementation of ignore logic.

The _are_there_local_changes method properly incorporates files_to_ignore into the skiplist when computing the directory hash, ensuring ignored files are excluded from local change detection.

dfetch/commands/update.py (2)

8-16: LGTM! Documentation presentation improved.

The tabbed interface provides clearer side-by-side documentation for Git and SVN workflows.

---

40-40: LGTM! Proper integration of ignore functionality.

The files_to_ignore function is correctly imported and called with project.destination to determine which files can be ignored during updates. Based on learnings, this correctly uses the superproject's VCS type to query ignored files from the project's destination path.

Also applies to: 89-92

dfetch/commands/check.py (2)

8-16: LGTM! Documentation presentation improved.

The tabbed interface enhances documentation clarity by presenting Git and SVN scenarios side-by-side.

---

37-37: LGTM! Consistent ignore functionality integration.

The check command mirrors the update command's approach: importing files_to_ignore and calling it with project.destination to obtain per-project ignore lists. This ensures consistent behavior across check and update operations. Based on learnings, this correctly uses the superproject's VCS type to query ignored files from the project's destination path.

Also applies to: 101-103

features/steps/generic_steps.py (3)

124-132: LGTM!

The implementation is clean and follows the same pattern as existing file utilities in this module. Using UTF-8 encoding is consistent with the rest of the codebase.

---

157-160: LGTM!

The step definition follows the established pattern in this file. Using only @given is appropriate if this step is intended solely for test setup/precondition contexts.

---

205-209: LGTM!

Adding the @given decorator alongside the existing @when allows this step to be used in both setup and action contexts, which is a common and useful Behave pattern consistent with other dual-decorator steps in this file (e.g., lines 167-168, 188-189).

doc/conf.py (1)

42-42: LGTM! Extension properly configured.

The sphinx_tabs.tabs extension is correctly added to enable tabbed content in documentation, and the corresponding dependency is declared in pyproject.toml.

pyproject.toml (1)

100-100: Version is current and secure.

The sphinx-tabs==3.4.7 dependency is the latest available version on PyPI and is actively used in the documentation via the sphinx_tabs.tabs extension in doc/conf.py. No known security vulnerabilities exist for this version.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

dfetch/commands/update.py (1)

191-201: Extract _files_to_ignore to dfetch/commands/common.py.

This static method is duplicated identically in both update.py and check.py. Move it to the existing common.py module and update imports in both files.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60090e0 and 0d3b3bc.

📒 Files selected for processing (14)

• CHANGELOG.rst
• dfetch/commands/check.py
• dfetch/commands/update.py
• dfetch/project/svn.py
• dfetch/project/vcs.py
• dfetch/vcs/git.py
• features/guard-against-overwriting-git.feature
• features/guard-against-overwriting-svn.feature
• features/guard-against-overwriting.feature
• features/steps/generic_steps.py
• features/steps/git_steps.py
• features/steps/svn_steps.py
• tests/test_update.py
• tests/test_vcs.py

💤 Files with no reviewable changes (2)

• features/guard-against-overwriting.feature
• features/steps/git_steps.py

🧰 Additional context used

🧬 Code graph analysis (6)

dfetch/vcs/git.py (2)

dfetch/util/util.py (1)

• in_directory (66-75)

dfetch/util/cmdline.py (1)

• run_on_cmdline (38-69)

dfetch/project/vcs.py (1)

dfetch/project/abstract_check_reporter.py (1)

• AbstractCheckReporter (9-91)

dfetch/project/svn.py (3)

dfetch/vcs/git.py (1)

• ignored_files (348-368)

dfetch/util/util.py (1)

• in_directory (66-75)

dfetch/util/cmdline.py (1)

• run_on_cmdline (38-69)

tests/test_vcs.py (2)

dfetch/manifest/project.py (1)

• vcs (406-408)

dfetch/project/vcs.py (1)

• _are_there_local_changes (349-361)

features/steps/svn_steps.py (1)

dfetch/util/util.py (1)

• in_directory (66-75)

dfetch/commands/update.py (6)

dfetch/project/svn.py (3)

• SvnRepo (42-450)
• ignored_files (435-450)
• check_path (146-153)

dfetch/manifest/project.py (2)

• vcs (406-408)
• destination (381-383)

dfetch/vcs/git.py (4)

• GitLocalRepo (175-501)
• is_git (45-61)
• is_git (184-191)
• ignored_files (348-368)

dfetch/commands/check.py (1)

• _files_to_ignore (134-143)

dfetch/project/__init__.py (1)

• make (11-22)

dfetch/project/vcs.py (1)

• update (95-146)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Publish Python distribution 📦 to TestPyPI
• GitHub Check: test (windows-latest, 3.11)
• GitHub Check: test (macos-latest, 3.12)
• GitHub Check: test (ubuntu-latest, 3.10)
• GitHub Check: test (macos-latest, 3.9)
• GitHub Check: test (windows-latest, 3.14)
• GitHub Check: test (windows-latest, 3.9)
• GitHub Check: test (ubuntu-latest, 3.12)
• GitHub Check: test (windows-latest, 3.12)
• GitHub Check: test (windows-latest, 3.10)
• GitHub Check: test (ubuntu-latest, 3.9)
• GitHub Check: build (windows-latest)
• GitHub Check: build (ubuntu-latest)
• GitHub Check: test-cygwin
• GitHub Check: build (macos-latest)
• GitHub Check: DevContainer Build & Test
• GitHub Check: test

🔇 Additional comments (18)

features/steps/svn_steps.py (1)

93-96: LGTM!

The new SVN tag step implementation is clean and follows established patterns in the file. It correctly uses the in_directory context manager and delegates to the existing create_tag helper.

CHANGELOG.rst (1)

22-22: LGTM!

The changelog entry clearly documents the fix and properly references issue #350.

tests/test_update.py (1)

68-68: LGTM!

The test correctly validates that the update method receives the new files_to_ignore parameter with an empty list as the default value.

dfetch/vcs/git.py (1)

347-368: LGTM!

The ignored_files implementation correctly uses git ls-files --ignored --others --exclude-standard to enumerate ignored files. The method properly handles non-existent paths and follows the established patterns in the codebase.

dfetch/commands/check.py (2)

24-24: LGTM!

The new imports are appropriately placed and support the ignore-file handling functionality.

Also applies to: 33-33, 40-40

---

96-99: LGTM!

The check flow correctly computes and passes the files_to_ignore list to check_for_update, assuming the VCS detection issue in _files_to_ignore is fixed.

features/guard-against-overwriting-git.feature (1)

1-47: LGTM!

The BDD feature file is well-structured and comprehensively covers the guard-against-overwriting behavior for Git repositories. The three scenarios appropriately test: normal blocking on local changes, force override, and ignored files not blocking updates.

dfetch/project/svn.py (1)

434-450: LGTM!

The ignored_files implementation correctly uses svn status --no-ignore to enumerate ignored files (marked with "I" status). The method properly handles non-existent paths and follows established patterns in the codebase, maintaining consistency with the Git equivalent.

features/guard-against-overwriting-svn.feature (2)

39-39: Note the @wip tag on the ignored files scenario.

The third scenario is marked as work-in-progress. This is acceptable, though you may want to verify that the underlying functionality works even if the test is not yet complete.

---

1-48: LGTM!

The BDD feature file appropriately covers the guard-against-overwriting behavior for SVN repositories, mirroring the Git equivalent. The first two scenarios are complete and should provide good coverage.

tests/test_vcs.py (1)

140-140: LGTM!

The test correctly passes an empty files_to_ignore list to match the updated method signature. This is appropriate since these test cases focus on hash comparison behavior, not the ignore functionality.

dfetch/project/vcs.py (3)

95-116: LGTM!

The update() method correctly:

1. Accepts an optional files_to_ignore parameter with None default
2. Defaults to an empty list when not provided
3. Passes the ignore list to _are_there_local_changes() to exclude VCS-ignored files from the local change detection

This properly addresses the issue where untracked/ignored files were incorrectly causing the directory hash to differ.

---

159-161: LGTM!

The check_for_update() method now requires files_to_ignore as a mandatory parameter, which is appropriate since the caller (check.py) always computes this list before calling.

---

349-361: LGTM!

The core fix is implemented correctly here. By adding files_to_ignore to the skiplist passed to hash_directory(), VCS-ignored files (e.g., files matching .gitignore patterns) are now excluded from the hash calculation. This directly resolves the issue where untracked/ignored files incorrectly affected local change detection.

features/steps/generic_steps.py (3)

124-132: LGTM!

The replace_in_file() utility is a clean helper for BDD tests. The implementation is straightforward and appropriate for test code where files are expected to exist.

---

157-159: LGTM!

The new step definition provides a convenient way to modify file contents in feature tests, enabling scenarios that test the ignore-file behavior.

---

205-209: LGTM!

Adding the @given decorator alongside the existing @when decorator provides flexibility for test scenarios to set up local file changes either as preconditions or actions.

dfetch/commands/update.py (1)

84-87: LGTM!

The integration correctly computes the files to ignore before calling update(), ensuring VCS-ignored files don't affect local change detection.