From 18cb98050855ef3ddd5de947cf30c00931cf3484 Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Tue, 14 Sep 2021 10:37:47 -0700 Subject: [PATCH 1/5] merge_blobs: Simplify post-merge marker detection --- gitrevise/merge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitrevise/merge.py b/gitrevise/merge.py index 0a27beb..3e2e978 100644 --- a/gitrevise/merge.py +++ b/gitrevise/merge.py @@ -245,7 +245,7 @@ def merge_blobs( if merged == preimage: print("(note) conflicted file is unchanged") - if b"<<<<<<<" in merged or b"=======" in merged or b">>>>>>>" in merged: + if any(marker in merged for marker in (b"<<<<<<<", b"=======", b">>>>>>>")): print("(note) conflict markers found in the merged file") # Was the merge successful? From 657bcb472d02e95a8b7f392ae22ab2fc3538642e Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Tue, 14 Sep 2021 10:58:08 -0700 Subject: [PATCH 2/5] merge/merge_files: Explicit option names to 'git merge-file' --- gitrevise/merge.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gitrevise/merge.py b/gitrevise/merge.py index 3e2e978..6bab02d 100644 --- a/gitrevise/merge.py +++ b/gitrevise/merge.py @@ -273,8 +273,10 @@ def merge_files( try: merged = repo.git( "merge-file", - "-q", - "-p", + # Do not print warnings on conflicts. + "--quiet", + # Send results to stdout instead of overwriting "current file". + "--stdout", f"-L{labels[0]}", f"-L{labels[1]}", f"-L{labels[2]}", From 24257152f14ff1518fbb9004fdbc8d705bb152dc Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Mon, 13 Sep 2021 13:47:37 -0700 Subject: [PATCH 3/5] merge/fmt: Consistent tmpdir argument ordering --- gitrevise/merge.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gitrevise/merge.py b/gitrevise/merge.py index 6bab02d..311a4ed 100644 --- a/gitrevise/merge.py +++ b/gitrevise/merge.py @@ -208,11 +208,11 @@ def merge_blobs( ) (is_clean_merge, merged) = merge_files( repo, + tmpdir, annotated_labels, current.body, base.body if base else b"", other.body, - tmpdir, ) if is_clean_merge: @@ -259,11 +259,11 @@ def merge_blobs( def merge_files( repo: Repository, + tmpdir: Path, labels: Tuple[str, str, str], current: bytes, base: bytes, other: bytes, - tmpdir: Path, ) -> Tuple[bool, bytes]: (tmpdir / "current").write_bytes(current) (tmpdir / "base").write_bytes(base) @@ -325,11 +325,11 @@ def replay_recorded_resolution( (is_clean_merge, merged) = merge_files( repo, + tmpdir=tmpdir, labels=("recorded postimage", "recorded preimage", "new preimage"), current=recorded_postimage, base=recorded_preimage, other=normalized_preimage, - tmpdir=tmpdir, ) if not is_clean_merge: # We could ask the user to merge this. However, that could be confusing. From 406d82d1e6bc196c392e00ac48e6acdfac8e2d3f Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Tue, 14 Sep 2021 14:44:15 -0700 Subject: [PATCH 4/5] merge: parameterize conflict-marker size --- gitrevise/merge.py | 60 +++++++++++++++++++++++++++++++++----------- tests/test_rerere.py | 22 +++++++++------- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/gitrevise/merge.py b/gitrevise/merge.py index 311a4ed..1824d2d 100644 --- a/gitrevise/merge.py +++ b/gitrevise/merge.py @@ -25,6 +25,10 @@ T = TypeVar("T") # pylint: disable=invalid-name +# See git/xdiff/xdiff.h +DEFAULT_CONFLICT_MARKER_SIZE = 7 + + class MergeConflict(Exception): pass @@ -197,10 +201,13 @@ def merge_blobs( base: Optional[Blob], other: Blob, ) -> Blob: + # pylint: disable=too-many-locals repo = current.repo tmpdir = repo.get_tempdir() + marker_size = DEFAULT_CONFLICT_MARKER_SIZE + annotated_labels = ( f"{path} (new parent): {labels[0]}", f"{path} (old parent): {labels[1]}", @@ -209,6 +216,7 @@ def merge_blobs( (is_clean_merge, merged) = merge_files( repo, tmpdir, + marker_size, annotated_labels, current.body, base.body if base else b"", @@ -226,7 +234,10 @@ def merge_blobs( preimage = merged (normalized_preimage, conflict_id, merged_blob) = replay_recorded_resolution( - repo, tmpdir, preimage + repo=repo, + tmpdir=tmpdir, + marker_size=marker_size, + preimage=preimage, ) if merged_blob is not None: return merged_blob @@ -245,7 +256,7 @@ def merge_blobs( if merged == preimage: print("(note) conflicted file is unchanged") - if any(marker in merged for marker in (b"<<<<<<<", b"=======", b">>>>>>>")): + if any((marker * marker_size) in merged for marker in (b"<", b"=", b">")): print("(note) conflict markers found in the merged file") # Was the merge successful? @@ -260,6 +271,7 @@ def merge_blobs( def merge_files( repo: Repository, tmpdir: Path, + marker_size: int, labels: Tuple[str, str, str], current: bytes, base: bytes, @@ -277,6 +289,9 @@ def merge_files( "--quiet", # Send results to stdout instead of overwriting "current file". "--stdout", + # Ensure markers are the expected length to ensure consistent rerere results. + "--marker-size", + str(marker_size), f"-L{labels[0]}", f"-L{labels[1]}", f"-L{labels[2]}", @@ -297,7 +312,10 @@ def merge_files( def replay_recorded_resolution( - repo: Repository, tmpdir: Path, preimage: bytes + repo: Repository, + tmpdir: Path, + marker_size: int, + preimage: bytes, ) -> Tuple[bytes, Optional[str], Optional[Blob]]: rr_cache = repo.git_path("rr-cache") if not repo.bool_config( @@ -306,7 +324,11 @@ def replay_recorded_resolution( ): return (b"", None, None) - (normalized_preimage, conflict_id) = normalize_conflicted_file(preimage) + (normalized_preimage, conflict_id) = normalize_conflicted_file( + marker_size=marker_size, + body=preimage, + ) + conflict_dir = rr_cache / conflict_id if not conflict_dir.is_dir(): return (normalized_preimage, conflict_id, None) @@ -326,6 +348,7 @@ def replay_recorded_resolution( (is_clean_merge, merged) = merge_files( repo, tmpdir=tmpdir, + marker_size=marker_size, labels=("recorded postimage", "recorded preimage", "new preimage"), current=recorded_postimage, base=recorded_preimage, @@ -367,6 +390,7 @@ class ConflictParseFailed(Exception): def normalize_conflict( + marker_size: int, lines: Iterator[bytes], hasher: Optional[hashlib._Hash], ) -> bytes: @@ -376,17 +400,17 @@ def normalize_conflict( line = next(lines, None) if line is None: raise ConflictParseFailed("unexpected eof") - if line.startswith(b"<<<<<<< "): + if line.startswith((b"<" * marker_size) + b" "): # parse recursive conflicts, including their processed output in the current hunk - conflict = normalize_conflict(lines, None) + conflict = normalize_conflict(marker_size, lines, None) if cur_hunk is not None: cur_hunk += conflict - elif line.startswith(b"|||||||"): + elif line.startswith(b"|" * marker_size): # ignore the diff3 original section. Must be still parsing the first hunk. if other_hunk is not None: raise ConflictParseFailed("unexpected ||||||| conflict marker") (other_hunk, cur_hunk) = (cur_hunk, None) - elif line.startswith(b"======="): + elif line.startswith(b"=" * marker_size): # switch into the second hunk # could be in either the diff3 original section or the first hunk if cur_hunk is not None: @@ -394,7 +418,7 @@ def normalize_conflict( raise ConflictParseFailed("unexpected ======= conflict marker") other_hunk = cur_hunk cur_hunk = b"" - elif line.startswith(b">>>>>>> "): + elif line.startswith((b">" * marker_size) + b" "): # end of conflict. update hasher, and return a normalized conflict if cur_hunk is None or other_hunk is None: raise ConflictParseFailed("unexpected >>>>>>> conflict marker") @@ -405,11 +429,14 @@ def normalize_conflict( hasher.update(hunk2 + b"\0") return b"".join( ( - b"<<<<<<<\n", + b"<" * marker_size, + b"\n", hunk1, - b"=======\n", + b"=" * marker_size, + b"\n", hunk2, - b">>>>>>>\n", + b">" * marker_size, + b"\n", ) ) elif cur_hunk is not None: @@ -418,7 +445,10 @@ def normalize_conflict( cur_hunk += line -def normalize_conflicted_file(body: bytes) -> Tuple[bytes, str]: +def normalize_conflicted_file( + marker_size: int, + body: bytes, +) -> Tuple[bytes, str]: hasher = hashlib.sha1() normalized = b"" @@ -427,7 +457,7 @@ def normalize_conflicted_file(body: bytes) -> Tuple[bytes, str]: line = next(lines, None) if line is None: return (normalized, hasher.hexdigest()) - if line.startswith(b"<<<<<<< "): - normalized += normalize_conflict(lines, hasher) + if line.startswith((b"<" * marker_size) + b" "): + normalized += normalize_conflict(marker_size, lines, hasher) else: normalized += line diff --git a/tests/test_rerere.py b/tests/test_rerere.py index 8b278b5..9c63d98 100644 --- a/tests/test_rerere.py +++ b/tests/test_rerere.py @@ -1,6 +1,6 @@ import textwrap -from gitrevise.merge import normalize_conflicted_file +from gitrevise.merge import DEFAULT_CONFLICT_MARKER_SIZE, normalize_conflicted_file from gitrevise.odb import Repository from .conftest import Editor, bash, changeline, editor_main, main @@ -222,7 +222,8 @@ def test_normalize_conflicted_file() -> None: # Normalize conflict markers and labels. assert ( normalize_conflicted_file( - dedent( + marker_size=DEFAULT_CONFLICT_MARKER_SIZE, + body=dedent( """\ <<<<<<< HEAD a @@ -238,7 +239,7 @@ def test_normalize_conflicted_file() -> None: d >>>>>>>>>> longer conflict marker, to be ignored """ - ) + ), ) == ( dedent( @@ -265,7 +266,8 @@ def test_normalize_conflicted_file() -> None: # Discard original-text-marker from merge.conflictStyle diff3. assert ( normalize_conflicted_file( - dedent( + marker_size=DEFAULT_CONFLICT_MARKER_SIZE, + body=dedent( """\ <<<<<<< theirs a @@ -275,7 +277,7 @@ def test_normalize_conflicted_file() -> None: c >>>>>>> ours """ - ) + ), )[0] == dedent( """\ @@ -291,7 +293,8 @@ def test_normalize_conflicted_file() -> None: # The two sides of the conflict are ordered. assert ( normalize_conflicted_file( - dedent( + marker_size=DEFAULT_CONFLICT_MARKER_SIZE, + body=dedent( """\ <<<<<<< this way round b @@ -299,7 +302,7 @@ def test_normalize_conflicted_file() -> None: a >>>>>>> (unsorted) """ - ) + ), )[0] == dedent( """\ @@ -315,7 +318,8 @@ def test_normalize_conflicted_file() -> None: # Nested conflict markers. assert ( normalize_conflicted_file( - dedent( + marker_size=DEFAULT_CONFLICT_MARKER_SIZE, + body=dedent( """\ <<<<<<< ours (outer) outer left @@ -330,7 +334,7 @@ def test_normalize_conflicted_file() -> None: outer right >>>>>>> theirs (outer) """ - ) + ), )[0] == dedent( """\ From 4359ed93f48412e5273cf2a67955123a765e2660 Mon Sep 17 00:00:00 2001 From: Robert Estelle Date: Tue, 14 Sep 2021 15:41:07 -0700 Subject: [PATCH 5/5] merge: Dummy determination of conflict marker size --- gitrevise/merge.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/gitrevise/merge.py b/gitrevise/merge.py index 1824d2d..9ea72ba 100644 --- a/gitrevise/merge.py +++ b/gitrevise/merge.py @@ -29,6 +29,12 @@ DEFAULT_CONFLICT_MARKER_SIZE = 7 +def get_conflict_marker_size(__repo: Repository, __file: Path) -> int: + # TODO: Determine on a per-file basis by its `conflict-marker-size` attribute. + # See ll_merge_marker_size in git/ll-merge.c + return DEFAULT_CONFLICT_MARKER_SIZE + + class MergeConflict(Exception): pass @@ -206,7 +212,7 @@ def merge_blobs( tmpdir = repo.get_tempdir() - marker_size = DEFAULT_CONFLICT_MARKER_SIZE + marker_size = get_conflict_marker_size(repo, path) annotated_labels = ( f"{path} (new parent): {labels[0]}",