Merge pull request #95 from koordinates/merge_context

Update how merge_context is stored, and JSON output

sno/merge.py

@@ -44,20 +44,20 @@ def do_merge(repo, ff, ff_only, dry_run, commit):
         raise InvalidOperation(f"Commits {theirs.id} and {ours.id} aren't related.")
 
     ancestor = CommitWithReference.resolve(repo, ancestor_id)
+    commit_with_ref3 = AncestorOursTheirs(ancestor, ours, theirs)
+    merge_context = MergeContext.from_commit_with_refs(commit_with_ref3, repo)
+    merge_message = merge_context.get_message()
 
-    merge_message = f"Merge {theirs.shorthand_with_type} into {ours.shorthand}"
     merge_jdict = {
-        "branch": ours.shorthand,
-        "ancestor": ancestor.id.hex,
-        "ours": ours.id.hex,
-        "theirs": theirs.id.hex,
+        "commit": ours.id.hex,
+        "branch": ours.branch_shorthand,
+        "merging": merge_context.as_json(),
         "message": merge_message,
         "conflicts": None,
     }
 
     # We're up-to-date if we're trying to merge our own common ancestor.
     if ancestor_id == theirs.id:
-        merge_jdict["mergeCommit"] = ours.id.hex
         merge_jdict["noOp"] = True
         return merge_jdict
 
@@ -76,30 +76,30 @@ def do_merge(repo, ff, ff_only, dry_run, commit):
     if can_ff and ff:
         # do fast-forward merge
         L.debug(f"Fast forward: {theirs.id.hex}")
-        merge_jdict["mergeCommit"] = theirs.id.hex
+        merge_jdict["commit"] = theirs.id.hex
         merge_jdict["fastForward"] = True
         if not dry_run:
             repo.head.set_target(theirs.id, f"{merge_message}: Fast-forward")
         return merge_jdict
 
-    commit_with_ref3 = AncestorOursTheirs(ancestor, ours, theirs)
     tree3 = commit_with_ref3.map(lambda c: c.tree)
     index = repo.merge_trees(**tree3.as_dict())
 
     if index.conflicts:
         merge_index = MergeIndex.from_pygit2_index(index)
-        merge_context = MergeContext.from_commit_with_refs(commit_with_ref3, repo)
+
         merge_jdict["conflicts"] = list_conflicts(
             merge_index, merge_context, "json", summarise=2
         )
+        merge_jdict["state"] = "merging"
         if not dry_run:
             move_repo_to_merging_state(
                 repo, merge_index, merge_context, merge_message,
             )
         return merge_jdict
 
     if dry_run:
-        merge_jdict["mergeCommit"] = "(dryRun)"
+        merge_jdict["commit"] = "(dryRun)"
         return merge_jdict
 
     merge_tree_id = index.write_tree(repo)
@@ -111,7 +111,7 @@ def do_merge(repo, ff, ff_only, dry_run, commit):
     )
 
     L.debug(f"Merge commit: {merge_commit_id}")
-    merge_jdict["mergeCommit"] = merge_commit_id.hex
+    merge_jdict["commit"] = merge_commit_id.hex
 
     return merge_jdict
 
@@ -169,14 +169,6 @@ def complete_merging_state(ctx):
     commit_ids = merge_context.versions.map(lambda v: v.repo_structure.id)
     merge_message = read_repo_file(repo, MERGE_MSG)
 
-    merge_jdict = {
-        "branch": CommitWithReference.resolve(repo, "HEAD").shorthand,
-        "ancestor": commit_ids.ancestor,
-        "ours": commit_ids.ours,
-        "theirs": commit_ids.theirs,
-        "message": merge_message,
-    }
-
     merge_tree_id = merge_index.write_resolved_tree(repo)
     L.debug(f"Merge tree: {merge_tree_id}")
 
@@ -191,7 +183,14 @@ def complete_merging_state(ctx):
     )
 
     L.debug(f"Merge commit: {merge_commit_id}")
-    merge_jdict["mergeCommit"] = merge_commit_id.hex
+
+    head = CommitWithReference.resolve(repo, "HEAD")
+    merge_jdict = {
+        "branch": head.branch_shorthand,
+        "commit": merge_commit_id,
+        "merging": merge_context.as_json(),
+        "message": merge_message,
+    }
 
     repo_structure = RepositoryStructure(repo)
     wc = repo_structure.working_copy
@@ -205,23 +204,30 @@ def complete_merging_state(ctx):
 
 
 def output_merge_json_as_text(jdict):
-    click.echo(jdict["message"].replace("Merge", "Merging", 1))
+    theirs = jdict["merging"]["theirs"]
+    ours = jdict["merging"]["ours"]
+    theirs_branch = theirs.get("branch", None)
+    theirs_desc = (
+        f'branch "{theirs_branch}"' if theirs_branch else theirs["abbrevCommit"]
+    )
+    ours_desc = ours.get("branch", None) or ours["abbrevCommit"]
+    click.echo(f"Merging {theirs_desc} into {ours_desc}")
 
     if jdict.get("noOp", False):
         click.echo("Already up to date")
         return
 
     dry_run = jdict.get("dryRun", False)
-    merge_commit = jdict.get("mergeCommit", None)
+    commit = jdict.get("commit", None)
 
     if jdict.get("fastForward", False):
         if dry_run:
             click.echo(
-                f"Can fast-forward to {merge_commit}\n"
+                f"Can fast-forward to {commit}\n"
                 "(Not actually fast-forwarding due to --dry-run)",
             )
         else:
-            click.echo(f"Fast-forwarded to {merge_commit}")
+            click.echo(f"Fast-forwarded to {commit}")
         return
 
     conflicts = jdict.get("conflicts", None)
@@ -232,7 +238,7 @@ def output_merge_json_as_text(jdict):
                 "(Not actually merging due to --dry-run)"
             )
         else:
-            click.echo(f"No conflicts!\nMerge commited as {merge_commit}")
+            click.echo(f"No conflicts!\nMerge commited as {commit}")
         return
 
     click.echo("Conflicts found:\n")
@@ -242,8 +248,14 @@ def output_merge_json_as_text(jdict):
         click.echo("(Not actually merging due to --dry-run)")
     else:
         # TODO: explain how to resolve conflicts, when this is possible
-        click.echo("Sorry, resolving merge conflicts is not yet supported", err=True)
-        click.echo("Use `sno merge --abort` to abort this merge", err=True)
+        click.echo('Repository is now in "merging" state.')
+        click.echo(
+            "View conflicts with `sno conflicts` and resolve them with `sno resolve`."
+        )
+        click.echo(
+            "Once no conflicts remain, complete this merge with `sno merge --continue`."
+        )
+        click.echo("Or use `sno merge --abort` to return to the previous state.")
 
 
 @click.command()
@@ -302,7 +314,7 @@ def merge(ctx, ff, ff_only, dry_run, do_json, commit):
         wc = repo_structure.working_copy
         if wc:
             L.debug(f"Updating {wc.path} ...")
-            merge_commit = repo[merge_jdict["mergeCommit"]]
+            merge_commit = repo[merge_jdict["commit"]]
             wc.reset(merge_commit, repo_structure)
 
     if do_json:

sno/merge_util.py

@@ -7,15 +7,16 @@
 from .diff_output import text_row, json_row, geojson_row
 from .exceptions import InvalidOperation
 from .repo_files import (
-    ORIG_HEAD,
     MERGE_HEAD,
     MERGE_INDEX,
-    MERGE_LABELS,
+    MERGE_BRANCH,
     is_ongoing_merge,
     read_repo_file,
     write_repo_file,
+    remove_repo_file,
     repo_file_path,
 )
+from .structs import CommitWithReference
 from .structure import RepositoryStructure
 from .utils import ungenerator
 
@@ -412,15 +413,38 @@ def _ensure_resolve(cls, resolve):
 class VersionContext:
     """
     The necessary context for categorising or outputting a single version of a conflict.
-    Holds the appropriate version of the repository structure,
-    the name of that version - one of "ancestor", "ours" or "theirs",
-    and a label for that version (the branch name or commit SHA).
+    Holds the name of that version - one of "ancestor", "ours" or "theirs", the commit ID
+    of that version, and optionally the name of the branch that was dereferenced to select
+    that commit ID (note that  the branch may or may not still point to that commit ID).
     """
 
-    def __init__(self, repo_structure, version_name, version_label):
-        self.repo_structure = repo_structure
+    def __init__(self, repo, version_name, commit_id, short_id, branch=None):
+        # The sno repository
+        self.repo = repo
+        # The name of the version - one of "ancestor", "ours" or "theirs".
         self.version_name = version_name
-        self.version_label = version_label
+        # The commit ID - a pygit2.Oid object.
+        self.commit_id = commit_id
+        # A shorter but still unique prefix of the commit ID - a string.
+        self.short_id = short_id
+        # The name of the branch used to find the commit, or None if none was used.
+        self.branch = branch
+
+    @property
+    def repo_structure(self):
+        if not hasattr(self, "_repo_structure"):
+            self._repo_structure = RepositoryStructure.lookup(self.repo, self.commit_id)
+        return self._repo_structure
+
+    @property
+    def shorthand(self):
+        return self.branch if self.branch else self.commit_id.hex
+
+    def as_json(self):
+        result = {"commit": self.commit_id.hex, "abbrevCommit": self.short_id}
+        if self.branch:
+            result["branch"] = self.branch
+        return result
 
 
 class MergeContext:
@@ -431,55 +455,73 @@ def __init__(self, versions):
         self.versions = versions
 
     @classmethod
-    def _zip_together(cls, repo_structures3, labels3):
+    def _zip_together(cls, repo, commit_ids3, short_ids3, branches3):
         names3 = AncestorOursTheirs.NAMES
         versions = AncestorOursTheirs(
             *(
-                VersionContext(rs, n, l)
-                for rs, n, l in zip(repo_structures3, names3, labels3)
+                VersionContext(repo, n, c, s, b)
+                for n, c, s, b in zip(names3, commit_ids3, short_ids3, branches3)
             )
         )
         return MergeContext(versions)
 
     @classmethod
     def from_commit_with_refs(cls, commit_with_refs3, repo):
-        repo_structures3 = commit_with_refs3.map(
-            lambda c: RepositoryStructure(repo, commit=c.commit)
-        )
-        labels3 = commit_with_refs3.map(lambda c: str(c))
-        return cls._zip_together(repo_structures3, labels3)
+        commit_ids3 = commit_with_refs3.map(lambda c: c.id)
+        short_ids3 = commit_with_refs3.map(lambda c: c.short_id)
+        branches3 = commit_with_refs3.map(lambda c: c.branch_shorthand)
+        return cls._zip_together(repo, commit_ids3, short_ids3, branches3)
 
     @classmethod
     def read_from_repo(cls, repo):
         if not is_ongoing_merge(repo):
             raise InvalidOperation("Repository is not in 'merging' state")
-        ours = RepositoryStructure.lookup(repo, "HEAD")
-        theirs = RepositoryStructure.lookup(
-            repo, read_repo_file(repo, MERGE_HEAD).strip()
+
+        # HEAD is assumed to be our side of the merge. MERGE_HEAD (and MERGE_INDEX)
+        # are not version controlled, but are simply files in the repo. For these
+        # reasons, the user should not be able to change branch mid merge.
+
+        head = CommitWithReference.resolve(repo, "HEAD")
+        ours_commit_id = head.id
+        theirs_commit_id = pygit2.Oid(hex=read_repo_file(repo, MERGE_HEAD).strip())
+
+        commit_ids3 = AncestorOursTheirs(
+            # We find the ancestor by recalculating it fresh each time.
+            repo.merge_base(ours_commit_id, theirs_commit_id),
+            ours_commit_id,
+            theirs_commit_id,
         )
-        # We find the ancestor be recalculating it fresh each time. TODO: is that good?
-        ancestor_id = repo.merge_base(theirs.id, ours.id)
-        ancestor = RepositoryStructure.lookup(repo, ancestor_id)
-        repo_structures3 = AncestorOursTheirs(ancestor, ours, theirs)
-        labels3 = AncestorOursTheirs(
-            *read_repo_file(repo, MERGE_LABELS).strip().split("\n")
+        short_ids3 = commit_ids3.map(lambda c: repo[c].short_id)
+        branches3 = AncestorOursTheirs(
+            None,
+            head.branch_shorthand,
+            read_repo_file(repo, MERGE_BRANCH, missing_ok=True),
         )
-        return cls._zip_together(repo_structures3, labels3)
+
+        return cls._zip_together(repo, commit_ids3, short_ids3, branches3)
 
     def write_to_repo(self, repo):
-        commits3 = self.versions.map(lambda v: v.repo_structure.head_commit)
-        labels3 = self.versions.map(lambda v: v.version_label)
-        # We don't write commits3.ancestor - we just recalculate it each time.
-        # And we don't write commits3.ours - thats already stored in HEAD.
-        # So we just write commits3.theirs:
-        write_repo_file(repo, MERGE_HEAD, commits3.theirs.id.hex)
-        # We also don't write an ORIG_HEAD, since we don't change HEAD during a merge.
-
-        # We write labels for what we are merging to MERGE_LABELS - these include
-        # the names of the branches sno merge was given to merge, although these
-        # are merely informational since those branch heads could move ahead
-        # to new commits before this merge is completed.
-        write_repo_file(repo, MERGE_LABELS, "".join(f"{l}\n" for l in labels3))
+        # We don't write ancestor.commit_id - we just recalculate it when needed.
+        # We don't write ours.commit_id - we can learn that from HEAD.
+        # So we just write theirs.commit_id in MERGE_HEAD.
+        write_repo_file(repo, MERGE_HEAD, self.versions.theirs.commit_id.hex)
+
+        # We don't write ancestor.branch, since it's always None anyway.
+        # We don't write ours.branch. we can learn that from HEAD.
+        # So we just write theirs.branch in MERGE_BRANCH, unless its None.
+        if self.versions.theirs.branch:
+            write_repo_file(repo, MERGE_BRANCH, self.versions.theirs.branch)
+        else:
+            remove_repo_file(repo, MERGE_BRANCH)
+
+    def get_message(self):
+        theirs = self.versions.theirs
+        theirs_desc = f'branch "{theirs.branch}"' if theirs.branch else theirs.shorthand
+        return f'Merge {theirs_desc} into {self.versions.ours.shorthand}'
+
+    def as_json(self):
+        json3 = self.versions.map(lambda v: v.as_json())
+        return json3.as_dict()
 
 
 class RichConflictVersion:

sno/repo_files.py

@@ -9,14 +9,16 @@
 from . import is_windows
 from .exceptions import SubprocessError
 
-
+# Standard git files:
 HEAD = "HEAD"
 COMMIT_EDITMSG = "COMMIT_EDITMSG"
 ORIG_HEAD = "ORIG_HEAD"
 MERGE_HEAD = "MERGE_HEAD"
 MERGE_MSG = "MERGE_MSG"
+
+# Sno-specific files:
 MERGE_INDEX = "MERGE_INDEX"
-MERGE_LABELS = "MERGE_LABELS"
+MERGE_BRANCH = "MERGE_BRANCH"
 
 
 def repo_file_path(repo, filename):
@@ -69,14 +71,16 @@ def user_edit_repo_file(repo, filename):
         ) from e
 
 
-def read_repo_file(repo, filename):
+def read_repo_file(repo, filename, missing_ok=False):
     path = repo_file_path(repo, filename)
+    if missing_ok and not path.exists():
+        return None
     return path.read_text(encoding="utf-8")
 
 
 def remove_repo_file(repo, filename, missing_ok=True):
     path = repo_file_path(repo, filename)
-    if not path.exists() and missing_ok:
+    if missing_ok and not path.exists():
         return  # TODO: use path.unlink(missing_ok=True) (python3.8)
     path.unlink()
 
@@ -92,8 +96,8 @@ def is_ongoing_merge(repo):
 
 
 def remove_all_merge_repo_files(repo):
-    """Deletes the following files (if they exist) - MERGE_HEAD, MERGE_MSG, MERGE_INDEX, MERGE_LABELS."""
+    """Deletes the following files (if they exist) - MERGE_HEAD, MERGE_BRANCH, MERGE_MSG, MERGE_INDEX"""
     remove_repo_file(repo, MERGE_HEAD)
+    remove_repo_file(repo, MERGE_BRANCH)
     remove_repo_file(repo, MERGE_MSG)
     remove_repo_file(repo, MERGE_INDEX)
-    remove_repo_file(repo, MERGE_LABELS)