Skip to content

Commit 410cef5

Browse files
nikzartclaude
andcommitted
fix: use destination path as working directory while rendering
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>
1 parent 454ec42 commit 410cef5

5 files changed

Lines changed: 181 additions & 43 deletions

File tree

copier/_main.py

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import sys
1010
import warnings
1111
from collections.abc import Callable, Iterable, Mapping, Sequence
12-
from contextlib import suppress
12+
from contextlib import nullcontext, suppress
1313
from contextvars import ContextVar
1414
from dataclasses import field, replace
1515
from filecmp import dircmp
@@ -332,9 +332,13 @@ def _render(path: str) -> str:
332332
return self._render_string(path)
333333

334334
def _load_external_data(path: str) -> Any:
335+
# NOTE: Use the absolute destination path instead of `self.dst_path`,
336+
# which may be relative to the original working directory, because
337+
# data may be loaded lazily while rendering, when the working
338+
# directory is the destination path.
335339
if (
336340
not (
337-
(self.dst_path / (path := _render(path)))
341+
(self.subproject.local_abspath / (path := _render(path)))
338342
.resolve()
339343
.is_relative_to(self.subproject.local_abspath)
340344
)
@@ -348,7 +352,9 @@ def _load_external_data(path: str) -> Any:
348352
" - API: `trust=True`"
349353
),
350354
)
351-
return load_answersfile_data(self.dst_path, path, warn_on_missing=True)
355+
return load_answersfile_data(
356+
self.subproject.local_abspath, path, warn_on_missing=True
357+
)
352358

353359
# Given those values are lazily rendered on 1st access then cached
354360
# the phase value is irrelevant and could be misleading.
@@ -777,47 +783,58 @@ def _render_template(self) -> None:
777783
"""Render the template in the subproject root."""
778784
follow_symlinks = not self.template.preserve_symlinks
779785
dst_root = self.dst_path.resolve()
780-
for src in scantree(str(self.template_copy_root), follow_symlinks):
781-
src_abspath = Path(src.path)
782-
# If the source is a symlink, we are not preserving symlinks, and the
783-
# symlink target is outside the template root, this means that we are
784-
# copying a file/directory from outside the template, which is
785-
# forbidden, so raise an error.
786-
if (
787-
src_abspath.is_symlink()
788-
and not self.template.preserve_symlinks
789-
and not (src_abspath.resolve()).is_relative_to(
790-
self.template.local_abspath
791-
)
792-
):
793-
raise ForbiddenPathError(
794-
path=src_abspath.relative_to(self.template_copy_root)
786+
if not self.pretend:
787+
dst_root.mkdir(parents=True, exist_ok=True)
788+
# Render with the subproject root as the working directory, so that
789+
# filesystem-related Jinja filters and extensions (e.g. `fileglob`)
790+
# operate relative to the destination path. In pretend mode, a missing
791+
# destination folder is not created, and the working directory is left
792+
# unchanged.
793+
with local.cwd(dst_root) if dst_root.is_dir() else nullcontext():
794+
for src in scantree(str(self.template_copy_root), follow_symlinks):
795+
src_abspath = Path(src.path)
796+
# If the source is a symlink, we are not preserving symlinks, and the
797+
# symlink target is outside the template root, this means that we are
798+
# copying a file/directory from outside the template, which is
799+
# forbidden, so raise an error.
800+
if (
801+
src_abspath.is_symlink()
802+
and not self.template.preserve_symlinks
803+
and not (src_abspath.resolve()).is_relative_to(
804+
self.template.local_abspath
805+
)
806+
):
807+
raise ForbiddenPathError(
808+
path=src_abspath.relative_to(self.template_copy_root)
809+
)
810+
src_relpath = Path(src_abspath).relative_to(self.template.local_abspath)
811+
dst_relpaths_ctxs = self._render_path(
812+
Path(src_abspath).relative_to(self.template_copy_root)
795813
)
796-
src_relpath = Path(src_abspath).relative_to(self.template.local_abspath)
797-
dst_relpaths_ctxs = self._render_path(
798-
Path(src_abspath).relative_to(self.template_copy_root)
799-
)
800-
for dst_relpath, ctx in dst_relpaths_ctxs:
801-
dst_abspath = dst_root / dst_relpath
802-
if dst_abspath.is_symlink() and self.template.preserve_symlinks:
803-
# If destination path is a symlink, it can safely point outside the
804-
# subproject dir, while still itself existing within the subproject.
805-
# (So long as nothing is templated into it (if it is a directory),
806-
# which would be caught by that path's own check.)
807-
# Therefore avoid resolving the symlink itself:
808-
dst_realpath = dst_abspath.parent.resolve() / dst_abspath.name
809-
else:
810-
dst_realpath = dst_abspath.resolve()
811-
if not dst_realpath.is_relative_to(dst_root):
812-
raise ForbiddenPathError(path=dst_relpath)
813-
if self.match_exclude(dst_relpath):
814-
continue
815-
if src.is_symlink() and self.template.preserve_symlinks:
816-
self._render_symlink(src_relpath, dst_relpath)
817-
elif src.is_dir(follow_symlinks=follow_symlinks):
818-
self._render_folder(dst_relpath)
819-
else:
820-
self._render_file(src_relpath, dst_relpath, extra_context=ctx or {})
814+
for dst_relpath, ctx in dst_relpaths_ctxs:
815+
dst_abspath = dst_root / dst_relpath
816+
if dst_abspath.is_symlink() and self.template.preserve_symlinks:
817+
# If destination path is a symlink, it can safely point
818+
# outside the subproject dir, while still itself existing
819+
# within the subproject. (So long as nothing is templated
820+
# into it (if it is a directory), which would be caught by
821+
# that path's own check.) Therefore avoid resolving the
822+
# symlink itself:
823+
dst_realpath = dst_abspath.parent.resolve() / dst_abspath.name
824+
else:
825+
dst_realpath = dst_abspath.resolve()
826+
if not dst_realpath.is_relative_to(dst_root):
827+
raise ForbiddenPathError(path=dst_relpath)
828+
if self.match_exclude(dst_relpath):
829+
continue
830+
if src.is_symlink() and self.template.preserve_symlinks:
831+
self._render_symlink(src_relpath, dst_relpath)
832+
elif src.is_dir(follow_symlinks=follow_symlinks):
833+
self._render_folder(dst_relpath)
834+
else:
835+
self._render_file(
836+
src_relpath, dst_relpath, extra_context=ctx or {}
837+
)
821838

822839
def _render_file( # noqa: C901
823840
self,

docs/configuring.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,15 @@ The following extensions are _always_ loaded:
12131213
You don't need to tell your template users to install these extensions: Copier depends
12141214
on them, so they are always installed when Copier is installed.
12151215

1216+
!!! note
1217+
1218+
While rendering the subproject, the current working directory is the destination
1219+
path, so filesystem-related filters, functions and tags provided by Jinja
1220+
extensions (e.g., the `fileglob` filter) operate relative to the destination path.
1221+
Keep in mind that Copier generates files in no particular order, so the result of
1222+
filesystem operations may depend on whether the files being queried were already
1223+
generated.
1224+
12161225
!!! warning
12171226

12181227
Including an extension allows Copier to execute uncontrolled code, thus making the

tests/test_answersfile.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import pytest
99
import yaml
10+
from plumbum import local
1011

1112
import copier
1213
from copier._user_data import load_answersfile_data
@@ -549,3 +550,30 @@ def test_external_data_path_outside_destination_root_is_unsafe_on_update(
549550

550551
with expected:
551552
copier.run_update(project, defaults=True, overwrite=True, unsafe=unsafe)
553+
554+
555+
def test_external_data_lazily_loaded_while_rendering(
556+
tmp_path_factory: pytest.TempPathFactory,
557+
) -> None:
558+
"""External data loads while rendering with a relative destination path.
559+
560+
While rendering, the working directory is the destination path
561+
(https://github.com/copier-org/copier/issues/1708), and external data may
562+
be loaded lazily on first access while rendering a file.
563+
"""
564+
src, invocation = map(tmp_path_factory.mktemp, ("src", "invocation"))
565+
build_file_tree(
566+
{
567+
(src / "copier.yml"): (
568+
"""\
569+
_external_data:
570+
other: other-data.yml
571+
"""
572+
),
573+
(src / "out.txt.jinja"): "{{ _external_data.other.key }}",
574+
(invocation / "dst" / "other-data.yml"): "key: value",
575+
}
576+
)
577+
with local.cwd(invocation):
578+
copier.run_copy(str(src), "dst", defaults=True, overwrite=True)
579+
assert (invocation / "dst" / "out.txt").read_text() == "value"

tests/test_copy.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,57 @@ def test_pretend_option(tmp_path: Path) -> None:
383383
assert not (tmp_path / "pyproject.toml").exists()
384384

385385

386+
def test_pretend_option_missing_dst(tmp_path_factory: pytest.TempPathFactory) -> None:
387+
src = tmp_path_factory.mktemp("src")
388+
build_file_tree({(src / "a.txt"): ""})
389+
dst = tmp_path_factory.mktemp("dst_parent") / "dst"
390+
copier.run_copy(str(src), dst, pretend=True)
391+
assert not dst.exists()
392+
393+
394+
def test_render_cwd_is_destination(tmp_path_factory: pytest.TempPathFactory) -> None:
395+
"""Filesystem-related Jinja filters operate relative to the destination.
396+
397+
https://github.com/copier-org/copier/issues/1708
398+
"""
399+
src, dst, invocation = map(tmp_path_factory.mktemp, ("src", "dst", "invocation"))
400+
build_file_tree(
401+
{
402+
(src / "fileglob.txt.jinja"): "{{ '*.txt' | fileglob | sort }}",
403+
(dst / "a.txt"): "",
404+
(dst / "b.txt"): "",
405+
(invocation / "decoy.txt"): "",
406+
}
407+
)
408+
with local.cwd(invocation):
409+
cwd_before = Path.cwd()
410+
copier.run_copy(str(src), dst, defaults=True, overwrite=True)
411+
assert Path.cwd() == cwd_before
412+
assert (dst / "fileglob.txt").read_text() == "['a.txt', 'b.txt']"
413+
414+
415+
def test_render_cwd_is_destination_relative_dst(
416+
tmp_path_factory: pytest.TempPathFactory,
417+
) -> None:
418+
"""A relative destination path works while rendering in the destination.
419+
420+
https://github.com/copier-org/copier/issues/1708
421+
"""
422+
src, invocation = map(tmp_path_factory.mktemp, ("src", "invocation"))
423+
build_file_tree(
424+
{
425+
(src / "fileglob.txt.jinja"): "{{ '*.txt' | fileglob | sort }}",
426+
(invocation / "dst" / "a.txt"): "",
427+
(invocation / "decoy.txt"): "",
428+
}
429+
)
430+
with local.cwd(invocation):
431+
cwd_before = Path.cwd()
432+
copier.run_copy(str(src), "dst", defaults=True, overwrite=True)
433+
assert Path.cwd() == cwd_before
434+
assert (invocation / "dst" / "fileglob.txt").read_text() == "['a.txt']"
435+
436+
386437
@pytest.mark.parametrize("generate", [True, False])
387438
def test_empty_dir(tmp_path_factory: pytest.TempPathFactory, generate: bool) -> None:
388439
src, dst = map(tmp_path_factory.mktemp, ("src", "dst"))

tests/test_updatediff.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,39 @@ def test_update_from_tagged_to_head(tmp_path_factory: pytest.TempPathFactory) ->
578578
assert load_answersfile_data(dst)["_commit"] == f"v1-1-g{sha}"
579579

580580

581+
def test_update_render_cwd_is_destination(
582+
tmp_path_factory: pytest.TempPathFactory,
583+
) -> None:
584+
"""While updating, the working directory during rendering is the destination.
585+
586+
https://github.com/copier-org/copier/issues/1708
587+
"""
588+
src, dst, invocation = map(tmp_path_factory.mktemp, ("src", "dst", "invocation"))
589+
with local.cwd(src):
590+
build_file_tree(
591+
{
592+
"{{ _copier_conf.answers_file }}.jinja": "{{ _copier_answers|to_nice_yaml }}",
593+
"version.txt": "1",
594+
}
595+
)
596+
git_init("v1")
597+
git("tag", "v1")
598+
run_copy(str(src), dst, defaults=True, overwrite=True)
599+
build_file_tree({(dst / "a.txt"): "", (invocation / "decoy.txt"): ""})
600+
with local.cwd(dst):
601+
git_init("copied")
602+
with local.cwd(src):
603+
build_file_tree({"fileglob.txt.jinja": "{{ '*.txt' | fileglob | sort }}"})
604+
git("add", ".")
605+
git("commit", "-m2")
606+
git("tag", "v2")
607+
with local.cwd(invocation):
608+
cwd_before = Path.cwd()
609+
run_update(dst, defaults=True, overwrite=True)
610+
assert Path.cwd() == cwd_before
611+
assert (dst / "fileglob.txt").read_text() == "['a.txt', 'version.txt']"
612+
613+
581614
@pytest.mark.parametrize("subproject_path", [Path(), Path("subproject")])
582615
@pytest.mark.parametrize("skip_pattern", ["skip_me", "/skip_me"])
583616
def test_skip_update(

0 commit comments

Comments
 (0)