]> source.dussan.org Git - jgit.git/commitdiff
Add Union merge strategy support 79/1203079/4
authorNasser Grainawi <quic_nasserg@quicinc.com>
Tue, 22 Oct 2024 02:26:35 +0000 (20:26 -0600)
committerNasser Grainawi <quic_nasserg@quicinc.com>
Mon, 4 Nov 2024 22:34:46 +0000 (15:34 -0700)
Allow users to specify the `union` strategy in their .gitattributes file
in order to keep lines from both versions of a conflict [1].

[1] https://git-scm.com/docs/gitattributes.html#Documentation/gitattributes.txt-union

Change-Id: I74cecceb2db819a8551b95fb10dfe7c2b160b709

org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/merge/MergeGitAttributeTest.java
org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmUnionTest.java [new file with mode: 0644]
org.eclipse.jgit/.settings/.api_filters
org.eclipse.jgit/src/org/eclipse/jgit/lib/Constants.java
org.eclipse.jgit/src/org/eclipse/jgit/merge/ContentMergeStrategy.java
org.eclipse.jgit/src/org/eclipse/jgit/merge/MergeAlgorithm.java
org.eclipse.jgit/src/org/eclipse/jgit/merge/ResolveMerger.java

index 009ca8a15203b44ae54d995c0e5c6c5acadb200e..ac30c6c5261d56b534663f47a1a568e95d7ab64b 100644 (file)
@@ -267,6 +267,51 @@ public class MergeGitAttributeTest extends RepositoryTestCase {
                }
        }
 
+       @Test
+       public void mergeTextualFile_SetUnionMerge() throws NoWorkTreeException,
+                       NoFilepatternException, GitAPIException, IOException {
+               try (Git git = createRepositoryBinaryConflict(g -> {
+                       try {
+                               writeTrashFile(".gitattributes", "*.cat merge=union");
+                               writeTrashFile("main.cat", "A\n" + "B\n" + "C\n" + "D\n");
+                       } catch (IOException e) {
+                               throw new UncheckedIOException(e);
+                       }
+               }, g -> {
+                       try {
+                               writeTrashFile("main.cat", "A\n" + "G\n" + "C\n" + "F\n");
+                       } catch (IOException e) {
+                               throw new UncheckedIOException(e);
+                       }
+               }, g -> {
+                       try {
+                               writeTrashFile("main.cat", "A\n" + "E\n" + "C\n" + "D\n");
+                       } catch (IOException e) {
+                               throw new UncheckedIOException(e);
+                       }
+               })) {
+                       // Check that the merge attribute is set to union
+                       assertAddMergeAttributeCustom(REFS_HEADS_LEFT, "main.cat", "union");
+                       assertAddMergeAttributeCustom(REFS_HEADS_RIGHT, "main.cat",
+                                       "union");
+
+                       checkoutBranch(REFS_HEADS_LEFT);
+                       // Merge refs/heads/left -> refs/heads/right
+
+                       MergeResult mergeResult = git.merge()
+                                       .include(git.getRepository().resolve(REFS_HEADS_RIGHT))
+                                       .call();
+                       assertEquals(MergeStatus.MERGED, mergeResult.getMergeStatus());
+
+                       // Check that the file is the union of both branches (no conflict
+                       // marker added)
+                       String result = read(writeTrashFile("res.cat",
+                                       "A\n" + "G\n" + "E\n" + "C\n" + "F\n"));
+                       assertEquals(result, read(git.getRepository().getWorkTree().toPath()
+                                       .resolve("main.cat").toFile()));
+               }
+       }
+
        @Test
        public void mergeBinaryFile_NoAttr_Conflict() throws IllegalStateException,
                        IOException, NoHeadException, ConcurrentRefUpdateException,
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmUnionTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/merge/MergeAlgorithmUnionTest.java
new file mode 100644 (file)
index 0000000..3a8af7a
--- /dev/null
@@ -0,0 +1,328 @@
+/*
+ * Copyright (C) 2024 Qualcomm Innovation Center, Inc.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * https://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+package org.eclipse.jgit.merge;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertEquals;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+import org.eclipse.jgit.diff.RawText;
+import org.eclipse.jgit.diff.RawTextComparator;
+import org.eclipse.jgit.lib.Constants;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.experimental.theories.DataPoints;
+import org.junit.experimental.theories.Theories;
+import org.junit.runner.RunWith;
+
+@RunWith(Theories.class)
+public class MergeAlgorithmUnionTest {
+       MergeFormatter fmt = new MergeFormatter();
+
+       private final boolean newlineAtEnd;
+
+       @DataPoints
+       public static boolean[] newlineAtEndDataPoints = { false, true };
+
+       public MergeAlgorithmUnionTest(boolean newlineAtEnd) {
+               this.newlineAtEnd = newlineAtEnd;
+       }
+
+       /**
+        * Check for a conflict where the second text was changed similar to the
+        * first one, but the second texts modification covers one more line.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoConflictingModifications() throws IOException {
+               assertEquals(t("abZZdefghij"),
+                               merge("abcdefghij", "abZdefghij", "aZZdefghij"));
+       }
+
+       /**
+        * Test a case where we have three consecutive chunks. The first text
+        * modifies all three chunks. The second text modifies the first and the
+        * last chunk. This should be reported as one conflicting region.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testOneAgainstTwoConflictingModifications() throws IOException {
+               assertEquals(t("aZZcZefghij"),
+                               merge("abcdefghij", "aZZZefghij", "aZcZefghij"));
+       }
+
+       /**
+        * Test a merge where only the second text contains modifications. Expect as
+        * merge result the second text.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testNoAgainstOneModification() throws IOException {
+               assertEquals(t("aZcZefghij"),
+                               merge("abcdefghij", "abcdefghij", "aZcZefghij"));
+       }
+
+       /**
+        * Both texts contain modifications but not on the same chunks. Expect a
+        * non-conflict merge result.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoNonConflictingModifications() throws IOException {
+               assertEquals(t("YbZdefghij"),
+                               merge("abcdefghij", "abZdefghij", "Ybcdefghij"));
+       }
+
+       /**
+        * Merge two complicated modifications. The merge algorithm has to extend
+        * and combine conflicting regions to get to the expected merge result.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoComplicatedModifications() throws IOException {
+               assertEquals(t("aZZZZfZhZjbYdYYYYiY"),
+                               merge("abcdefghij", "aZZZZfZhZj", "abYdYYYYiY"));
+       }
+
+       /**
+        * Merge two modifications with a shared delete at the end. The underlying
+        * diff algorithm has to provide consistent edit results to get the expected
+        * merge result.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoModificationsWithSharedDelete() throws IOException {
+               assertEquals(t("Cb}n}"), merge("ab}n}n}", "ab}n}", "Cb}n}"));
+       }
+
+       /**
+        * Merge modifications with a shared insert in the middle. The underlying
+        * diff algorithm has to provide consistent edit results to get the expected
+        * merge result.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testModificationsWithMiddleInsert() throws IOException {
+               assertEquals(t("aBcd123123uvwxPq"),
+                               merge("abcd123uvwxpq", "aBcd123123uvwxPq", "abcd123123uvwxpq"));
+       }
+
+       /**
+        * Merge modifications with a shared delete in the middle. The underlying
+        * diff algorithm has to provide consistent edit results to get the expected
+        * merge result.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testModificationsWithMiddleDelete() throws IOException {
+               assertEquals(t("Abz}z123Q"),
+                               merge("abz}z}z123q", "Abz}z123Q", "abz}z123q"));
+       }
+
+       @Test
+       public void testInsertionAfterDeletion() throws IOException {
+               assertEquals(t("abcd"), merge("abd", "ad", "abcd"));
+       }
+
+       @Test
+       public void testInsertionBeforeDeletion() throws IOException {
+               assertEquals(t("acbd"), merge("abd", "ad", "acbd"));
+       }
+
+       /**
+        * Test a conflicting region at the very start of the text.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testConflictAtStart() throws IOException {
+               assertEquals(t("ZYbcdefghij"),
+                               merge("abcdefghij", "Zbcdefghij", "Ybcdefghij"));
+       }
+
+       /**
+        * Test a conflicting region at the very end of the text.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testConflictAtEnd() throws IOException {
+               assertEquals(t("abcdefghiZY"),
+                               merge("abcdefghij", "abcdefghiZ", "abcdefghiY"));
+       }
+
+       /**
+        * Check for a conflict where the second text was changed similar to the
+        * first one, but the second texts modification covers one more line.
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testSameModification() throws IOException {
+               assertEquals(t("abZdefghij"),
+                               merge("abcdefghij", "abZdefghij", "abZdefghij"));
+       }
+
+       /**
+        * Check that a deleted vs. a modified line shows up as conflict (see Bug
+        * 328551)
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testDeleteVsModify() throws IOException {
+               assertEquals(t("abZdefghij"),
+                               merge("abcdefghij", "abdefghij", "abZdefghij"));
+       }
+
+       @Test
+       public void testInsertVsModify() throws IOException {
+               assertEquals(t("abZXY"), merge("ab", "abZ", "aXY"));
+       }
+
+       @Test
+       public void testAdjacentModifications() throws IOException {
+               assertEquals(t("aZcbYd"), merge("abcd", "aZcd", "abYd"));
+       }
+
+       @Test
+       public void testSeparateModifications() throws IOException {
+               assertEquals(t("aZcYe"), merge("abcde", "aZcde", "abcYe"));
+       }
+
+       @Test
+       public void testBlankLines() throws IOException {
+               assertEquals(t("aZc\nYe"), merge("abc\nde", "aZc\nde", "abc\nYe"));
+       }
+
+       /**
+        * Test merging two contents which do one similar modification and one
+        * insertion is only done by one side, in the middle. Between modification
+        * and insertion is a block which is common between the two contents and the
+        * common base
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoSimilarModsAndOneInsert() throws IOException {
+               assertEquals(t("aBcDde"), merge("abcde", "aBcde", "aBcDde"));
+
+               assertEquals(t("IAAAJCAB"), merge("iACAB", "IACAB", "IAAAJCAB"));
+
+               assertEquals(t("HIAAAJCAB"), merge("HiACAB", "HIACAB", "HIAAAJCAB"));
+
+               assertEquals(t("AGADEFHIAAAJCAB"),
+                               merge("AGADEFHiACAB", "AGADEFHIACAB", "AGADEFHIAAAJCAB"));
+       }
+
+       /**
+        * Test merging two contents which do one similar modification and one
+        * insertion is only done by one side, at the end. Between modification and
+        * insertion is a block which is common between the two contents and the
+        * common base
+        *
+        * @throws java.io.IOException
+        */
+       @Test
+       public void testTwoSimilarModsAndOneInsertAtEnd() throws IOException {
+               Assume.assumeTrue(newlineAtEnd);
+               assertEquals(t("IAAJ"), merge("iA", "IA", "IAAJ"));
+
+               assertEquals(t("IAJ"), merge("iA", "IA", "IAJ"));
+
+               assertEquals(t("IAAAJ"), merge("iA", "IA", "IAAAJ"));
+       }
+
+       @Test
+       public void testTwoSimilarModsAndOneInsertAtEndNoNewlineAtEnd()
+                       throws IOException {
+               Assume.assumeFalse(newlineAtEnd);
+               assertEquals(t("IAAAJ"), merge("iA", "IA", "IAAJ"));
+
+               assertEquals(t("IAAJ"), merge("iA", "IA", "IAJ"));
+
+               assertEquals(t("IAAAAJ"), merge("iA", "IA", "IAAAJ"));
+       }
+
+       // Test situations where (at least) one input value is the empty text
+
+       @Test
+       public void testEmptyTextModifiedAgainstDeletion() throws IOException {
+               // NOTE: git.git merge-file appends a '\n' to the end of the file even
+               // when the input files do not have a newline at the end. That appears
+               // to be a bug in git.git.
+               assertEquals(t("AB"), merge("A", "AB", ""));
+               assertEquals(t("AB"), merge("A", "", "AB"));
+       }
+
+       @Test
+       public void testEmptyTextUnmodifiedAgainstDeletion() throws IOException {
+               assertEquals(t(""), merge("AB", "AB", ""));
+
+               assertEquals(t(""), merge("AB", "", "AB"));
+       }
+
+       @Test
+       public void testEmptyTextDeletionAgainstDeletion() throws IOException {
+               assertEquals(t(""), merge("AB", "", ""));
+       }
+
+       private String merge(String commonBase, String ours, String theirs)
+                       throws IOException {
+               MergeAlgorithm ma = new MergeAlgorithm();
+               ma.setContentMergeStrategy(ContentMergeStrategy.UNION);
+               MergeResult<RawText> r = ma.merge(RawTextComparator.DEFAULT,
+                               T(commonBase), T(ours), T(theirs));
+               ByteArrayOutputStream bo = new ByteArrayOutputStream(50);
+               fmt.formatMerge(bo, r, "B", "O", "T", UTF_8);
+               return bo.toString(UTF_8);
+       }
+
+       public String t(String text) {
+               StringBuilder r = new StringBuilder();
+               for (int i = 0; i < text.length(); i++) {
+                       char c = text.charAt(i);
+                       switch (c) {
+                       case '<':
+                               r.append("<<<<<<< O\n");
+                               break;
+                       case '=':
+                               r.append("=======\n");
+                               break;
+                       case '|':
+                               r.append("||||||| B\n");
+                               break;
+                       case '>':
+                               r.append(">>>>>>> T\n");
+                               break;
+                       default:
+                               r.append(c);
+                               if (newlineAtEnd || i < text.length() - 1)
+                                       r.append('\n');
+                       }
+               }
+               return r.toString();
+       }
+
+       public RawText T(String text) {
+               return new RawText(Constants.encode(t(text)));
+       }
+}
index 50a04d20f014c6702716c1c522038d73e4628410..28b20ab01f5051a51c4db9c94c9c1e979c9513c1 100644 (file)
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/lib/Constants.java" type="org.eclipse.jgit.lib.Constants">
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="6.10.1"/>
+                <message_argument value="ATTR_BUILTIN_UNION_MERGE_DRIVER"/>
+            </message_arguments>
+        </filter>
+    </resource>
+    <resource path="src/org/eclipse/jgit/merge/ContentMergeStrategy.java" type="org.eclipse.jgit.merge.ContentMergeStrategy">
+        <filter id="1176502275">
+            <message_arguments>
+                <message_argument value="6.10.1"/>
+                <message_argument value="UNION"/>
+            </message_arguments>
+        </filter>
+    </resource>
 </component>
index 60a23dde057052b4388ec56b03ed667ebfb9c370..1a97d111e34e14a8b285932a89b7b61a7c0b65c3 100644 (file)
@@ -493,6 +493,13 @@ public final class Constants {
         */
        public static final String ATTR_BUILTIN_BINARY_MERGER = "binary"; //$NON-NLS-1$
 
+       /**
+        * Union built-in merge driver
+        *
+        * @since 6.10.1
+        */
+       public static final String ATTR_BUILTIN_UNION_MERGE_DRIVER = "union"; //$NON-NLS-1$
+
        /**
         * Create a new digest function for objects.
         *
index 6d568643d5a46c7ef3bf1c2198034b235640607d..a835a1dfc527a2e30d7afcbffa34648f4612f287 100644 (file)
@@ -23,5 +23,12 @@ public enum ContentMergeStrategy {
        OURS,
 
        /** Resolve the conflict hunk using the theirs version. */
-       THEIRS
-}
\ No newline at end of file
+       THEIRS,
+
+       /**
+        * Resolve the conflict hunk using a union of both ours and theirs versions.
+        *
+        * @since 6.10.1
+        */
+       UNION
+}
index 5734a25276bc6d19318852fc737ce2ad38638de8..d0d4d367b91ee6a19a5cd8d01ea6a5745aa9e26a 100644 (file)
@@ -120,6 +120,7 @@ public final class MergeAlgorithm {
                                                result.add(1, 0, 0, ConflictState.NO_CONFLICT);
                                                break;
                                        case THEIRS:
+                                       case UNION:
                                                result.add(2, 0, theirs.size(),
                                                                ConflictState.NO_CONFLICT);
                                                break;
@@ -148,6 +149,7 @@ public final class MergeAlgorithm {
                                // we modified, they deleted
                                switch (strategy) {
                                case OURS:
+                               case UNION:
                                        result.add(1, 0, ours.size(), ConflictState.NO_CONFLICT);
                                        break;
                                case THEIRS:
@@ -158,7 +160,7 @@ public final class MergeAlgorithm {
                                        result.add(1, 0, ours.size(),
                                                        ConflictState.FIRST_CONFLICTING_RANGE);
                                        result.add(0, 0, base.size(),
-                                               ConflictState.BASE_CONFLICTING_RANGE);
+                                                       ConflictState.BASE_CONFLICTING_RANGE);
                                        result.add(2, 0, 0, ConflictState.NEXT_CONFLICTING_RANGE);
                                        break;
                                }
@@ -329,6 +331,15 @@ public final class MergeAlgorithm {
                                                                ConflictState.NO_CONFLICT);
                                                break;
                                        case THEIRS:
+                                               result.add(2, theirsBeginB + commonPrefix,
+                                                               theirsEndB - commonSuffix,
+                                                               ConflictState.NO_CONFLICT);
+                                               break;
+                                       case UNION:
+                                               result.add(1, oursBeginB + commonPrefix,
+                                                               oursEndB - commonSuffix,
+                                                               ConflictState.NO_CONFLICT);
+
                                                result.add(2, theirsBeginB + commonPrefix,
                                                                theirsEndB - commonSuffix,
                                                                ConflictState.NO_CONFLICT);
index 1ad41be42361a4db91adb283be6fc0782fe941fc..85b85cf855cbaebed1cb08bc123e712447978cb9 100644 (file)
@@ -1489,11 +1489,23 @@ public class ResolveMerger extends ThreeWayMerger {
                                : getRawText(ours.getEntryObjectId(), attributes[T_OURS]);
                RawText theirsText = theirs == null ? RawText.EMPTY_TEXT
                                : getRawText(theirs.getEntryObjectId(), attributes[T_THEIRS]);
-               mergeAlgorithm.setContentMergeStrategy(strategy);
+               mergeAlgorithm.setContentMergeStrategy(
+                               getAttributesContentMergeStrategy(attributes[T_OURS],
+                                               strategy));
                return mergeAlgorithm.merge(RawTextComparator.DEFAULT, baseText,
                                ourText, theirsText);
        }
 
+       private ContentMergeStrategy getAttributesContentMergeStrategy(
+                       Attributes attributes, ContentMergeStrategy strategy) {
+               Attribute attr = attributes.get(Constants.ATTR_MERGE);
+               if (attr != null && attr.getValue()
+                               .equals(Constants.ATTR_BUILTIN_UNION_MERGE_DRIVER)) {
+                       return ContentMergeStrategy.UNION;
+               }
+               return strategy;
+       }
+
        private boolean isIndexDirty() {
                if (inCore) {
                        return false;