doc: Document executable-bit behavior during checkout#2614
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR clarifies the behavior of Options::overwrite_existing during checkout, specifically that metadata (like the executable bit) may still be updated even when file contents are not overwritten.
Changes:
- Expanded the doc comment for
overwrite_existingto document a metadata-update edge case - Clarified interaction with
destination_is_initially_empty
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47fbe95a42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
|
Thanks a lot. Even though my comment here mentioned that it shouldn't touch metadata at all in this case, I think it's good to start with a documentation update. The prior CI failure is strange and I have a feeling it might be a race condition of sorts, related to packs, `hyperfine --runs 100 '../target/debug/deps/odb-be2b33301340af5a multi_index_access' didn't reproduce the issue which seems to consistently happen here. Rebasing, maybe it's just a bad commit somehow. |
47fbe95 to
2b60381
Compare
Reported issue
checkout::Options::overwrite_existingcurrently documents that existing worktree entries won't be overwritten unless the option is enabled. However, it doesn't mention that in a non-exclusive checkout, pre-existing files may still have their executable bit updated to match the index even whenoverwrite_existingisfalse.This can be surprising because callers may interpret
overwrite_existing = falseas preventing all changes to existing files, while the current behavior only prevents overwriting file contents.Closes #1787.
Summary
checkout::Options::overwrite_existing.destination_is_initially_emptyisfalse, existing files may still receive executable-bit updates even ifoverwrite_existingisfalse.Validation
cargo fmtI also tried to run
cargo check -p gix-worktree-state, but it failed while compilinggix-hashwithout hash features, which appears unrelated to this documentation-only change.