diff options
author | Thomas Wolf <thomas.wolf@paranor.ch> | 2019-08-06 18:31:35 +0200 |
---|---|---|
committer | Matthias Sohn <matthias.sohn@sap.com> | 2019-11-27 03:03:06 +0100 |
commit | 59f9d206c954b5633f5978723bd0a2e7db31c2e8 (patch) | |
tree | eee0571db82d18ebbeb7b9f396c04a174277720e /org.eclipse.jgit.pgm.test/tst/org/eclipse | |
parent | e9d728ceb7365b6cf0b1472021fdbb76d59a02d1 (diff) | |
download | jgit-59f9d206c954b5633f5978723bd0a2e7db31c2e8.tar.gz jgit-59f9d206c954b5633f5978723bd0a2e7db31c2e8.zip |
Make blame work correctly on merge conflicts
When a conflicting file was blamed, JGit would not identify lines
coming from the merge parents. The main cause for this was that
Blame and BlameCommand simply added the first DirCacheEntry found
for a file to its queue of candidates (blobs or commits) to consider.
In case of a conflict this typically is the merge base commit, and
comparing a auto-merged contents against that base would yield
incorrect results.
Such cases have to be handled specially. The candidate to be
considered by the blame must use the working tree contents, but
at the same time behave like a merge commit/candidate with HEAD
and the MERGE_HEADs as parents. Canonical git does something very
similar, see [1].
Implement that and add tests.
I first did this for the JGit pgm Blame command. When I then tried
to do the same in BlameCommand, I noticed that the latter also
included some fancy but incomplete CR-LF handling. In order to
be able to use the new BlameGenerator.prepareHead() also in
BlameCommand this CR-LF handling was also moved into BlameGenerator
and corrected in doing so.
(Just considering the git config settings was not good enough,
CR-LF behavior can also be influenced by .gitattributes, and even
by whether the file in the index has CR-LF. To correctly determine
CR-LF handling for check-in one needs to do a TreeWalk with at
least a FileTreeIterator and a DirCacheIterator.)
[1] https://github.com/git/git/blob/v2.22.0/blame.c#L174
Bug: 434330
Change-Id: I9d763dd6ba478b0b6ebf9456049d6301f478ef7c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Diffstat (limited to 'org.eclipse.jgit.pgm.test/tst/org/eclipse')
-rw-r--r-- | org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BlameTest.java | 35 |
1 files changed, 35 insertions, 0 deletions
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BlameTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BlameTest.java index e806872c14..732c54e332 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BlameTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BlameTest.java @@ -42,8 +42,13 @@ */ package org.eclipse.jgit.pgm; +import static org.junit.Assert.assertTrue; + import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.lib.CLIRepositoryTestCase; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -119,4 +124,34 @@ public class BlameTest extends CLIRepositoryTestCase { thrown.expectMessage("no such path 'sub/does_not_exist.txt' in HEAD"); execute("git blame sub/does_not_exist.txt"); } + + @Test + public void testBlameMergeConflict() throws Exception { + try (Git git = new Git(db)) { + writeTrashFile("file", "Origin\n"); + git.add().addFilepattern("file").call(); + git.commit().setMessage("initial commit").call(); + git.checkout().setCreateBranch(true) + .setName("side").call(); + writeTrashFile("file", + "Conflicting change from side branch\n"); + git.add().addFilepattern("file").call(); + RevCommit side = git.commit().setMessage("side commit").call(); + git.checkout().setName(Constants.MASTER).call(); + writeTrashFile("file", "Change on master branch\n"); + git.add().addFilepattern("file").call(); + git.commit().setMessage("Commit conflict on master").call(); + MergeResult result = git.merge() + .include("side", side).call(); + assertTrue("Expected conflict on 'file'", + result.getConflicts().containsKey("file")); + } + String[] expected = { + " (Not Committed Yet 1) <<<<<<< HEAD", + "7a918de5 (GIT_COMMITTER_NAME 2009-08-15 20:12:58 -0330 2) Change on master branch", + " (Not Committed Yet 3) =======", + "beb52f68 (GIT_COMMITTER_NAME 2009-08-15 20:12:58 -0330 4) Conflicting change from side branch", + " (Not Committed Yet 5) >>>>>>> side" }; + assertArrayOfLinesEquals(expected, execute("git blame file")); + } } |