diff --git a/sno/merge.py b/sno/merge.py index 4810f3ef2..b1a5386de 100644 --- a/sno/merge.py +++ b/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,22 +76,22 @@ 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, @@ -99,7 +99,7 @@ def do_merge(repo, ff, ff_only, dry_run, commit): 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: diff --git a/sno/merge_util.py b/sno/merge_util.py index 12c030552..458f9691b 100644 --- a/sno/merge_util.py +++ b/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: diff --git a/sno/repo_files.py b/sno/repo_files.py index a1069a003..e09c2d74a 100644 --- a/sno/repo_files.py +++ b/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) diff --git a/sno/structs.py b/sno/structs.py index 9182bc257..5611df0c1 100644 --- a/sno/structs.py +++ b/sno/structs.py @@ -6,8 +6,8 @@ class CommitWithReference: """ Simple struct containing a commit, and optionally the reference used to find the commit. - When struct is passed around in place of sole commit, then any code that uses the commit is able to give a better - human-readable name to describe it. + When struct is passed around in place of sole commit, then any code that uses the commit + is able to give a better human-readable name to describe it. Example: >>> cwr = CommitWithReference.resolve(repo, "master") >>> cwr.commit.id @@ -38,6 +38,10 @@ def __repr__(self): def id(self): return self.commit.id + @property + def short_id(self): + return self.commit.short_id + @property def tree(self): return self.commit.tree @@ -46,7 +50,7 @@ def tree(self): def shorthand(self): if self.reference is not None: return self.reference.shorthand - return self.id.hex + return self.commit.short_id @property def reference_type(self): @@ -59,14 +63,8 @@ def reference_type(self): return None @property - def shorthand_with_type(self): - if self.reference is not None: - ref_type = self.reference_type - if ref_type: - return f'{ref_type} "{self.reference.shorthand}"' - else: - return f'"{self.reference.shorthand}"' - return self.id.hex + def branch_shorthand(self): + return self.reference.shorthand if self.reference_type == "branch" else None @staticmethod def resolve(repo, refish): diff --git a/tests/test_conflicts.py b/tests/test_conflicts.py index 329fbefdc..fa516aa24 100644 --- a/tests/test_conflicts.py +++ b/tests/test_conflicts.py @@ -1,15 +1,12 @@ -import contextlib import json import pytest -import pygit2 - from sno.merge_util import MergeIndex, MergedOursTheirs from sno.structs import CommitWithReference from sno.repo_files import ( MERGE_HEAD, + MERGE_BRANCH, MERGE_MSG, - MERGE_LABELS, MERGE_INDEX, repo_file_exists, read_repo_file, @@ -48,8 +45,16 @@ def test_merge_conflicts( assert r.exit_code == 0, r if output_format == "text": - dry_run_message = ( - ["(Not actually merging due to --dry-run)", ""] if dry_run else [""] + merging_state_message = ( + ["(Not actually merging due to --dry-run)", ""] + if dry_run + else [ + 'Repository is now in "merging" state.', + "View conflicts with `sno conflicts` and resolve them with `sno resolve`.", + "Once no conflicts remain, complete this merge with `sno merge --continue`.", + "Or use `sno merge --abort` to return to the previous state.", + "", + ] ) assert ( @@ -64,7 +69,7 @@ def test_merge_conflicts( " edit/edit: 3", "", ] - + dry_run_message + + merging_state_message ) else: @@ -72,9 +77,23 @@ def test_merge_conflicts( assert jdict == { "sno.merge/v1": { "branch": "ours_branch", - "ancestor": ancestor.id.hex, - "ours": ours.id.hex, - "theirs": theirs.id.hex, + "commit": ours.id.hex, + "merging": { + "ancestor": { + "commit": ancestor.id.hex, + "abbrevCommit": ancestor.short_id, + }, + "ours": { + "branch": "ours_branch", + "commit": ours.id.hex, + "abbrevCommit": ours.short_id, + }, + "theirs": { + "branch": "theirs_branch", + "commit": theirs.id.hex, + "abbrevCommit": theirs.short_id, + }, + }, "dryRun": dry_run, "message": "Merge branch \"theirs_branch\" into ours_branch", "conflicts": { @@ -82,26 +101,25 @@ def test_merge_conflicts( "featureConflicts": {"add/add": 1, "edit/edit": 3} }, }, + "state": "merging", }, } if not dry_run: assert read_repo_file(repo, MERGE_HEAD) == theirs.id.hex + "\n" + assert read_repo_file(repo, MERGE_BRANCH) == "theirs_branch\n" assert ( read_repo_file(repo, MERGE_MSG) == "Merge branch \"theirs_branch\" into ours_branch\n" ) - assert ( - read_repo_file(repo, MERGE_LABELS) - == f'({ancestor.id.hex})\n"ours_branch" ({ours.id.hex})\n"theirs_branch" ({theirs.id.hex})\n' - ) + merge_index = MergeIndex.read_from_repo(repo) assert len(merge_index.conflicts) == 4 cli_runner.invoke(["merge", "--abort"]) assert not repo_file_exists(repo, MERGE_HEAD) + assert not repo_file_exists(repo, MERGE_BRANCH) assert not repo_file_exists(repo, MERGE_MSG) - assert not repo_file_exists(repo, MERGE_LABELS) assert not repo_file_exists(repo, MERGE_INDEX) diff --git a/tests/test_merge.py b/tests/test_merge.py index 313110912..6db64a26b 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -92,7 +92,7 @@ def test_merge_fastforward_noff( H.git_graph(request, "post-merge") - merge_commit_id = json.loads(r.stdout)["sno.merge/v1"]["mergeCommit"] + merge_commit_id = json.loads(r.stdout)["sno.merge/v1"]["commit"] assert repo.head.name == "refs/heads/master" assert repo.head.target.hex == merge_commit_id @@ -148,7 +148,7 @@ def test_merge_true( assert r.exit_code == 0, r H.git_graph(request, "post-merge") - merge_commit_id = json.loads(r.stdout)["sno.merge/v1"]["mergeCommit"] + merge_commit_id = json.loads(r.stdout)["sno.merge/v1"]["commit"] assert repo.head.name == "refs/heads/master" assert repo.head.target.hex == merge_commit_id