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/src/org/eclipse/jgit/api | |
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/src/org/eclipse/jgit/api')
-rw-r--r-- | org.eclipse.jgit/src/org/eclipse/jgit/api/BlameCommand.java | 72 |
1 files changed, 2 insertions, 70 deletions
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/BlameCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/BlameCommand.java index 0c765b01cb..a69aa70c6e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/BlameCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/BlameCommand.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011, GitHub Inc. + * Copyright (C) 2011, 2019 GitHub Inc. * and other copyright owners as documented in the project's IP log. * * This program and the accompanying materials are made available @@ -42,11 +42,7 @@ */ package org.eclipse.jgit.api; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -56,17 +52,10 @@ import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.blame.BlameGenerator; import org.eclipse.jgit.blame.BlameResult; import org.eclipse.jgit.diff.DiffAlgorithm; -import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.diff.RawTextComparator; -import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.lib.AnyObjectId; -import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.CoreConfig.AutoCRLF; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.treewalk.WorkingTreeOptions; -import org.eclipse.jgit.util.IO; -import org.eclipse.jgit.util.io.AutoLFInputStream; /** * Blame command for building a {@link org.eclipse.jgit.blame.BlameResult} for a @@ -221,68 +210,11 @@ public class BlameCommand extends GitCommand<BlameResult> { else if (startCommit != null) gen.push(null, startCommit); else { - gen.push(null, repo.resolve(Constants.HEAD)); - if (!repo.isBare()) { - DirCache dc = repo.readDirCache(); - int entry = dc.findEntry(path); - if (0 <= entry) - gen.push(null, dc.getEntry(entry).getObjectId()); - - File inTree = new File(repo.getWorkTree(), path); - if (repo.getFS().isFile(inTree)) { - RawText rawText = getRawText(inTree); - gen.push(null, rawText); - } - } + gen.prepareHead(); } return gen.computeBlameResult(); } catch (IOException e) { throw new JGitInternalException(e.getMessage(), e); } } - - private RawText getRawText(File inTree) throws IOException, - FileNotFoundException { - RawText rawText; - - WorkingTreeOptions workingTreeOptions = getRepository().getConfig() - .get(WorkingTreeOptions.KEY); - AutoCRLF autoCRLF = workingTreeOptions.getAutoCRLF(); - switch (autoCRLF) { - case FALSE: - case INPUT: - // Git used the repo format on checkout, but other tools - // may change the format to CRLF. We ignore that here. - rawText = new RawText(inTree); - break; - case TRUE: - try (AutoLFInputStream in = new AutoLFInputStream( - new FileInputStream(inTree), true)) { - // Canonicalization should lead to same or shorter length - // (CRLF to LF), so the file size on disk is an upper size bound - rawText = new RawText(toByteArray(in, (int) inTree.length())); - } - break; - default: - throw new IllegalArgumentException( - "Unknown autocrlf option " + autoCRLF); //$NON-NLS-1$ - } - return rawText; - } - - private static byte[] toByteArray(InputStream source, int upperSizeLimit) - throws IOException { - byte[] buffer = new byte[upperSizeLimit]; - try { - int read = IO.readFully(source, buffer, 0); - if (read == upperSizeLimit) { - return buffer; - } - byte[] copy = new byte[read]; - System.arraycopy(buffer, 0, copy, 0, read); - return copy; - } finally { - source.close(); - } - } } |