-
-
Notifications
You must be signed in to change notification settings - Fork 270
feat(update): preserve developer-owned regions across updates (#2184) #2762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
490da4a
405786b
9170be0
04da30f
f55c390
677ce4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -45,7 +45,7 @@ | |||||
| from questionary import confirm, unsafe_prompt | ||||||
|
|
||||||
| from ._deprecation import deprecate_answers_file_template_path | ||||||
| from ._jinja_ext import YieldExtension, get_yield_context | ||||||
| from ._jinja_ext import IgnoreExtension, YieldExtension, get_yield_context | ||||||
| from ._settings import Settings, SettingsModel, is_trusted_repository | ||||||
| from ._subproject import Subproject | ||||||
| from ._template import Task, Template | ||||||
|
|
@@ -461,14 +461,20 @@ def _render_context(self) -> AnyByStrMutableMapping: | |||||
| "os": lambda: OS, | ||||||
| } | ||||||
| ) | ||||||
| return dict( | ||||||
| context = dict( | ||||||
| **self.answers.combined, | ||||||
| _copier_answers=self._answers_to_remember(), | ||||||
| _copier_conf=conf, | ||||||
| _folder_name=self.subproject.local_abspath.name, | ||||||
| _copier_python=sys.executable, | ||||||
| _copier_phase=Phase.current(), | ||||||
| ) | ||||||
| # ``_copier_operation`` is only defined within a copy/update/recopy run. | ||||||
| # Outside of one (e.g. ``check_update`` or other bare-``Worker`` use), | ||||||
| # leave it unset so rendering doesn't require an active operation. | ||||||
| if (operation := _operation.get(None)) is not None: | ||||||
| context["_copier_operation"] = operation | ||||||
| return context | ||||||
|
|
||||||
| def _path_matcher(self, patterns: Iterable[str]) -> Callable[[Path], bool]: | ||||||
| """Produce a function that matches against specified patterns.""" | ||||||
|
|
@@ -707,6 +713,7 @@ def jinja_env(self) -> SandboxedEnvironment: | |||||
| default_extensions = [ | ||||||
| "jinja2_ansible_filters.AnsibleCoreFiltersExtension", | ||||||
| YieldExtension, | ||||||
| IgnoreExtension, | ||||||
| ] | ||||||
| extensions = default_extensions + list(self.template.jinja_extensions) | ||||||
| envops = dict(self.template.envops) | ||||||
|
|
@@ -852,7 +859,7 @@ def _render_file( # noqa: C901 | |||||
| new_content = src_abspath.read_bytes() | ||||||
| else: | ||||||
| new_content = tpl.render( | ||||||
| **self._render_context(), **(extra_context or {}) | ||||||
| {**self._render_context(), **(extra_context or {})} | ||||||
| ).encode() | ||||||
| if get_yield_context(self.jinja_env).yield_name: | ||||||
| raise YieldTagInFileError( | ||||||
|
|
@@ -1175,7 +1182,7 @@ def _render_string( | |||||
| Additional variables to use for rendering the template. | ||||||
| """ | ||||||
| tpl = self.jinja_env.from_string(string) | ||||||
| return tpl.render(**self._render_context(), **(extra_context or {})) | ||||||
| return tpl.render({**self._render_context(), **(extra_context or {})}) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is diff noise, not strictly needed to implement the new feature.
Suggested change
|
||||||
|
|
||||||
| def _render_value( | ||||||
| self, value: _T, extra_context: AnyByStrDict | None = None | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -255,6 +255,85 @@ git clean -d -i # remove untracked files and folders | |||||||||||||||||||
| If you want fine-grained control to restore files selectively, read the output of the | ||||||||||||||||||||
| `git status` command attentively. It shows all the commands you may need as hints! | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ## Developer-owned regions with `{% ignore %}` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Templates often ship files with dummy content or `TODO` markers that you are meant to | ||||||||||||||||||||
| replace right after generating a project (e.g. a stub function body, a placeholder | ||||||||||||||||||||
| config value). By default, if the template later changes that placeholder, the | ||||||||||||||||||||
| [update](#how-the-update-works) 3-way merge treats it like any other template-owned | ||||||||||||||||||||
| content, so the template's new placeholder can overwrite your implementation or raise a | ||||||||||||||||||||
| needless merge conflict. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Wrap such a region in the `{% ignore %}` / `{% endignore %}` tag to make it | ||||||||||||||||||||
| **developer-owned**. The block is rendered on `copier copy` (so you get the initial | ||||||||||||||||||||
| scaffolding), but it is _omitted_ from the renders Copier produces internally during | ||||||||||||||||||||
| `copier update`. Because the region is absent from both sides of the update's 3-way | ||||||||||||||||||||
| merge, template changes inside it never reach the diff, and your version of the region | ||||||||||||||||||||
| in the generated project is left untouched. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ```jinja title="app.py.jinja" | ||||||||||||||||||||
| HEADER = "generated by {{ project_name }}" | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| {% ignore -%} | ||||||||||||||||||||
| def greeting() -> str: | ||||||||||||||||||||
| # TODO: implement your greeting | ||||||||||||||||||||
| return "dummy greeting" | ||||||||||||||||||||
| {%- endignore %} | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| FOOTER = "generated by {{ project_name }}" | ||||||||||||||||||||
| ``` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| This renders on the initial `copier copy` as: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ```python title="app.py" | ||||||||||||||||||||
| HEADER = "generated by Foo" | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def greeting() -> str: | ||||||||||||||||||||
| # TODO: implement your greeting | ||||||||||||||||||||
| return "dummy greeting" | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| FOOTER = "generated by Foo" | ||||||||||||||||||||
| ``` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| You then replace the body with your real implementation. On the next `copier update`, | ||||||||||||||||||||
| your `greeting()` is kept even if the template changed the placeholder, while `HEADER` | ||||||||||||||||||||
| and `FOOTER` still receive template updates. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| !!! tip "Nothing leaks into the rendered file" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Unlike a comment-based marker, the `{% ignore %}` tag is pure Jinja — it is stripped | ||||||||||||||||||||
| during rendering, so no Copier-specific syntax ends up in your generated project. It | ||||||||||||||||||||
| is therefore **language-agnostic**: it works in `.js`, `.go`, `.c`, `.rs` … or any | ||||||||||||||||||||
| other text file, since it never has to be a valid comment in the target language. | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Comment on lines
+306
to
+312
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is pretty obvious, let's try keeping the docs concise. I presume AI was in a chatty mood. 😄
Suggested change
|
||||||||||||||||||||
| !!! important "Leave stable context around the block" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| The `-%}` and `{%-` [whitespace-control][whitespace] markers strip the tag's own | ||||||||||||||||||||
| lines so the output stays clean. Keep at least one unchanging line — such as the | ||||||||||||||||||||
| blank lines above — directly around the block, and avoid editing the lines | ||||||||||||||||||||
| immediately adjacent to it in later template versions. The update re-applies your | ||||||||||||||||||||
| region as a diff against the surrounding lines; if the template changes a line right | ||||||||||||||||||||
| next to the block, the merge falls back to a normal Copier | ||||||||||||||||||||
| [conflict](#recover-from-a-broken-update) that you resolve by hand (your content is | ||||||||||||||||||||
| never silently lost). Embedding the block among stable code, rather than at the very | ||||||||||||||||||||
| top or bottom of a tiny file, gives the merge the anchors it needs. | ||||||||||||||||||||
|
Comment on lines
+313
to
+323
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we should phrase this as a recommendation because users may not be able to avoid touching adjacent lines that are part of the hunk context. It might be better to inform users about the behavior, so they aren't surprised when merge conflicts involving the ignored block occur. |
||||||||||||||||||||
|
|
||||||||||||||||||||
| The tag relies on the `_copier_operation` render context variable, which is `"copy"` | ||||||||||||||||||||
| during generation and `"update"` during an update. If you prefer to be explicit, or need | ||||||||||||||||||||
| a condition the tag can't express, you can write the equivalent directly: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ```jinja | ||||||||||||||||||||
| {% if _copier_operation != "update" %} | ||||||||||||||||||||
| ...developer-owned content... | ||||||||||||||||||||
| {% endif %} | ||||||||||||||||||||
| ``` | ||||||||||||||||||||
|
Comment on lines
+325
to
+333
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we should include this snippet, I don't quite see when anybody would want to use the explicit condition, so including it might suggest this is a reasonably common scenario.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| [whitespace]: https://jinja.palletsprojects.com/en/stable/templates/#whitespace-control | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ## Checking for updates | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Copier provides a subcommand `copier check-update` that can be used to check if there | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,178 @@ | ||||||||||||||||
| """Tests for the ``{% ignore %}`` tag that keeps developer-owned regions (#2184). | ||||||||||||||||
|
|
||||||||||||||||
| The tag renders its body on ``copier copy`` but is omitted from the renders | ||||||||||||||||
| Copier produces internally during ``copier update``. Because the region is | ||||||||||||||||
| absent from both the old and new template renders that feed the 3-way merge, | ||||||||||||||||
| template changes inside it never reach the diff and the developer's own version | ||||||||||||||||
| of the region survives, with no Copier syntax leaking into the rendered file. | ||||||||||||||||
| """ | ||||||||||||||||
|
|
||||||||||||||||
| from __future__ import annotations | ||||||||||||||||
|
|
||||||||||||||||
| from pathlib import Path | ||||||||||||||||
|
|
||||||||||||||||
| import pytest | ||||||||||||||||
| from jinja2.sandbox import SandboxedEnvironment | ||||||||||||||||
| from plumbum import local | ||||||||||||||||
|
|
||||||||||||||||
| from copier._jinja_ext import IgnoreExtension | ||||||||||||||||
| from copier._main import run_copy, run_update | ||||||||||||||||
|
|
||||||||||||||||
| from .helpers import build_file_tree, git | ||||||||||||||||
|
|
||||||||||||||||
| # Unit tests for the extension in isolation. | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @pytest.fixture | ||||||||||||||||
| def env() -> SandboxedEnvironment: | ||||||||||||||||
| return SandboxedEnvironment(extensions=[IgnoreExtension]) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_ignore_renders_body_outside_update(env: SandboxedEnvironment) -> None: | ||||||||||||||||
| template = env.from_string("a\n{% ignore %}b{% endignore %}\nc") | ||||||||||||||||
| assert template.render({"_copier_operation": "copy"}) == "a\nb\nc" | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_ignore_omits_body_on_update(env: SandboxedEnvironment) -> None: | ||||||||||||||||
| template = env.from_string("a\n{% ignore %}b{% endignore %}\nc") | ||||||||||||||||
| assert template.render({"_copier_operation": "update"}) == "a\n\nc" | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_ignore_trim_markers_clean_output(env: SandboxedEnvironment) -> None: | ||||||||||||||||
| """The idiomatic ``-%}``/``{%-`` markers strip the tag lines entirely.""" | ||||||||||||||||
| template = env.from_string("a\n{% ignore -%}\nb\n{%- endignore %}\nc") | ||||||||||||||||
| assert template.render({"_copier_operation": "copy"}) == "a\nb\nc" | ||||||||||||||||
| assert template.render({"_copier_operation": "update"}) == "a\n\nc" | ||||||||||||||||
|
Comment on lines
+23
to
+45
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we need these tests. In the end, what really matters is the behavior involving |
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # End-to-end tests exercising the full copy + update flow. | ||||||||||||||||
| # These also confirm the extension is registered by default (they use the tag | ||||||||||||||||
| # with no ``_jinja_extensions`` configured). | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def _commit_template(src: Path, body: str, tag: str) -> None: | ||||||||||||||||
| build_file_tree( | ||||||||||||||||
| { | ||||||||||||||||
| src / "{{ _copier_conf.answers_file }}.jinja": ( | ||||||||||||||||
| "{{ _copier_answers|to_nice_yaml }}\n" | ||||||||||||||||
| ), | ||||||||||||||||
| src / "app.py.jinja": body, | ||||||||||||||||
| }, | ||||||||||||||||
| dedent=False, | ||||||||||||||||
| ) | ||||||||||||||||
| with local.cwd(src): | ||||||||||||||||
| git("init") if not (src / ".git").exists() else None | ||||||||||||||||
| git("add", "-A") | ||||||||||||||||
| git("commit", "-m", tag) | ||||||||||||||||
| git("tag", tag) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| _V1 = """\ | ||||||||||||||||
| import os | ||||||||||||||||
| CONFIG = "v1" | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def stable_helper(): | ||||||||||||||||
| return 42 | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| {% ignore -%} | ||||||||||||||||
| def user_code(): | ||||||||||||||||
| return "dummy v1" | ||||||||||||||||
| {%- endignore %} | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def another_stable(): | ||||||||||||||||
| return "keep" | ||||||||||||||||
| """ | ||||||||||||||||
|
|
||||||||||||||||
| _V2 = ( | ||||||||||||||||
| _V1.replace('CONFIG = "v1"', 'CONFIG = "v2"') | ||||||||||||||||
| .replace('return "dummy v1"', 'return "dummy v2"') | ||||||||||||||||
| .replace('return "keep"', 'return "kept in v2"') | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+53
to
+93
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be more idiomatic to create a fixture that creates the test template with two commits/tags rather than having a helper function and constants. For brevity and convenience, you can use the |
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_copy_renders_block_without_leaking_syntax( | ||||||||||||||||
| tmp_path_factory: pytest.TempPathFactory, | ||||||||||||||||
| ) -> None: | ||||||||||||||||
| """Initial generation renders the block; no marker/tag survives.""" | ||||||||||||||||
| src, dst = map(tmp_path_factory.mktemp, ("src", "dst")) | ||||||||||||||||
| _commit_template(src, _V1, "v1") | ||||||||||||||||
|
|
||||||||||||||||
| run_copy(str(src), dst, defaults=True, overwrite=True, vcs_ref="v1") | ||||||||||||||||
|
|
||||||||||||||||
| rendered = (dst / "app.py").read_text(encoding="utf-8") | ||||||||||||||||
| assert 'return "dummy v1"' in rendered # scaffolding is present after copy | ||||||||||||||||
| assert "ignore" not in rendered # no Copier syntax leaks into the file | ||||||||||||||||
| assert "{%" not in rendered and "%}" not in rendered | ||||||||||||||||
|
Comment on lines
+96
to
+108
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd omit this test because the (updated version of the) tests below also covers this implicitly. |
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_update_preserves_developer_region( | ||||||||||||||||
| tmp_path_factory: pytest.TempPathFactory, | ||||||||||||||||
| ) -> None: | ||||||||||||||||
| """Developer content in an ``{% ignore %}`` block survives an update. | ||||||||||||||||
|
|
||||||||||||||||
| The template changes the surrounding code *and* the placeholder inside the | ||||||||||||||||
| block; only the surroundings update while the developer's implementation is | ||||||||||||||||
| kept, with no merge conflict. | ||||||||||||||||
| """ | ||||||||||||||||
| src, dst = map(tmp_path_factory.mktemp, ("src", "dst")) | ||||||||||||||||
| _commit_template(src, _V1, "v1") | ||||||||||||||||
|
|
||||||||||||||||
| run_copy(str(src), dst, defaults=True, overwrite=True, vcs_ref="v1") | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we can add an assertion that checks the content of |
||||||||||||||||
| with local.cwd(dst): | ||||||||||||||||
| git("init") | ||||||||||||||||
| git("add", "-A") | ||||||||||||||||
| git("commit", "-m", "generated") | ||||||||||||||||
|
|
||||||||||||||||
| # Developer replaces the placeholder body with a real implementation. | ||||||||||||||||
| app = dst / "app.py" | ||||||||||||||||
| app.write_text( | ||||||||||||||||
| app.read_text(encoding="utf-8").replace( | ||||||||||||||||
| 'return "dummy v1"', 'return "REAL IMPLEMENTATION"' | ||||||||||||||||
| ), | ||||||||||||||||
| encoding="utf-8", | ||||||||||||||||
| ) | ||||||||||||||||
| with local.cwd(dst): | ||||||||||||||||
| git("commit", "-am", "customize") | ||||||||||||||||
|
|
||||||||||||||||
| _commit_template(src, _V2, "v2") | ||||||||||||||||
| run_update(dst, defaults=True, overwrite=True, vcs_ref="v2") | ||||||||||||||||
|
|
||||||||||||||||
| result = app.read_text(encoding="utf-8") | ||||||||||||||||
| assert "<<<<<<<" not in result # clean merge | ||||||||||||||||
| assert 'return "REAL IMPLEMENTATION"' in result # developer content kept | ||||||||||||||||
| assert 'return "dummy v2"' not in result # template placeholder ignored | ||||||||||||||||
| assert 'CONFIG = "v2"' in result # surrounding code updated | ||||||||||||||||
| assert 'return "kept in v2"' in result | ||||||||||||||||
| assert "ignore" not in result # still no leaked syntax | ||||||||||||||||
|
Comment on lines
+144
to
+149
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be simpler and more explicit to assert the full file content. You can use
Suggested change
For this, add Same applies to the other test below. |
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| def test_update_without_customization_takes_template_default( | ||||||||||||||||
| tmp_path_factory: pytest.TempPathFactory, | ||||||||||||||||
| ) -> None: | ||||||||||||||||
| """If the developer never touched the block, they keep the v1 scaffolding. | ||||||||||||||||
|
|
||||||||||||||||
| Because the block is omitted from the update renders, the template's newer | ||||||||||||||||
| placeholder does not overwrite the region -- the region is developer-owned | ||||||||||||||||
| from the very first ``copy``. | ||||||||||||||||
| """ | ||||||||||||||||
| src, dst = map(tmp_path_factory.mktemp, ("src", "dst")) | ||||||||||||||||
| _commit_template(src, _V1, "v1") | ||||||||||||||||
|
|
||||||||||||||||
| run_copy(str(src), dst, defaults=True, overwrite=True, vcs_ref="v1") | ||||||||||||||||
| with local.cwd(dst): | ||||||||||||||||
| git("init") | ||||||||||||||||
| git("add", "-A") | ||||||||||||||||
| git("commit", "-m", "generated") | ||||||||||||||||
|
|
||||||||||||||||
| _commit_template(src, _V2, "v2") | ||||||||||||||||
| run_update(dst, defaults=True, overwrite=True, vcs_ref="v2") | ||||||||||||||||
|
|
||||||||||||||||
| result = (dst / "app.py").read_text(encoding="utf-8") | ||||||||||||||||
| assert "<<<<<<<" not in result | ||||||||||||||||
| # The region keeps the originally generated content, not the v2 placeholder. | ||||||||||||||||
| assert 'return "dummy v1"' in result | ||||||||||||||||
| assert 'return "dummy v2"' not in result | ||||||||||||||||
| assert 'CONFIG = "v2"' in result # surroundings still update | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is diff noise, not strictly needed to implement the new feature.