fix: clean up tmp dir

Until now, Copier always left a lot of garbage in the tmp dir.

When running in batch, this could become a big problem, as templates would fill up memory (`/tmp/` is usually mounted as tmpfs).

@moduon MT-3411
Co-authored-by: Sigurd Spieckermann <[email protected]>

Author: yajo

copier/cli.py

@@ -234,13 +234,14 @@ def main(self, template_src: str, destination_path: str) -> int:
             destination_path:
                 Where to generate the new subproject. It must not exist or be empty.
         """
-        self._worker(
+        with self._worker(
             template_src,
             destination_path,
             cleanup_on_error=self.cleanup_on_error,
             defaults=self.force or self.defaults,
             overwrite=self.force or self.overwrite,
-        ).run_copy()
+        ) as worker:
+            worker.run_copy()
         return 0
 
 
@@ -295,11 +296,12 @@ def main(self, destination_path: cli.ExistingDirectory = ".") -> int:
                 The subproject must exist. If not specified, the currently
                 working directory is used.
         """
-        self._worker(
+        with self._worker(
             dst_path=destination_path,
             defaults=self.force or self.defaults,
             overwrite=self.force or self.overwrite,
-        ).run_recopy()
+        ) as worker:
+            worker.run_recopy()
         return 0
 
 
@@ -360,13 +362,14 @@ def main(self, destination_path: cli.ExistingDirectory = ".") -> int:
                 The subproject must exist. If not specified, the currently
                 working directory is used.
         """
-        self._worker(
+        with self._worker(
             dst_path=destination_path,
             conflict=self.conflict,
             context_lines=self.context_lines,
             defaults=self.defaults,
             overwrite=True,
-        ).run_update()
+        ) as worker:
+            worker.run_update()
         return 0
 
 

copier/main.py

@@ -15,6 +15,7 @@
 from typing import (
     Callable,
     Iterable,
+    List,
     Literal,
     Mapping,
     Optional,
@@ -187,6 +188,7 @@ class Worker:
     unsafe: bool = False
 
     answers: AnswersMap = field(default_factory=AnswersMap, init=False)
+    _cleanup_hooks: List[Callable] = field(default_factory=list, init=False)
 
     def __enter__(self):
         """Allow using worker as a context manager."""
@@ -204,7 +206,9 @@ def __exit__(self, type, value, traceback):
         self._cleanup()
 
     def _cleanup(self):
-        self.template._cleanup()
+        """Execute all stored cleanup methods."""
+        for method in self._cleanup_hooks:
+            method()
 
     def _check_unsafe(self, mode: Literal["copy", "update"]) -> None:
         """Check whether a template uses unsafe features."""
@@ -283,6 +287,7 @@ def _render_context(self) -> Mapping:
         # Backwards compatibility
         # FIXME Remove it?
         conf = asdict(self)
+        conf.pop("_cleanup_hooks")
         conf.update(
             {
                 "answers_file": self.answers_relpath,
@@ -682,10 +687,12 @@ def _render_string(self, string: str) -> str:
     @cached_property
     def subproject(self) -> Subproject:
         """Get related subproject."""
-        return Subproject(
+        result = Subproject(
             local_abspath=self.dst_path.absolute(),
             answers_relpath=self.answers_file or Path(".copier-answers.yml"),
         )
+        self._cleanup_hooks.append(result._cleanup)
+        return result
 
     @cached_property
     def template(self) -> Template:
@@ -695,7 +702,11 @@ def template(self) -> Template:
             if self.subproject.template is None:
                 raise TypeError("Template not found")
             url = str(self.subproject.template.url)
-        return Template(url=url, ref=self.vcs_ref, use_prereleases=self.use_prereleases)
+        result = Template(
+            url=url, ref=self.vcs_ref, use_prereleases=self.use_prereleases
+        )
+        self._cleanup_hooks.append(result._cleanup)
+        return result
 
     @cached_property
     def template_copy_root(self) -> Path:
@@ -750,8 +761,8 @@ def run_recopy(self) -> None:
                 "Cannot recopy because cannot obtain old template references "
                 f"from `{self.subproject.answers_relpath}`."
             )
-        new_worker = replace(self, src_path=self.subproject.template.url)
-        return new_worker.run_copy()
+        with replace(self, src_path=self.subproject.template.url) as new_worker:
+            return new_worker.run_copy()
 
     def run_update(self) -> None:
         """Update a subproject that was already generated.
@@ -816,16 +827,16 @@ def _apply_update(self):
             prefix=f"{__name__}.old_copy."
         ) as old_copy, TemporaryDirectory(prefix=f"{__name__}.new_copy.") as new_copy:
             # Copy old template into a temporary destination
-            old_worker = replace(
+            with replace(
                 self,
                 dst_path=old_copy / subproject_subdir,
                 data=self.subproject.last_answers,
                 defaults=True,
                 quiet=True,
                 src_path=self.subproject.template.url,
                 vcs_ref=self.subproject.template.commit,
-            )
-            old_worker.run_copy()
+            ) as old_worker:
+                old_worker.run_copy()
             # Extract diff between temporary destination and real destination
             with local.cwd(old_copy):
                 self._git_initialize_repo()
@@ -853,26 +864,26 @@ def _apply_update(self):
             with suppress(AttributeError):
                 del self.subproject.last_answers
             # Do a normal update in final destination
-            current_worker = replace(
+            with replace(
                 self,
                 # Files can change due to the historical diff, and those
                 # changes are not detected in this process, so it's better to
                 # say nothing than lie.
                 # TODO
                 quiet=True,
-            )
-            current_worker.run_copy()
-            self.answers = current_worker.answers
+            ) as current_worker:
+                current_worker.run_copy()
+                self.answers = current_worker.answers
             # Render with the same answers in an empty dir to avoid pollution
-            new_worker = replace(
+            with replace(
                 self,
                 dst_path=new_copy / subproject_subdir,
                 data=self.answers.combined,
                 defaults=True,
                 quiet=True,
                 src_path=self.subproject.template.url,
-            )
-            new_worker.run_copy()
+            ) as new_worker:
+                new_worker.run_copy()
             compared = dircmp(old_copy, new_copy)
             # Try to apply cached diff into final destination
             with local.cwd(subproject_top):

copier/subproject.py

@@ -3,9 +3,10 @@
 A *subproject* is a project that gets rendered and/or updated with Copier.
 """
 
+from dataclasses import field
 from functools import cached_property
 from pathlib import Path
-from typing import Optional
+from typing import Callable, List, Optional
 
 import yaml
 from plumbum.cmd import git
@@ -32,6 +33,8 @@ class Subproject:
     local_abspath: AbsolutePath
     answers_relpath: Path = Path(".copier-answers.yml")
 
+    _cleanup_hooks: List[Callable] = field(default_factory=list, init=False)
+
     def is_dirty(self) -> bool:
         """Indicate if the local template root is dirty.
 
@@ -42,6 +45,11 @@ def is_dirty(self) -> bool:
                 return bool(git("status", "--porcelain").strip())
         return False
 
+    def _cleanup(self):
+        """Remove temporary files and folders created by the subproject."""
+        for method in self._cleanup_hooks:
+            method()
+
     @property
     def _raw_answers(self) -> AnyByStrDict:
         """Get last answers, loaded raw as yaml."""
@@ -67,7 +75,9 @@ def template(self) -> Optional[Template]:
         last_url = self.last_answers.get("_src_path")
         last_ref = self.last_answers.get("_commit")
         if last_url:
-            return Template(url=last_url, ref=last_ref)
+            result = Template(url=last_url, ref=last_ref)
+            self._cleanup_hooks.append(result._cleanup)
+            return result
 
     @cached_property
     def vcs(self) -> Optional[VCSTypes]:

tests/test_tmpdir.py

@@ -0,0 +1,110 @@
+from pathlib import Path
+
+import pytest
+from plumbum.cmd import git
+
+from copier.cli import CopierApp
+from copier.main import run_copy, run_recopy, run_update
+
+from .helpers import build_file_tree
+
+
+@pytest.fixture(scope="module")
+def template_path(tmp_path_factory: pytest.TempPathFactory) -> str:
+    # V1 of the template
+    root = tmp_path_factory.mktemp("template")
+    build_file_tree(
+        {
+            root / "copier.yml": "favorite_app: Copier",
+            root / "fav.txt.jinja": "{{ favorite_app }}",
+            root
+            / "{{ _copier_conf.answers_file }}.jinja": "{{ _copier_answers|to_nice_yaml }}",
+        }
+    )
+    _git = git["-C", root]
+    _git("init")
+    _git("add", "-A")
+    _git("commit", "-m", "Initial commit")
+    _git("tag", "v1")
+    # V2 of the template
+    build_file_tree({root / "v2": "true"})
+    _git("add", "-A")
+    _git("commit", "-m", "Second commit")
+    _git("tag", "v2")
+    return str(root)
+
+
+def empty_dir(dir: Path):
+    assert dir.is_dir()
+    assert dir.exists()
+    assert len(list(dir.iterdir())) == 0
+
+
+def test_api(
+    template_path: str,
+    tmp_path_factory: pytest.TempPathFactory,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    tmp, dst = map(tmp_path_factory.mktemp, ["tmp", "dst"])
+    _git = git["-C", dst]
+    # Mock tmp dir to assert it ends up clean
+    monkeypatch.setattr("tempfile.tempdir", str(tmp))
+    # Copy
+    run_copy(template_path, dst, vcs_ref="v1", quiet=True, defaults=True)
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert not (dst / "v2").exists()
+    _git("init")
+    _git("add", "-A")
+    _git("commit", "-m", "Initial commit")
+    empty_dir(tmp)
+    # Recopy
+    run_recopy(dst, vcs_ref="v1", quiet=True, defaults=True)
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert not (dst / "v2").exists()
+    empty_dir(tmp)
+    # Update
+    run_update(dst, quiet=True, defaults=True, overwrite=True)
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert (dst / "v2").read_text() == "true"
+    empty_dir(tmp)
+
+
+def test_cli(
+    template_path: str,
+    tmp_path_factory: pytest.TempPathFactory,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    tmp, dst = map(tmp_path_factory.mktemp, ["tmp", "dst"])
+    _git = git["-C", dst]
+    # Mock tmp dir to assert it ends up clean
+    monkeypatch.setattr("tempfile.tempdir", str(tmp))
+    # Copy
+    run_result = CopierApp.run(
+        [
+            "copier",
+            "copy",
+            "-fqrv1",
+            template_path,
+            str(dst),
+        ],
+        exit=False,
+    )
+    assert run_result[1] == 0
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert not (dst / "v2").exists()
+    empty_dir(tmp)
+    _git("init")
+    _git("add", "-A")
+    _git("commit", "-m", "Initial commit")
+    # Recopy
+    run_result = CopierApp.run(["copier", "recopy", "-fqrv1", str(dst)], exit=False)
+    assert run_result[1] == 0
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert not (dst / "v2").exists()
+    empty_dir(tmp)
+    # Update
+    run_result = CopierApp.run(["copier", "update", "-fq", str(dst)], exit=False)
+    assert run_result[1] == 0
+    assert (dst / "fav.txt").read_text() == "Copier"
+    assert (dst / "v2").read_text() == "true"
+    empty_dir(tmp)