Skip to content

fix: use destination path as working directory while rendering#2754

Open
nikzart wants to merge 2 commits into
copier-org:masterfrom
nikzart:fix/render-cwd-dst-path
Open

fix: use destination path as working directory while rendering#2754
nikzart wants to merge 2 commits into
copier-org:masterfrom
nikzart:fix/render-cwd-dst-path

Conversation

@nikzart

@nikzart nikzart commented Jul 5, 2026

Copy link
Copy Markdown

Fixes #1708.

What

While rendering a subproject through copier {copy,recopy,update}, the working directory was whatever directory Copier was invoked from, so filesystem-related Jinja filters and extensions (e.g. the fileglob filter from the always-loaded jinja2_ansible_filters.AnsibleCoreFiltersExtension) operated relative to the invocation directory. With this PR, rendering happens with the destination path as the working directory, so the reproducible example from #1708 produces the expected output:

$ copier copy src/ dst/
$ cat dst/fileglob.txt
['a.txt', 'b.txt']

Design decisions

Since #1708 ended with "WDYT?", here are the choices made, explicitly:

  • Scope: the working-directory change wraps the whole render loop in _render_template (both path-name and content rendering), so it applies to copy, recopy, and update — all three renders of the update flow go through run_copy, so the temporary old/new copies render with the working directory at their own temporary destination, and the render into the real destination runs with the working directory there. Tasks and migrations already run with the destination as working directory, so this aligns rendering with the rest of the codebase.
  • Destination creation: the destination folder is now created eagerly at the start of rendering, because it must exist to change into it. This matches run_copy's documented behavior ("If dst_path was missing, it will be created"); the only observable difference is that a template which renders zero files now still creates the (empty) destination folder.
  • Pretend mode: never creates the destination. If the destination already exists, rendering happens with the working directory there, like a real run; if it is missing, the working directory is left unchanged, since creating it would violate --pretend.
  • External data: _external_data now resolves against the absolute destination path instead of the possibly-relative dst_path, because external data may be loaded lazily during rendering, when the working directory is the destination and a relative dst_path would no longer resolve correctly. No behavior change otherwise.
  • Non-determinism caveat: as noted in the issue, files are generated in no particular order, so destination-relative filesystem operations may or may not observe files generated earlier in the same run. This is called out in a new docs note under jinja_extensions.

Tests

  • copy: working directory is the destination while rendering, with both absolute and relative dst_path, and the invocation directory is restored afterwards (a decoy file next to the invocation directory guards against regressions).
  • update: working directory is the real destination while rendering into it.
  • pretend: a missing destination is not created.
  • external data loading lazily during rendering with a relative dst_path.

Full test suite passes locally (1143 passed), as do mypy, ruff check, ruff format, and codespell.


@sopw686 asked about working on this back in June — apologies if there's any overlap; happy to defer to your PR if you have one in progress.

While rendering a subproject, the working directory was the directory
Copier was invoked from, so filesystem-related Jinja filters and
extensions (e.g. `fileglob`) operated relative to it. Change the
working directory to the destination path while rendering, creating it
eagerly (except in pretend mode) so this also applies when copying into
a not-yet-existing folder.

External data is now loaded from the absolute destination path, because
it may be loaded lazily while rendering, when the working directory is
the destination path and a relative `dst_path` would no longer resolve
correctly.

Fixes copier-org#1708

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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.

Thanks for taking the time to tackle #1708. It looks pretty solid, but I have a conceptual question.

Comment thread tests/test_updatediff.py Outdated
cwd_before = Path.cwd()
run_update(dst, defaults=True, overwrite=True)
assert Path.cwd() == cwd_before
assert (dst / "fileglob.txt").read_text() == "['a.txt', 'version.txt']"

@sisp sisp Jul 6, 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 wonder whether a.txt should be listed by the fileglob filter during an update, especially considering #2376 which only replays a fresh copy based on the old template, renders a fresh copy based on the new template, and merges the latter into HEAD given the former, which is the common ancestor (i.e., a "simple" 3-way merge). The new update algorithm implementation in #2376 never copies into the user-specified destination unlike the current update algorithm implementation – and I think that's the only reason why a.txt is listed by fileglob here. When we switch to a final version of #2376, the behavior asserted by this PR might change.

Perhaps the more important question is even: Should the CWD be the user-specified destination during an update, or should it be the destination where files are being rendered? The latter seems more consistent and predictable and less susceptible to unexpected side-effects to me at first glance. Also, updating currently relies on replaying a fresh copy based on the old template, but from my perspective this is conceptually flawed – restoring a cached old copy would be correct IMO – although it's a mostly sufficient approximation of the correct approach. If we ever find/implement a good way cache and restore that cached old copy, setting the CWD to the destination where files are being rendered when replaying the fresh copy is more likely to yield the same result because the replayed copy is not affected by the content of the user-specified destination. We already have this side-effect problem with loading external data from the user-specified destination.

WDYT?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point — and I think we're aligned: the chdir lives inside _render_template, so each render's CWD is its own dst_path — the old-copy replay renders with CWD at the old-copy temp dir, the new-copy render at the new-copy temp dir, never the user-specified destination as such. The only reason a.txt showed up in this test is that the current update algorithm's "normal update in final destination" step renders into the real destination, so for that one render the two notions coincide. Under #2376, no render would target the user-specified destination, so fileglob would never see user files — which I agree is the more consistent and predictable semantics (and composes better with restoring a cached old copy per #1170).

So the a.txt assertion was pinning an artifact of the current algorithm rather than intended behavior. I've pushed a commit that drops the user-added file and asserts ['version.txt'] instead, which asserts only the invariant both algorithms share: renders see the destination being rendered, never the invocation directory (the decoy file still guards the original bug from #1708). It should survive the switch to #2376 unchanged, and I left a note in the test explaining why user-added files are deliberately not asserted.

…ing update

Whether files added by the user in the destination are visible to
filesystem filters during an update depends on whether the update
algorithm renders into the user-specified destination, which is an
implementation detail that may change (copier-org#2376). Assert only the
invariant shared by both algorithms: rendering happens in the
destination being rendered, never the invocation directory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

Current working directory during copier {copy,recopy,update} should be destination path

2 participants