Skip to content

feat(updating): propagate exec bit on new template files when core.fileMode=false#2640

Open
willemkokke wants to merge 1 commit into
copier-org:masterfrom
willemkokke:feat/intent-to-add-new-files-with-exec-bit
Open

feat(updating): propagate exec bit on new template files when core.fileMode=false#2640
willemkokke wants to merge 1 commit into
copier-org:masterfrom
willemkokke:feat/intent-to-add-new-files-with-exec-bit

Conversation

@willemkokke

Copy link
Copy Markdown
Contributor

Summary

Closes #2630. Follow-up to #2605.

#2605 fixed executable-bit propagation for files that already exist in the destination. Files introduced by the template for the first time on copier update were still affected when core.fileMode=false:

  1. Template v2 adds a new script.sh with 100755 in the index.
  2. Destination has core.fileMode=false (Windows default).
  3. copier writes the file, chmods it on disk (invisible to git with fileMode=false), then calls _sync_git_index_executable_bit — which short-circuited because the file is untracked.
  4. The user's next git add script.sh records 100644 because git ignores on-disk mode with fileMode=false. Exec bit lost.

Changes

copier/_main.py_sync_git_index_executable_bit:

For untracked files that should be executable, run git add --intent-to-add <path> to register an empty index entry, then fall through to the existing --cacheinfo path to set the mode to 100755. The user's eventual git add replaces the empty blob with the file's actual content while preserving the mode bit. The intent-to-add marker (A in git status) is also a more accurate signal than ?? for files that are deliberate template output.

  • Skipped when the file does not need an executable bit (the user's git add would record 100644 either way).
  • Skipped when core.fileMode=true (git picks up the on-disk chmod naturally — pre-staging would be inconsistent with copier's "leave changes unstaged for user review" behavior).

tests/test_updatediff.py — 3 new tests:

Test What it covers
test_update_introduces_new_executable_file[True|False] New +x file in template v2: index records 100755 on fileMode=false; file stays untracked on fileMode=true
test_update_introduces_new_non_executable_file[True|False] New non-exec file is NOT pre-staged in either fileMode case
test_update_introduces_new_executable_file_then_user_commits End-to-end on core.fileMode=false: copier update → git addgit commit → committed tree records 100755 and the file's actual content. The template setup uses git update-index --chmod=+x (rather than Path.chmod) so the test runs identically on every platform — Linux, macOS, and Windows.

Test plan

  • All new tests pass on macOS and windows
  • Full test suite still green on macOS (1073 passed)
  • ruff check, ruff format, mypy, editorconfig-checker all clean

@codecov

codecov Bot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.17%. Comparing base (4e969bf) to head (830bd06).
⚠️ Report is 78 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #2640    +/-   ##
========================================
  Coverage   97.17%   97.17%            
========================================
  Files          60       60            
  Lines        7147     7269   +122     
========================================
+ Hits         6945     7064   +119     
- Misses        202      205     +3     
Flag Coverage Δ
unittests 97.17% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sisp

sisp commented Apr 30, 2026

Copy link
Copy Markdown
Member

Correct me if I'm wrong, but doesn't this PR only mark files added by the update algorithm as intended to be added if core.fileMode=false. My original intent was to mark all files added by an update as intended to be added, regardless of the file mode and changes in the executable bit.

@willemkokke

Copy link
Copy Markdown
Contributor Author

Ah I misunderstood then, I thought we wanted to deviate as little as possible from existing behaviour. Happy to adjust, brb!

@willemkokke
willemkokke force-pushed the feat/intent-to-add-new-files-with-exec-bit branch from 88beeaa to 43892eb Compare April 30, 2026 15:39
@willemkokke

Copy link
Copy Markdown
Contributor Author

I've adjusted the PR to always intent-to-add new files:

copier/_main.py — split into two helpers:

  • New _register_in_git_index: runs git add --intent-to-add on any newly-rendered untracked file, regardless of core.fileMode or executable bit. Silently no-ops if the file is gitignored, the destination isn't a git repo, or git is unavailable.
  • _sync_git_index_executable_bit: simplified — it now just rewrites the index entry's mode via --cacheinfo when core.fileMode=false, relying on the entry that _register_in_git_index left behind. The intent-to-add fallback was removed.

Tests — updated to match:

  • test_update_introduces_new_executable_file[True\|False] now asserts both parametrizations land at 100755 in the index. The two cases exercise different paths to get there: with fileMode=true the intent-to-add call reads the on-disk chmod directly; with fileMode=false Copier additionally rewrites the entry's mode (the on-disk chmod is invisible there).
  • test_update_introduces_new_non_executable_file[True\|False] now asserts the file is intent-to-add'd at 100644 (previously asserted untracked).
  • New test_update_introduces_new_file_can_be_aborted regression-tests the aborting an update flow: git reset drops the intent-to-add markers so git clean picks the files up like any other untracked content. There were no existing tests for the abort flow at all, which seemed worth fixing while we're here.

docs/updating.md — new section "How Copier registers new files in your Git index" explaining the user-visible behavior change (A instead of ?? in git status), why it matters on Windows/core.fileMode=false, and that the abort flow still works the same way.

Verified locally: 1077 passed, 5 skipped, 8 xfailed, 1 xpassed on macOS. ruff/mypy/editorconfig-checker all clean.

@willemkokke
willemkokke force-pushed the feat/intent-to-add-new-files-with-exec-bit branch from 43892eb to 91797c0 Compare April 30, 2026 15:55
…leMode=false

Files newly introduced by the template on `copier update` were not yet
tracked in the destination's index, so the existing
`_sync_git_index_executable_bit` short-circuited for them. With
`core.fileMode=false` (Windows default) the user's eventual `git add`
would record `100644` regardless of the template's intended mode,
silently losing the executable bit.

When an untracked file with desired exec bits is rendered into a
`core.fileMode=false` repo, copier now registers an intent-to-add
entry via `git add --intent-to-add` and rewrites its mode to `100755`
via `update-index --cacheinfo`. The empty blob is preserved, so the
user's eventual `git add` replaces it with the file's actual content
while preserving `100755`. The intent-to-add marker (`A` in `git
status`) is also a more accurate signal than `??` for files that are
deliberate template output rather than random untracked content.

When `core.fileMode=true`, copier still leaves new files untracked —
git would have picked up the on-disk chmod naturally, and pre-staging
would be inconsistent with copier's normal "leave rendered changes
unstaged for user review" behavior.

Closes copier-org#2630
@willemkokke
willemkokke force-pushed the feat/intent-to-add-new-files-with-exec-bit branch from 91797c0 to 830bd06 Compare April 30, 2026 16:02

@sisp sisp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to my inline comment, I've been giving this PR some more thought. Once #2376 lands, the update algorithm will use git merge instead of a complex sequence of low-level git commands. git merge automatically tracks new files (added in the source branch), so marking new files with "intent-to-add" would be inconsistent with the future (and arguably more correct) behavior. This means, auto-staging will naturally occur even when core.fileMode=true (we currently assert files to remain unstaged)

if file_mode:
# With ``core.fileMode=true``, copier must NOT stage the index mode
# change — git picks up the on-disk ``chmod`` as an unstaged
# modification, matching copier's normal behavior of leaving
# rendered changes unstaged for user review. Guards against
# auto-staging regression.
assert mode == "100644"
assert status.startswith(" M"), f"expected unstaged change, got {status!r}"

so the future behavior will be fully consistent for both core.fileMode=true and core.fileMode=false.

However, the Worker._render_file method is currently not aware of the operation mode, so staging (with or without "intent-to-add") applies to both initial copying and updating. I wonder what the best behavior in anticipation of #2376 is. 🤔

And another note that I had missed when reviewing #2605: We need to support copier copy even when git is not installed. I've restored this regression with #2651.

Comment thread copier/_main.py
# TODO: simplify with ``--format %(objectmode)`` once the
# minimum git version is raised to 2.38+ (--format cannot be
# combined with --stage; see git-ls-files(1)).
result = git("ls-files", "--stage", "--", str(dst_relpath)).strip()

@sisp sisp May 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check is not sufficient because it also passes for files with merge conflicts which shouldn't be marked with "intent-to-add" because it changes the state. Perhaps we can check the status with git status --porcelain=v1 <dst_relpath> and only proceed for files without merge conflicts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the existing check already excludes conflicted files, so I'd argue the porcelain check is redundant — but happy to add it as belt-and-suspenders if you prefer.

Conflicted files are tracked: they have stages 1, 2, and 3 in the index. git ls-files --stage <path> returns three lines for them (one per stage), so result is non-empty and the early return at line 932 triggers. The cases are:

  • Tracked, clean: 100644 abc 0\tfile (1 line) → return
  • Tracked, conflicted: stages 1/2/3 (3 lines) → return
  • Untracked: empty → proceed to intent-to-add

The function is called from _render_file for every rendered file, but only untracked files reach the git add --intent-to-add line.

As a backstop, git add --intent-to-add itself refuses to run on conflicted files (fatal: cannot add the path '<file>' because of a merge conflict), and our try/except (OSError, ProcessExecutionError) would swallow that.

That said: under the current update flow, I don't think _register_in_git_index can even be reached with a conflicted file. Conflicts get registered at L1487-L1556 of _apply_update, which runs after current_worker.run_copy() (and thus after _render_file for every file). So at the time our function runs, the index has only stage 0 entries.

Let me know if there's a scenario I'm missing — happy to swap in git status --porcelain=v1 if you'd prefer.

@willemkokke

Copy link
Copy Markdown
Contributor Author

#2376 looks great — leveraging git merge directly is much cleaner than the low-level git ls-files / update-index plumbing we're adding here.

I had a look and ported the exec-bit tests from #2605 and this PR onto refactor/new-update-algorithm to see what holds up. Most of them pass cleanly — the git merge-based flow handles exec-bit propagation natively (correct mode preserved through git's trees regardless of core.fileMode), and new files get auto-staged with real content. That's strictly better than the intent-to-add empty-blob marker we're adding here.

Concrete results running the tests on refactor/new-update-algorithm:

  • executable_bit_addition[False] and executable_bit_removal[False] (the core core.fileMode=false propagation cases): ✅ pass
  • All four introduces_new_* tests: ✅ pass
  • introduces_new_executable_file_then_user_commits (end-to-end): ✅ pass
  • introduces_new_file_can_be_aborted (the abort flow): ✅ pass
  • executable_bit_addition[True] / executable_bit_removal[True] fail only because they assert "remain unstaged" — no longer true under refactor!: introduce new update algorithm based on git merge #2376's auto-staging. Behavior is correct, assertion is stale.
  • The merge-conflict tests fail purely on the run_update(conflict=...) API removal.

So you're right: the git_index_modes / _sync_git_index_executable_bit / _register_in_git_index machinery from #2605 + this PR becomes redundant under #2376. Once #2376 lands, all of it can be ripped out.

Happy to open a follow-up PR against refactor/new-update-algorithm that ports the exec-bit propagation and new-file behavior tests (with the stale assertions updated for the auto-staging semantics, and the conflict-test API calls adjusted) — most of the coverage carries over with minor changes.

One observation while I was poking around: refactor/new-update-algorithm is currently ~78 commits behind master (last touched 2026-03-16) and would have a non-trivial rebase against #2605 and a few other shipped fixes (#2651, the git clone cleanup, the PathSpec backslash fix, etc.). Is there anything blocking it, or anything I can help with to get it landed? I'm happy to take on rebase work, conflict resolution, or specific items if you have a list of what's left.

@sisp

sisp commented Jun 12, 2026

Copy link
Copy Markdown
Member

It turns out that the test suite (specifically tests/test_updatediff.py::test_update_propagates_executable_bit_removal[False]) of #2541 is failing because the internal fresh copies of the old and new template versions are Git repos there and

result = git("ls-files", "--stage", "--", str(dst_relpath)).strip()

returns an empty string because none of the files are staged in those empty repos. This leads to incorrect permission updates:

copier/copier/_main.py

Lines 1621 to 1646 in b80196e

perms_sha_mode, path = line.split("\t")
perms, sha, _ = perms_sha_mode.split()
input_lines.append(f"0 {'0' * 40}\t{path}")
input_lines.append(f"{perms} {sha} 2\t{path}")
with suppress(ProcessExecutionError):
# The following command will fail
# if the file did not exist in the previous version.
old_sha = git(
"hash-object",
"-w",
old_path / normalize_git_path(path),
).strip()
input_lines.append(f"{perms} {old_sha} 1\t{path}")
with suppress(ProcessExecutionError):
# The following command will fail
# if the file was deleted in the latest version.
new_sha = git(
"hash-object",
"-w",
new_path / normalize_git_path(path),
).strip()
input_lines.append(f"{perms} {new_sha} 3\t{path}")
(
git["update-index", "--index-info"]
<< "\n".join(input_lines)
)()

Staging (not just as intent-to-add) all rendered files would fix the failing test. It would also imitate a regular Git 3-way merge better as we discussed before. What do you think about staging all rendered files (if the destination is a Git repo)? This would also propagate the exec bit on new template files as well as on newly generated projects whose destination is already a Git repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

copier update loses executable bit on NEW files introduced by the template when core.fileMode=false

2 participants