Skip to content

Commit

Permalink
Improve conflict resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
Thierry committed Mar 27, 2022
1 parent e983140 commit d7515f0
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 90 deletions.
134 changes: 74 additions & 60 deletions copier/main.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Main functions and classes, used to generate or update projects."""

import os
import platform
import subprocess
import sys
Expand All @@ -8,15 +9,14 @@
from functools import partial
from itertools import chain
from pathlib import Path
from shutil import rmtree
from shutil import copyfile, copytree, rmtree
from typing import Callable, Iterable, List, Mapping, Optional, Sequence
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
Expand Down Expand Up @@ -218,10 +218,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",
Expand All @@ -239,16 +236,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,
Expand Down Expand Up @@ -657,26 +646,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")
Expand All @@ -686,37 +680,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)
Expand Down
25 changes: 9 additions & 16 deletions docs/updating.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
hooks:
- id: forbidden-files
name: forbidden files
entry: found copier update rejection files; review them and remove them
language: fail
files: "\\.rej$"
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
hooks:
- id: check-merge-conflict
args: [--assume-in-merge]
```
## Never change the answers file manually
Expand Down
1 change: 0 additions & 1 deletion tests/test_output.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 5 additions & 2 deletions tests/test_subdirectory.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
28 changes: 17 additions & 11 deletions tests/test_updatediff.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down

0 comments on commit d7515f0

Please sign in to comment.