Improve conflict resolution

CHANGELOG.md

@@ -52,6 +52,8 @@ Summary:
 -   Changed `--force` to be the same as `--defaults --overwrite`.
 -   Copied files will reflect permissions on the same files in the template.
 -   Copier now uses `git clone --filter=blob:none` when cloning, to be faster.
+-   Improved `copier update` conficts resolution by replacing the `.rej` files by merge
+    markers put in the destination file directly.
 
 ### Deprecated
 

copier/main.py

@@ -1,5 +1,6 @@
 """Main functions and classes, used to generate or update projects."""
 
+import os
 import platform
 import subprocess
 import sys
@@ -8,15 +9,14 @@
 from functools import partial
 from itertools import chain
 from pathlib import Path
-from shutil import rmtree
-from typing import Callable, Iterable, List, Mapping, Optional, Sequence
+from shutil import copyfile, rmtree
+from typing import Callable, Iterable, List, Mapping, Optional, Sequence, Set
 from unicodedata import normalize
 
 import pathspec
 from jinja2.loaders import FileSystemLoader
 from jinja2.sandbox import SandboxedEnvironment
-from plumbum import ProcessExecutionError, colors
-from plumbum.cli.terminal import ask
+from plumbum import colors
 from plumbum.cmd import git
 from plumbum.machines import local
 from pydantic.dataclasses import dataclass
@@ -40,9 +40,23 @@
 # HACK https://github.com/python/mypy/issues/8520#issuecomment-772081075
 if sys.version_info >= (3, 8):
     from functools import cached_property
+
 else:
     from backports.cached_property import cached_property
 
+# Backport of `shutil.copytree` for python 3.7 to accept `dirs_exist_ok` argument
+if sys.version_info >= (3, 8):
+    from shutil import copytree
+else:
+    from distutils.dir_util import copy_tree
+
+    def copytree(src: Path, dst: Path, dirs_exist_ok: bool = False):
+        """Backport of `shutil.copytree` with `dirs_exist_ok` argument.
+
+        Can be remove once python 3.7 dropped.
+        """
+        copy_tree(str(src), str(dst))
+
 
 @dataclass
 class Worker:
@@ -218,10 +232,7 @@ def _path_matcher(self, patterns: Iterable[str]) -> Callable[[Path], bool]:
         return spec.match_file
 
     def _solve_render_conflict(self, dst_relpath: Path):
-        """Properly solve render conflicts.
-
-        It can ask the user if running in interactive mode.
-        """
+        """Properly solve render conflicts."""
         assert not dst_relpath.is_absolute()
         printf(
             "conflict",
@@ -239,16 +250,8 @@ def _solve_render_conflict(self, dst_relpath: Path):
                 file_=sys.stderr,
             )
             return False
-        if self.overwrite or dst_relpath == self.answers_relpath:
-            printf(
-                "overwrite",
-                dst_relpath,
-                style=Style.WARNING,
-                quiet=self.quiet,
-                file_=sys.stderr,
-            )
-            return True
-        return bool(ask(f" Overwrite {dst_relpath}?", default=True))
+
+        return True
 
     def _render_allowed(
         self,
@@ -657,26 +660,31 @@ def run_update(self) -> None:
             print(
                 f"Updating to template version {self.template.version}", file=sys.stderr
             )
+
         # Copy old template into a temporary destination
-        with TemporaryDirectory(prefix=f"{__name__}.update_diff.") as dst_temp:
-            old_worker = replace(
+        with TemporaryDirectory(
+            prefix=f"{__name__}.update_diff.reference."
+        ) as reference_dst_temp, TemporaryDirectory(
+            prefix=f"{__name__}.update_diff.original."
+        ) as original_dst_temp, TemporaryDirectory(
+            prefix=f"{__name__}.update_diff.merge."
+        ) as merge_dst_temp:
+            # Copy reference to be used as base by merge-file
+            copytree(self.dst_path, reference_dst_temp, dirs_exist_ok=True)
+
+            # Compute modification from the original template to be used as other by merge-file
+            original_worker = replace(
                 self,
-                dst_path=dst_temp,
+                dst_path=original_dst_temp,
                 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()
-            # Extract diff between temporary destination and real destination
-            with local.cwd(dst_temp):
-                subproject_top = git(
-                    "-C",
-                    self.subproject.local_abspath.absolute(),
-                    "rev-parse",
-                    "--show-toplevel",
-                ).strip()
+            original_worker.run_copy()
+            # Apply pre-commit hooks if provided by .git
+            with local.cwd(original_dst_temp):
                 git("init", retcode=None)
                 git("add", ".")
                 git("config", "user.name", "Copier")
@@ -686,37 +694,57 @@ def run_update(self) -> None:
                 git("commit", "--allow-empty", "-am", "dumb commit 2")
                 git("config", "--unset", "user.name")
                 git("config", "--unset", "user.email")
-                git("remote", "add", "real_dst", "file://" + subproject_top)
-                git("fetch", "--depth=1", "real_dst", "HEAD")
-                diff_cmd = git["diff-tree", "--unified=1", "HEAD...FETCH_HEAD"]
-                try:
-                    diff = diff_cmd("--inter-hunk-context=-1")
-                except ProcessExecutionError:
-                    print(
-                        colors.warn
-                        | "Make sure Git >= 2.24 is installed to improve updates.",
-                        file=sys.stderr,
-                    )
-                    diff = diff_cmd("--inter-hunk-context=0")
-        # Run pre-migration tasks
-        self._execute_tasks(
-            self.template.migration_tasks("before", self.subproject.template)
-        )
-        # Clear last answers cache to load possible answers migration
-        with suppress(AttributeError):
-            del self.answers
-        with suppress(AttributeError):
-            del self.subproject.last_answers
-        # Do a normal update in final destination
-        self.run_copy()
-        # Try to apply cached diff into final destination
-        with local.cwd(self.subproject.local_abspath):
-            apply_cmd = git["apply", "--reject", "--exclude", self.answers_relpath]
-            for skip_pattern in chain(
-                self.skip_if_exists, self.template.skip_if_exists
-            ):
-                apply_cmd = apply_cmd["--exclude", skip_pattern]
-            (apply_cmd << diff)(retcode=None)
+
+            # Run pre-migration tasks
+            self._execute_tasks(
+                self.template.migration_tasks("before", self.subproject.template)
+            )
+            # Clear last answers cache to load possible answers migration
+            with suppress(AttributeError):
+                del self.answers
+            with suppress(AttributeError):
+                del self.subproject.last_answers
+            # Do a normal update in final destination
+            self.run_copy()
+
+            # Extract the list of files to merge
+            participating_files: Set[Path] = set()
+            for src_dir in (original_dst_temp, reference_dst_temp):
+                for root, dirs, files in os.walk(src_dir, topdown=True):
+                    if root == src_dir and ".git" in dirs:
+                        dirs.remove(".git")
+                    root = Path(root).relative_to(src_dir)
+                    participating_files.update(Path(root, f) for f in files)
+
+            # Merging files
+            for basename in sorted(participating_files):
+                subfile_names = []
+                for subfile_kind, src_dir in [
+                    ("modified", reference_dst_temp),
+                    ("old upstream", original_dst_temp),
+                    ("new upstream", self.dst_path),
+                ]:
+                    path = Path(src_dir, basename)
+                    if path.is_file():
+                        copyfile(path, Path(merge_dst_temp, subfile_kind))
+                    else:
+                        subfile_kind = os.devnull
+                    subfile_names.append(subfile_kind)
+
+                with local.cwd(merge_dst_temp):
+                    output = git("merge-file", "-p", *subfile_names, retcode=None)
+
+                dest_path = Path(self.dst_path, basename)
+                # Remove the file if it was already removed in the project
+                if not output and "modified" not in subfile_names:
+                    try:
+                        dest_path.unlink()
+                    except FileNotFoundError:
+                        pass
+                else:
+                    dest_path.parent.mkdir(parents=True, exist_ok=True)
+                    dest_path.write_text(output)
+
         # Run post-migration tasks
         self._execute_tasks(
             self.template.migration_tasks("after", self.subproject.template)

docs/updating.md

@@ -22,25 +22,18 @@ other git ref you want.
 
 When updating, Copier will do its best to respect your project evolution by using the
 answers you provided when copied last time. However, sometimes it's impossible for
-Copier to know what to do with a diff code hunk. In those cases, you will find `*.rej`
-files that contain the unresolved diffs. _You should review those manually_ before
-committing.
-
-You probably don't want `*.rej` files in your git history, but if you add them to
-`.gitignore`, some important changes could pass unnoticed to you. That's why the
-recommended way to deal with them is to _not_ add them to add a
-[pre-commit](https://pre-commit.com/) (or equivalent) hook that forbids them, just like
-this:
+Copier to know what to do with a diff code hunk. In those cases, each conflicted files
+will contain the unresolved diffs. _Those conflicts must be reviewed before committing_.
+That's why we recommand to add them to add a [pre-commit](https://pre-commit.com/) (or
+equivalent) hook that detects them, just like this:
 
 ```yaml title=".pre-commit-config.yaml"
 repos:
-    - repo: local
+    - repo: https://github.com/pre-commit/pre-commit-hooks
+      rev: v4.0.1
       hooks:
-          - id: forbidden-files
-            name: forbidden files
-            entry: found copier update rejection files; review them and remove them
-            language: fail
-            files: "\\.rej$"
+          - id: check-merge-conflict
+            args: [--assume-in-merge]
 ```
 
 ## Never change the answers file manually

tests/test_output.py

@@ -25,7 +25,6 @@ def test_output_force(capsys, tmp_path):
     render(tmp_path, quiet=False, defaults=True, overwrite=True)
     _, err = capsys.readouterr()
     assert re.search(r"conflict[^\s]*  config\.py", err)
-    assert re.search(r"overwrite[^\s]*  config\.py", err)
     assert re.search(r"identical[^\s]*  pyproject\.toml", err)
     assert re.search(r"identical[^\s]*  doc[/\\]images[/\\]nslogo\.gif", err)
 

tests/test_subdirectory.py

@@ -212,10 +212,13 @@ def test_new_version_uses_subdirectory(tmp_path_factory):
     copier.copy(dst_path=project_path, defaults=True, overwrite=True)
     assert "_commit: v2" in (project_path / ".copier-answers.yml").read_text()
 
-    # Assert that the README still exists, and was force updated to "upstream version 2"
+    # Assert that the README still exists with conflicts to solve
     assert (project_path / "README.md").exists()
     with (project_path / "README.md").open() as fd:
-        assert fd.read() == "upstream version 2\n"
+        assert fd.read() == (
+            "<<<<<<< modified\ndownstream version 1\n"
+            "=======\nupstream version 2\n>>>>>>> new upstream\n"
+        )
 
     # Also assert the subdirectory itself was not rendered
     assert not (project_path / "subdir").exists()

tests/test_updatediff.py

@@ -5,7 +5,7 @@
 
 import pytest
 import yaml
-from plumbum import local
+from plumbum import ProcessExecutionError, local
 from plumbum.cmd import git
 
 from copier import Worker, copy
@@ -305,17 +305,15 @@ def test_commit_hooks_respected(tmp_path_factory):
                 """,
                 ".pre-commit-config.yaml": r"""
                 repos:
+                - repo: https://github.com/pre-commit/pre-commit-hooks
+                  rev: v4.0.1
+                  hooks:
+                    - id: check-merge-conflict
+                      args: [--assume-in-merge]
                 - repo: https://github.com/pre-commit/mirrors-prettier
                   rev: v2.0.4
                   hooks:
                     - id: prettier
-                - repo: local
-                  hooks:
-                    - id: forbidden-files
-                      name: forbidden files
-                      entry: found forbidden files; remove them
-                      language: fail
-                      files: "\\.rej$"
                 """,
                 "life.yml.tmpl": """
                 # Following code should be reformatted by pre-commit after copying
@@ -401,7 +399,7 @@ def test_commit_hooks_respected(tmp_path_factory):
     with local.cwd(dst2):
         # Subproject re-updates just to change some values
         copy(data={"what": "study"}, defaults=True, overwrite=True)
-        git("commit", "-am", "re-updated to change values after evolving")
+
         # Subproject evolution was respected up to sane possibilities.
         # In an ideal world, this file would be exactly the same as what's written
         # a few lines above, just changing "grog" for "study". However, that's nearly
@@ -416,14 +414,22 @@ def test_commit_hooks_respected(tmp_path_factory):
             """\
             Line 1: hello world
             Line 2: grow up
+            <<<<<<< modified
+            Line 2.5: make friends
+            Line 3: grog
+            =======
             Line 3: study
+            >>>>>>> new upstream
             Line 4: grow old
             Line 4.5: no more work
             Line 5: bye bye world
             """
         )
-        # This time a .rej file is unavoidable
-        assert Path(f"{life}.rej").is_file()
+
+        # Assert the conflict is detected by precommit
+        with pytest.raises(ProcessExecutionError) as execution_error:
+            git("commit", "-am", "re-updated to change values after evolving")
+        assert 'Merge conflict string "<<<<<<< "' in execution_error.value.stderr
 
 
 def test_skip_update(tmp_path_factory):