diff --git a/kart/apply.py b/kart/apply.py index ca9e877d..6d700c61 100644 --- a/kart/apply.py +++ b/kart/apply.py @@ -275,7 +275,7 @@ def apply_patch( if ref != "HEAD": if not do_commit: raise click.UsageError("--no-commit and --ref are incompatible") - if not ref.startswith("refs/heads/"): + if not ref.startswith("refs/"): ref = f"refs/heads/{ref}" try: repo.references[ref] diff --git a/kart/merge.py b/kart/merge.py index d0f5ce3c..184956d9 100644 --- a/kart/merge.py +++ b/kart/merge.py @@ -8,7 +8,7 @@ from .conflicts_writer import BaseConflictsWriter from .core import check_git_user from .diff_util import get_repo_diff -from .exceptions import InvalidOperation +from .exceptions import InvalidOperation, MERGE_CONFLICT from .merge_util import ( ALL_MERGE_FILES, AncestorOursTheirs, @@ -53,7 +53,16 @@ def get_commit_message( def do_merge( - repo, ff, ff_only, dry_run, commit, message, launch_editor=True, quiet=False + repo, + ff, + ff_only, + dry_run, + commit, + message, + into="HEAD", + fail_on_conflict=False, + launch_editor=True, + quiet=False, ): """Does a merge, but doesn't update the working copy.""" if ff_only and not ff: @@ -69,9 +78,12 @@ def do_merge( # accept ref-ish things (refspec, branch, commit) theirs = CommitWithReference.resolve(repo, commit) - ours = CommitWithReference.resolve(repo, "HEAD") + ours = CommitWithReference.resolve(repo, into) ancestor_id = repo.merge_base(theirs.id, ours.id) + if not ours.reference: + raise click.BadParameter(f"--into: Ref {into!r} doesn't exist") + if not ancestor_id: raise InvalidOperation(f"Commits {theirs.id} and {ours.id} aren't related.") @@ -110,7 +122,7 @@ def do_merge( merge_jdict["commit"] = theirs.id.hex merge_jdict["fastForward"] = True if not dry_run: - repo.head.set_target( + ours.reference.set_target( theirs.id, f"{merge_context.get_message()}: Fast-forward" ) return merge_jdict @@ -125,8 +137,11 @@ def do_merge( repo, summarise=2, merged_index=merged_index, merge_context=merge_context ) merge_jdict["conflicts"] = conflicts_writer.list_conflicts() - merge_jdict["state"] = "merging" - if not dry_run: + # If ref isn't HEAD, then we can't put the repo in a 'merging' state, + # so there's currently no way to resolve conflicts. + if not fail_on_conflict: + merge_jdict["state"] = "merging" + if not fail_on_conflict and not dry_run: move_repo_to_merging_state( repo, merged_index, @@ -155,7 +170,7 @@ def do_merge( quiet=quiet, ) merge_commit_id = repo.create_commit( - repo.head.name, + ours.reference.name, user, user, message, @@ -329,6 +344,18 @@ def complete_merging_state(ctx): help="Use the given message as the commit message.", is_eager=True, # -m is eager and --continue is non-eager so we can access -m from complete_merging_state callback. ) +@click.option( + "--into", + help="Merge into the given ref instead of the currently active branch. Implies --fail-on-conflict.", + hidden=True, + default="HEAD", +) +@click.option( + "--fail-on-conflict", + help="Exits with code 1 if there are conflicts rather than entering a merging state.", + is_flag=True, + default=False, +) @click.option( " /--no-editor", "launch_editor", @@ -351,7 +378,18 @@ def complete_merging_state(ctx): ) @click.argument("commit", required=True, metavar="COMMIT") @click.pass_context -def merge(ctx, ff, ff_only, dry_run, message, launch_editor, output_format, commit): +def merge( + ctx, + ff, + ff_only, + dry_run, + message, + into, + fail_on_conflict, + launch_editor, + output_format, + commit, +): """Incorporates changes from the named commits (usually other branch heads) into the current branch.""" repo = ctx.obj.get_repo( @@ -361,6 +399,8 @@ def merge(ctx, ff, ff_only, dry_run, message, launch_editor, output_format, comm ctx.obj.check_not_dirty() do_json = output_format == "json" + if into != "HEAD": + fail_on_conflict = True jdict = do_merge( repo, @@ -369,6 +409,8 @@ def merge(ctx, ff, ff_only, dry_run, message, launch_editor, output_format, comm dry_run, commit, message, + into=into, + fail_on_conflict=fail_on_conflict, launch_editor=launch_editor, quiet=do_json, ) @@ -379,6 +421,10 @@ def merge(ctx, ff, ff_only, dry_run, message, launch_editor, output_format, comm dump_json_output({"kart.merge/v1": jdict}, sys.stdout) else: click.echo(merge_status_to_text(jdict, fresh=True)) - if not no_op and not conflicts: + if not no_op and not conflicts and into == "HEAD": repo.gc("--auto") repo.working_copy.reset_to_head(quiet=do_json) + if fail_on_conflict and conflicts: + raise InvalidOperation( + "Merge failed due to conflicts", exit_code=MERGE_CONFLICT + ) diff --git a/tests/test_merge.py b/tests/test_merge.py index 904b8392..0664a64c 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -1,7 +1,12 @@ +import io import json import pytest +from unittest.mock import ANY -from kart.exceptions import SUCCESS, INVALID_OPERATION, NO_CONFLICT +import pygit2 + +from kart.apply import apply_patch +from kart.exceptions import SUCCESS, INVALID_OPERATION, NO_CONFLICT, MERGE_CONFLICT from kart.merge_util import ( MergedIndex, CommitWithReference, @@ -360,3 +365,139 @@ def test_merge_state_lock(data_archive, cli_runner): assert r.exit_code == SUCCESS r = cli_runner.invoke(["resolve", "dummy_conflict", "--with=delete"]) assert r.exit_code == NO_CONFLICT # "dummy_conflict" is not a real conflict + + +def test_merge_into_branch(data_archive, tmp_path, cli_runner): + with data_archive("points") as repo_path: + # create two branches and put a commit on each + r = cli_runner.invoke(["branch", "b1", "main"]) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke( + ["commit-files", f"--ref=refs/heads/b1", "-m", "B1", "a=1"] + ) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke(["branch", "b2", "main"]) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke( + ["commit-files", f"--ref=refs/heads/b2", "-m", "B2", "b=1"] + ) + assert r.exit_code == 0, r.stderr + + # Merge b1 into b2, even though main is still checked out + r = cli_runner.invoke(["merge", "--into=b2", "b1", "-m", "merged"]) + assert r.exit_code == 0, r.stderr + + repo = KartRepo(repo_path) + # HEAD is unchanged + assert repo.head.name == "refs/heads/main" + head_commit = repo.head_commit + assert head_commit.message == "Improve naming on Coromandel East coast" + + # b2 ref has a merge commit on it + b2_commit = repo.references["refs/heads/b2"].peel(pygit2.Commit) + assert len(b2_commit.parents) == 2 + assert b2_commit.message == "merged" + + +def test_merge_into_branch_fastforward(data_archive, tmp_path, cli_runner): + with data_archive("points") as repo_path: + # create two branches and put a commit on one of them + r = cli_runner.invoke(["branch", "b1", "main"]) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke( + ["commit-files", f"--ref=refs/heads/b1", "-m", "B1", "a=1"] + ) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke(["branch", "b2", "main"]) + assert r.exit_code == 0, r.stderr + + # Merge b1 into b2, even though main is still checked out + r = cli_runner.invoke(["merge", "--into=b2", "b1"]) + assert r.exit_code == 0, r.stderr + + repo = KartRepo(repo_path) + b1_commit = repo.references["refs/heads/b1"].peel(pygit2.Commit) + + # HEAD is unchanged + assert repo.head.name == "refs/heads/main" + head_commit = repo.head_commit + assert head_commit.message == "Improve naming on Coromandel East coast" + + # b2 ref is now the same as b1 (because it was fastforwarded) + b2_commit = repo.references["refs/heads/b2"].peel(pygit2.Commit) + assert b2_commit.hex == b1_commit.hex + + +def _apply_features(repo, features, ref): + patch = { + "kart.diff/v1+hexwkb": { + "nz_pa_points_topo_150k": { + "feature": features, + } + }, + "kart.patch/v1": {"message": "m", "base": repo.references[ref].target.hex}, + } + patch_file = io.StringIO() + json.dump(patch, patch_file) + patch_file.seek(0) + apply_patch( + repo=repo, + ref=ref, + do_commit=True, + patch_file=patch_file, + allow_empty=False, + ) + + +def test_merge_into_branch_with_conflict(data_archive, tmp_path, cli_runner): + with data_archive("points") as repo_path: + # create two branches + r = cli_runner.invoke(["branch", "b1", "main"]) + assert r.exit_code == 0, r.stderr + r = cli_runner.invoke(["branch", "b2", "main"]) + assert r.exit_code == 0, r.stderr + + # Apply conflicting patches to two branches + FEATURE = { + "fid": 1168, + "geom": "0101000000FFA26275E7FA65405CAC5D37987E42C0", + "t50_fid": 2427412, + "name_ascii": "Tairua", + "macronated": "N", + "name": "Tairua", + } + repo = KartRepo(repo_path) + b1_commit = _apply_features( + repo, + ref="refs/heads/b1", + features=[{"-": FEATURE, "+": {**FEATURE, "name": "b1"}}], + ) + b2_commit = _apply_features( + repo, + ref="refs/heads/b2", + features=[{"-": FEATURE, "+": {**FEATURE, "name": "b2"}}], + ) + + # Merge b1 into b2, even though main is still checked out + r = cli_runner.invoke( + [ + "merge", + "--into=b2", + "b1", + "-m", + "merged", + "--output-format=json", + "--fail-on-conflict", + ] + ) + assert r.exit_code == MERGE_CONFLICT, r.stderr + assert r.stderr == "Error: Merge failed due to conflicts\n" + jdict = json.loads(r.stdout)["kart.merge/v1"] + jdict.pop("merging") + assert jdict == { + "commit": ANY, + "branch": "b2", + "message": "merged", + "conflicts": {"nz_pa_points_topo_150k": {"feature": 1}}, + "dryRun": False, + }