Skip to content

Commit 0780374

Browse files
committed
fix(updating): never recreate deleted paths on update
Previously, paths that matched a pattern in `skip_if_exists` and that were deleted in the generated project were recreated during each update. These deletions are contained in the diff between old vanilla generated project and the (pre-update) current one. They are not executed though since the paths are excluded from the diff application. Similarly, if a file that was deleted intentionally in a generated project was changed in the template, an update would have regenerated it (but not if it did not have any changes). I'm not completely sure how this manifested, but it probably involved a suppressed merge conflict. This commit ensures that update behavior is always consistent - never regenerate deleted paths during `update` - by acquiring a list of deleted paths and excluding them in the copy operations that are relevant for acquiring the update diff.
1 parent 6fd3ad4 commit 0780374

File tree

2 files changed

+87
-1
lines changed

2 files changed

+87
-1
lines changed

Diff for: copier/main.py

+13-1
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,16 @@ def _apply_update(self) -> None: # noqa: C901
933933
self._execute_tasks(
934934
self.template.migration_tasks("before", self.subproject.template) # type: ignore[arg-type]
935935
)
936+
with local.cwd(old_copy):
937+
self._git_initialize_repo()
938+
git("remote", "add", "real_dst", "file://" + str(subproject_top))
939+
git("fetch", "--depth=1", "real_dst", "HEAD")
940+
# Save a list of files that were intentionally removed in the generated
941+
# project to avoid recreating them during the update.
942+
diff_removed = git(
943+
"diff-tree", "-r", "--diff-filter=D", "--name-only", "HEAD...FETCH_HEAD",
944+
).splitlines()
945+
temp_exclude = list(set(self.exclude).union(diff_removed))
936946
# Create a copy of the real destination after applying migrations
937947
# but before performing any further update for extracting the diff
938948
# between the temporary destination of the old template and the
@@ -954,6 +964,8 @@ def _apply_update(self) -> None: # noqa: C901
954964
# Do a normal update in final destination
955965
with replace(
956966
self,
967+
# Don't regenerate intentionally deleted paths
968+
exclude=temp_exclude,
957969
# Files can change due to the historical diff, and those
958970
# changes are not detected in this process, so it's better to
959971
# say nothing than lie.
@@ -970,6 +982,7 @@ def _apply_update(self) -> None: # noqa: C901
970982
defaults=True,
971983
quiet=True,
972984
src_path=self.subproject.template.url, # type: ignore[union-attr]
985+
exclude=temp_exclude,
973986
) as new_worker:
974987
new_worker.run_copy()
975988
with local.cwd(new_copy):
@@ -978,7 +991,6 @@ def _apply_update(self) -> None: # noqa: C901
978991
# real destination with some special handling of newly added files
979992
# in both the poject and the template.
980993
with local.cwd(old_copy):
981-
self._git_initialize_repo()
982994
git("remote", "add", "dst_copy", "file://" + str(dst_copy))
983995
git("fetch", "--depth=1", "dst_copy", "HEAD:dst_copy")
984996
git("remote", "add", "new_copy", "file://" + str(new_copy))

Diff for: tests/test_updatediff.py

+74
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,80 @@ def test_skip_update(tmp_path_factory: pytest.TempPathFactory) -> None:
554554
assert not (dst / "skip_me.rej").exists()
555555

556556

557+
def test_skip_update_deleted(tmp_path_factory: pytest.TempPathFactory) -> None:
558+
"""
559+
Ensure that ``skip_if_exists`` does not interfere with
560+
deleted paths during updates (i.e. they stay deleted).
561+
Issue #1718
562+
"""
563+
src, dst = map(tmp_path_factory.mktemp, ("src", "dst"))
564+
with local.cwd(src):
565+
build_file_tree(
566+
{
567+
"copier.yaml": "_skip_if_exists: [skip_me]",
568+
"{{ _copier_conf.answers_file }}.jinja": "{{ _copier_answers|to_yaml }}",
569+
"skip_me": "1",
570+
"another_file": "foobar",
571+
}
572+
)
573+
git("init")
574+
git("add", ".")
575+
git("commit", "-m1")
576+
git("tag", "1.0.0")
577+
run_copy(str(src), dst, defaults=True, overwrite=True)
578+
skip_me = dst / "skip_me"
579+
answers_file = dst / ".copier-answers.yml"
580+
answers = yaml.safe_load(answers_file.read_text())
581+
assert skip_me.read_text() == "1"
582+
assert answers["_commit"] == "1.0.0"
583+
skip_me.unlink()
584+
with local.cwd(dst):
585+
git("init")
586+
git("add", ".")
587+
git("commit", "-m1")
588+
run_update(dst, skip_answered=True, overwrite=True)
589+
assert not (dst / "skip_me").exists()
590+
591+
592+
def test_update_deleted_path(tmp_path_factory: pytest.TempPathFactory) -> None:
593+
"""
594+
Ensure that deleted paths are not regenerated during updates,
595+
even if the template has changes in that path.
596+
Issue #1721
597+
"""
598+
src, dst = map(tmp_path_factory.mktemp, ("src", "dst"))
599+
with local.cwd(src):
600+
build_file_tree(
601+
{
602+
"copier.yaml": "_skip_if_exists: [skip_me]",
603+
"{{ _copier_conf.answers_file }}.jinja": "{{ _copier_answers|to_yaml }}",
604+
"updated_file": "foo",
605+
"another_file": "foobar",
606+
}
607+
)
608+
git("init")
609+
git("add", ".")
610+
git("commit", "-m1")
611+
git("tag", "1.0.0")
612+
run_copy(str(src), dst, defaults=True, overwrite=True)
613+
updated_file = dst / "updated_file"
614+
answers_file = dst / ".copier-answers.yml"
615+
answers = yaml.safe_load(answers_file.read_text())
616+
assert updated_file.read_text() == "foo"
617+
assert answers["_commit"] == "1.0.0"
618+
updated_file.unlink()
619+
with local.cwd(dst):
620+
git("init")
621+
git("add", ".")
622+
git("commit", "-m1")
623+
with local.cwd(src):
624+
build_file_tree({"updated_file": "bar"})
625+
git("commit", "-am2")
626+
git("tag", "2.0.0")
627+
run_update(dst, skip_answered=True, overwrite=True)
628+
assert not (dst / "updated_file").exists()
629+
630+
557631
@pytest.mark.parametrize(
558632
"answers_file", [None, ".copier-answers.yml", ".custom.copier-answers.yaml"]
559633
)

0 commit comments

Comments
 (0)