Skip to content

Fix submodule skip directory matching#1352

Open
JamBalaya56562 wants to merge 1 commit into
facebook:mainfrom
JamBalaya56562:codex/fix-submodule-skip-dir-matching
Open

Fix submodule skip directory matching#1352
JamBalaya56562 wants to merge 1 commit into
facebook:mainfrom
JamBalaya56562:codex/fix-submodule-skip-dir-matching

Conversation

@JamBalaya56562

Copy link
Copy Markdown
Contributor

Summary

This fixes a false-positive sl status deletion report when a repository has a root submodule path whose basename also appears in a normal nested directory.

The physical working copy walker receives submodule paths as directories to skip. It was comparing those skip entries against the current directory entry basename, so a root submodule such as examples also caused tests/misc/ino2cpp/examples to be skipped. Tracked files under the skipped nested directory were then reported as deleted.

Changes

  • Compare skip directories against the root-relative candidate_path instead of the entry basename.
  • Add a walker unit test that skips root examples while still walking tests/misc/ino2cpp/examples.

Validation

  • mise exec rust@nightly -- cmd /v:on /c "set CARGO_HOME=C:\c&& cargo fmt --check -p sapling-workingcopy"
  • Attempted mise exec rust@nightly -- cmd /v:on /c "set CARGO_HOME=C:\c&& cargo test -p sapling-workingcopy --lib test_multiwalker_skip_dirs_are_repo_relative -- --exact", but the local Windows build stopped before running the test because openssl-sys could not find an OpenSSL/vcpkg installation.

@meta-cla meta-cla Bot added the CLA Signed label Jul 5, 2026
@meta-codesync

meta-codesync Bot commented Jul 5, 2026

Copy link
Copy Markdown

This pull request has been imported. If you are a Meta employee, you can view this in D110681347. (Because this pull request was imported automatically, there will not be any future comments.)

@JamBalaya56562 JamBalaya56562 marked this pull request as ready for review July 5, 2026 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant