From 59f9d206c954b5633f5978723bd0a2e7db31c2e8 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 6 Aug 2019 18:31:35 +0200 Subject: 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 --- .../src/org/eclipse/jgit/pgm/Blame.java | 28 +++------------------- 1 file changed, 3 insertions(+), 25 deletions(-) (limited to 'org.eclipse.jgit.pgm/src/org/eclipse') diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Blame.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Blame.java index b67b04c5be..f85d8d49a2 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Blame.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Blame.java @@ -50,7 +50,6 @@ import static java.lang.Integer.valueOf; import static java.lang.Long.valueOf; import static org.eclipse.jgit.lib.Constants.OBJECT_ID_STRING_LENGTH; -import java.io.File; import java.io.IOException; import java.text.MessageFormat; import java.text.SimpleDateFormat; @@ -60,11 +59,11 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; +import org.eclipse.jgit.api.errors.NoHeadException; import org.eclipse.jgit.blame.BlameGenerator; import org.eclipse.jgit.blame.BlameResult; import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.diff.RawTextComparator; -import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.errors.NoWorkTreeException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; @@ -186,28 +185,7 @@ class Blame extends TextBuiltin { } generator.push(null, rev); } else { - ObjectId head = db.resolve(Constants.HEAD); - if (head == null) { - throw die(MessageFormat.format(CLIText.get().noSuchRef, - Constants.HEAD)); - } - generator.push(null, head); - if (!db.isBare()) { - DirCache dc = db.readDirCache(); - int entry = dc.findEntry(file); - if (0 <= entry) { - generator.push(null, dc.getEntry(entry).getObjectId()); - } else { - throw die(MessageFormat.format( - CLIText.get().noSuchPathInRef, file, - Constants.HEAD)); - } - - File inTree = new File(db.getWorkTree(), file); - if (db.getFS().isFile(inTree)) { - generator.push(null, new RawText(inTree)); - } - } + generator.prepareHead(); } blame = BlameResult.create(generator); @@ -280,7 +258,7 @@ class Blame extends TextBuiltin { } while (++line < end && sameCommit(blame.getSourceCommit(line), c)); } - } catch (NoWorkTreeException | IOException e) { + } catch (NoWorkTreeException | NoHeadException | IOException e) { throw die(e.getMessage(), e); } } -- cgit v1.2.3