fix: propagate executable-bit changes on copier update#2605
fix: propagate executable-bit changes on copier update#2605willemkokke wants to merge 1 commit intocopier-org:masterfrom
copier update#2605Conversation
sisp
left a comment
There was a problem hiding this comment.
Thanks for discovering this bug and contributing a fix, @willemkokke! 🙇 I've left a couple of remarks.
|
All very valid, thanks! Will update my PR now |
db63917 to
c8d6108
Compare
|
Thanks for the thorough review, @sisp! I've addressed all your feedback. Here's a summary of the changes:
Intentionally kept |
|
Thanks for updating the PR, @willemkokke! 👍 I noticed that running |
|
Thanks for raising this, @sisp! You're right on both counts — I did some investigation and here's what I found: 1. Auto-staging with
|
|
Sounds reasonable, let's go ahead as you suggested. |
c8d6108 to
667845a
Compare
`copier update` did not reliably propagate template executable-bit (file mode) changes to destinations: 1. **Mode-only template change**: `_render_allowed` classified the file as "identical" (content matched) and `_render_file` short-circuited before reaching the `chmod` call — a complete silent no-op. 2. **Mode + content change on `core.fileMode=false`** (Windows default): copier wrote the new content and `chmod`d the file on disk, but the destination's git index was never updated. Since git ignores on-disk mode bits when `core.fileMode=false`, the executable bit was silently lost on the user's next commit. 3. **Template authored on Linux/macOS, consumed on Windows**: copier read the template's intended mode from `Path.stat().st_mode`, which on Windows never carries UNIX exec bits. Templates committed with `100755` on Linux were indistinguishable from `100644` when rendered on Windows, so the executable bit was lost end-to-end. This commit fixes all three: - Thread an `expected_mode` parameter into `_render_allowed` so that a mode-only change (comparing only the executable bits, `0o111`) is no longer misclassified as "identical". - Add a `Template.git_index_modes` cached property that reads file modes directly from the template clone's git index via `git ls-files --stage`. `_render_file` now prefers this intended mode over `stat().st_mode`, which makes the fix work even on filesystems that don't represent exec bits on disk. - After the existing on-disk `chmod` in `_render_file`, call a new `_sync_git_index_executable_bit` helper that rewrites the destination's index entry when `core.fileMode=false`. It uses `git update-index --cacheinfo <mode>,<sha>,<path>` (not `--chmod`), which rewrites *only* the mode on the existing blob SHA. `--chmod` looks simpler but has a hidden side effect of re-staging the current working-tree content as the new blob, which would stomp the downstream-edited blob that `_apply_update` needs to reconstruct merge conflicts. When `core.fileMode=true` the method is a no-op — git picks up the on-disk `chmod` naturally as an unstaged modification, matching copier's normal "leave changes unstaged for user review" behavior. Adds three new tests: - `test_update_propagates_executable_bit_addition`: parametrized over `core.fileMode=[true, false]`. The `true` case asserts the change is unstaged after update (guards against auto-staging regression); the `false` case asserts the index records `100755`. - `test_update_propagates_executable_bit_removal`: symmetric. - `test_update_preserves_exec_bit_authored_on_unix`: Windows-only test that sets up the template with `git update-index --chmod` (no on-disk chmod, mimicking a clone from a Linux-authored commit) and verifies the exec bit propagates end-to-end on `core.fileMode=false`. - `test_update_with_exec_bit_change_and_merge_conflict`: parametrized regression test that locks in the ordering guarantee: during `copier update`, `_sync_git_index_executable_bit` runs in `_render_file` *before* the conflict registration in `_apply_update`, and uses `--cacheinfo` so it does not corrupt the conflict stages. Fixes copier-org#2604
667845a to
6b53f4c
Compare
|
Hey @sisp — update on this PR. The second round of feedback prompted a deeper investigation that uncovered a few things worth flagging, and the fix is now broader than originally planned. TL;DR: cross-platform support is in, conflict corruption is guarded against, and I verified on Windows. What changed from the previous plan I outlined1.
|
Summary
Fixes #2604.
copier updatedid not reliably propagate template executable-bit (file mode) changes to destinations. Three distinct failure modes:_render_allowedclassified the file as "identical" and_render_fileshort-circuited before reaching thechmodcall — a silent no-op on every platform.core.fileMode=false(Windows default): copier wrote the new content andchmodd the file on disk, but the destination's git index was never updated. Since git ignores on-disk mode bits whencore.fileMode=false, the executable bit was silently lost on the user's next commit.Path.stat().st_mode, which on Windows never carries UNIX exec bits. Templates committed with100755on Linux were indistinguishable from100644when rendered on Windows, so the executable bit was lost end-to-end.Changes
copier/_main.py:_render_allowed: newexpected_modeparameter. Compares only the executable bits (0o111) against the existing destination file so that a mode-only template change is no longer misclassified as "identical"._render_file: now consultsself.template.git_index_modesfor exec-bit determination, merging the git-tracked exec bits with the filesystem's non-exec flags (fall back to purestat()for non-git templates and untracked files). This makes the fix work even on filesystems that don't represent exec bits on disk._sync_git_index_executable_bit(new): runs only whencore.fileMode=false. Usesgit update-index --cacheinfo <mode>,<sha>,<path>(not--chmod), which rewrites only the mode on the existing blob SHA.--chmodlooks simpler but has a hidden side effect of re-staging the current working-tree content as the new blob, which would stomp the downstream-edited blob that_apply_updateneeds to reconstruct merge conflicts. Whencore.fileMode=truethe method is a no-op — git picks up the on-diskchmodnaturally as an unstaged modification, matching copier's normal "leave changes unstaged for user review" behavior. All git failures are silently swallowed so the render path cannot be broken by unrelated git problems.copier/_template.py:Template.git_index_modescached property: reads file modes directly from the template clone's git index via a singlegit ls-files --stagecall. Returns an empty mapping for non-git templates, silently falling back tostat()callers.tests/test_updatediff.py— new regression tests:test_update_propagates_executable_bit_addition[True|False]+x, no content change), parametrized overcore.fileMode. TheTruecase asserts the change is unstaged after update, guarding against auto-staging regression; theFalsecase asserts the index records100755.test_update_propagates_executable_bit_removal[True|False]+x, destination index/working tree must lose the exec bit.test_update_preserves_exec_bit_authored_on_unixgit update-index --chmod(noPath.chmod) and verifies the exec bit survives all the way into the destination's git index oncore.fileMode=false— exercising theTemplate.git_index_modescode path that's load-bearing on Windows.test_update_with_exec_bit_change_and_merge_conflict[True|False]+xbetween template versions, parametrized overcore.fileMode. Asserts the conflict is properly registered (UU, conflict markers, stages 1/2/3 present). Locks in the ordering guarantee that_sync_git_index_executable_bitruns in_render_filebefore the conflict registration in_apply_update, and that--cacheinfodoes not corrupt the conflict stages.Test plan
test_update_preserves_exec_bit_authored_on_unixpasses on a real Windows machine (first run — previously only reasoned about)tests/test_symlinks.pyandtests/test_dirty_local.pyreproduce identically onmasteron the same machine (local environment issues, unrelated to this PR)