Merge pull request #1020 from koordinates/merge-into-alternate-branch

merge: Add `--into <ref>`

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]

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
+        )

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,
+        }