Abort rename detection in a timely manner if cancelled

If progress monitor is cancelled break loops in rename detection by
throwing a CanceledException.

Bug: 536324
Change-Id: Ia3511fb749d2a5d45005e72c156b874ab7a0da26
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
This commit is contained in:
Matthias Sohn 2018-07-21 21:50:30 +02:00
parent 26c5d0e56a
commit da254106a7
6 changed files with 80 additions and 25 deletions

View File

@ -579,6 +579,7 @@ remoteNameCantBeNull=Remote name can't be null.
renameBranchFailedBecauseTag=Can not rename as Ref {0} is a tag renameBranchFailedBecauseTag=Can not rename as Ref {0} is a tag
renameBranchFailedUnknownReason=Rename failed with unknown reason renameBranchFailedUnknownReason=Rename failed with unknown reason
renameBranchUnexpectedResult=Unexpected rename result {0} renameBranchUnexpectedResult=Unexpected rename result {0}
renameCancelled=Rename detection was cancelled
renameFileFailed=Could not rename file {0} to {1} renameFileFailed=Could not rename file {0} to {1}
renamesAlreadyFound=Renames have already been found. renamesAlreadyFound=Renames have already been found.
renamesBreakingModifies=Breaking apart modified file pairs renamesBreakingModifies=Breaking apart modified file pairs

View File

@ -52,6 +52,7 @@
import java.util.Collections; import java.util.Collections;
import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.api.errors.CanceledException;
import org.eclipse.jgit.blame.Candidate.BlobCandidate; import org.eclipse.jgit.blame.Candidate.BlobCandidate;
import org.eclipse.jgit.blame.Candidate.ReverseCandidate; import org.eclipse.jgit.blame.Candidate.ReverseCandidate;
import org.eclipse.jgit.blame.ReverseWalk.ReverseCommit; import org.eclipse.jgit.blame.ReverseWalk.ReverseCommit;
@ -627,9 +628,15 @@ private boolean processOne(Candidate n) throws IOException {
if (n.sourceCommit == null) if (n.sourceCommit == null)
return result(n); return result(n);
DiffEntry r = findRename(parent, n.sourceCommit, n.sourcePath); DiffEntry r;
if (r == null) try {
r = findRename(parent, n.sourceCommit, n.sourcePath);
if (r == null) {
return result(n);
}
} catch (CanceledException e) {
return result(n); return result(n);
}
if (0 == r.getOldId().prefixCompare(n.sourceBlob)) { if (0 == r.getOldId().prefixCompare(n.sourceBlob)) {
// A 100% rename without any content change can also // A 100% rename without any content change can also
@ -687,7 +694,8 @@ private boolean split(Candidate parent, Candidate source)
return false; return false;
} }
private boolean processMerge(Candidate n) throws IOException { private boolean processMerge(Candidate n)
throws IOException {
int pCnt = n.getParentCount(); int pCnt = n.getParentCount();
// If any single parent exactly matches the merge, follow only // If any single parent exactly matches the merge, follow only
@ -714,9 +722,15 @@ private boolean processMerge(Candidate n) throws IOException {
if (ids != null && ids[pIdx] != null) if (ids != null && ids[pIdx] != null)
continue; continue;
DiffEntry r = findRename(parent, n.sourceCommit, n.sourcePath); DiffEntry r;
if (r == null) try {
r = findRename(parent, n.sourceCommit, n.sourcePath);
if (r == null) {
continue;
}
} catch (CanceledException e) {
continue; continue;
}
if (n instanceof ReverseCandidate) { if (n instanceof ReverseCandidate) {
if (ids == null) if (ids == null)
@ -1021,7 +1035,7 @@ private static final boolean isFile(int rawMode) {
} }
private DiffEntry findRename(RevCommit parent, RevCommit commit, private DiffEntry findRename(RevCommit parent, RevCommit commit,
PathFilter path) throws IOException { PathFilter path) throws IOException, CanceledException {
if (renameDetector == null) if (renameDetector == null)
return null; return null;

View File

@ -62,6 +62,7 @@
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import org.eclipse.jgit.api.errors.CanceledException;
import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm; import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm;
import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.dircache.DirCacheIterator; import org.eclipse.jgit.dircache.DirCacheIterator;
@ -577,7 +578,11 @@ private List<DiffEntry> detectRenames(List<DiffEntry> files)
throws IOException { throws IOException {
renameDetector.reset(); renameDetector.reset();
renameDetector.addAll(files); renameDetector.addAll(files);
return renameDetector.compute(reader, progressMonitor); try {
return renameDetector.compute(reader, progressMonitor);
} catch (CanceledException e) {
return Collections.emptyList();
}
} }
private boolean isAdd(List<DiffEntry> files) { private boolean isAdd(List<DiffEntry> files) {

View File

@ -55,6 +55,7 @@
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import org.eclipse.jgit.api.errors.CanceledException;
import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.diff.SimilarityIndex.TableFullException; import org.eclipse.jgit.diff.SimilarityIndex.TableFullException;
import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.JGitText;
@ -320,7 +321,11 @@ public void add(DiffEntry entry) {
* file contents cannot be read from the repository. * file contents cannot be read from the repository.
*/ */
public List<DiffEntry> compute() throws IOException { public List<DiffEntry> compute() throws IOException {
return compute(NullProgressMonitor.INSTANCE); try {
return compute(NullProgressMonitor.INSTANCE);
} catch (CanceledException e) {
return Collections.emptyList();
}
} }
/** /**
@ -332,8 +337,11 @@ public List<DiffEntry> compute() throws IOException {
* representing all files that have been changed. * representing all files that have been changed.
* @throws java.io.IOException * @throws java.io.IOException
* file contents cannot be read from the repository. * file contents cannot be read from the repository.
* @throws CanceledException
* if rename detection was cancelled
*/ */
public List<DiffEntry> compute(ProgressMonitor pm) throws IOException { public List<DiffEntry> compute(ProgressMonitor pm)
throws IOException, CanceledException {
if (!done) { if (!done) {
try { try {
return compute(objectReader, pm); return compute(objectReader, pm);
@ -355,9 +363,11 @@ public List<DiffEntry> compute(ProgressMonitor pm) throws IOException {
* representing all files that have been changed. * representing all files that have been changed.
* @throws java.io.IOException * @throws java.io.IOException
* file contents cannot be read from the repository. * file contents cannot be read from the repository.
* @throws CanceledException
* if rename detection was cancelled
*/ */
public List<DiffEntry> compute(ObjectReader reader, ProgressMonitor pm) public List<DiffEntry> compute(ObjectReader reader, ProgressMonitor pm)
throws IOException { throws IOException, CanceledException {
final ContentSource cs = ContentSource.create(reader); final ContentSource cs = ContentSource.create(reader);
return compute(new ContentSource.Pair(cs, cs), pm); return compute(new ContentSource.Pair(cs, cs), pm);
} }
@ -373,9 +383,11 @@ public List<DiffEntry> compute(ObjectReader reader, ProgressMonitor pm)
* representing all files that have been changed. * representing all files that have been changed.
* @throws java.io.IOException * @throws java.io.IOException
* file contents cannot be read from the repository. * file contents cannot be read from the repository.
* @throws CanceledException
* if rename detection was cancelled
*/ */
public List<DiffEntry> compute(ContentSource.Pair reader, ProgressMonitor pm) public List<DiffEntry> compute(ContentSource.Pair reader, ProgressMonitor pm)
throws IOException { throws IOException, CanceledException {
if (!done) { if (!done) {
done = true; done = true;
@ -415,8 +427,15 @@ public void reset() {
done = false; done = false;
} }
private void advanceOrCancel(ProgressMonitor pm) throws CanceledException {
if (pm.isCancelled()) {
throw new CanceledException(JGitText.get().renameCancelled);
}
pm.update(1);
}
private void breakModifies(ContentSource.Pair reader, ProgressMonitor pm) private void breakModifies(ContentSource.Pair reader, ProgressMonitor pm)
throws IOException { throws IOException, CanceledException {
ArrayList<DiffEntry> newEntries = new ArrayList<>(entries.size()); ArrayList<DiffEntry> newEntries = new ArrayList<>(entries.size());
pm.beginTask(JGitText.get().renamesBreakingModifies, entries.size()); pm.beginTask(JGitText.get().renamesBreakingModifies, entries.size());
@ -437,13 +456,13 @@ private void breakModifies(ContentSource.Pair reader, ProgressMonitor pm)
} else { } else {
newEntries.add(e); newEntries.add(e);
} }
pm.update(1); advanceOrCancel(pm);
} }
entries = newEntries; entries = newEntries;
} }
private void rejoinModifies(ProgressMonitor pm) { private void rejoinModifies(ProgressMonitor pm) throws CanceledException {
HashMap<String, DiffEntry> nameMap = new HashMap<>(); HashMap<String, DiffEntry> nameMap = new HashMap<>();
ArrayList<DiffEntry> newAdded = new ArrayList<>(added.size()); ArrayList<DiffEntry> newAdded = new ArrayList<>(added.size());
@ -452,7 +471,7 @@ private void rejoinModifies(ProgressMonitor pm) {
for (DiffEntry src : deleted) { for (DiffEntry src : deleted) {
nameMap.put(src.oldPath, src); nameMap.put(src.oldPath, src);
pm.update(1); advanceOrCancel(pm);
} }
for (DiffEntry dst : added) { for (DiffEntry dst : added) {
@ -468,7 +487,7 @@ private void rejoinModifies(ProgressMonitor pm) {
} else { } else {
newAdded.add(dst); newAdded.add(dst);
} }
pm.update(1); advanceOrCancel(pm);
} }
added = newAdded; added = newAdded;
@ -498,7 +517,7 @@ private int calculateModifyScore(ContentSource.Pair reader, DiffEntry d)
private void findContentRenames(ContentSource.Pair reader, private void findContentRenames(ContentSource.Pair reader,
ProgressMonitor pm) ProgressMonitor pm)
throws IOException { throws IOException, CanceledException {
int cnt = Math.max(added.size(), deleted.size()); int cnt = Math.max(added.size(), deleted.size());
if (getRenameLimit() == 0 || cnt <= getRenameLimit()) { if (getRenameLimit() == 0 || cnt <= getRenameLimit()) {
SimilarityRenameDetector d; SimilarityRenameDetector d;
@ -516,7 +535,7 @@ private void findContentRenames(ContentSource.Pair reader,
} }
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
private void findExactRenames(ProgressMonitor pm) { private void findExactRenames(ProgressMonitor pm) throws CanceledException {
pm.beginTask(JGitText.get().renamesFindingExact, // pm.beginTask(JGitText.get().renamesFindingExact, //
added.size() + added.size() + deleted.size() added.size() + added.size() + deleted.size()
+ added.size() * deleted.size()); + added.size() * deleted.size());
@ -562,7 +581,7 @@ private void findExactRenames(ProgressMonitor pm) {
} else { } else {
left.add(a); left.add(a);
} }
pm.update(1); advanceOrCancel(pm);
} }
for (List<DiffEntry> adds : nonUniqueAdds) { for (List<DiffEntry> adds : nonUniqueAdds) {
@ -604,6 +623,10 @@ private void findExactRenames(ProgressMonitor pm) {
int score = SimilarityRenameDetector.nameScore(addedName, deletedName); int score = SimilarityRenameDetector.nameScore(addedName, deletedName);
matrix[mNext] = SimilarityRenameDetector.encode(score, delIdx, addIdx); matrix[mNext] = SimilarityRenameDetector.encode(score, delIdx, addIdx);
mNext++; mNext++;
if (pm.isCancelled()) {
throw new CanceledException(
JGitText.get().renameCancelled);
}
} }
} }
@ -617,7 +640,7 @@ private void findExactRenames(ProgressMonitor pm) {
DiffEntry a = adds.get(addIdx); DiffEntry a = adds.get(addIdx);
if (a == null) { if (a == null) {
pm.update(1); advanceOrCancel(pm);
continue; // was already matched earlier continue; // was already matched earlier
} }
@ -635,11 +658,12 @@ private void findExactRenames(ProgressMonitor pm) {
entries.add(DiffEntry.pair(type, d, a, 100)); entries.add(DiffEntry.pair(type, d, a, 100));
adds.set(addIdx, null); // Claim the destination was matched. adds.set(addIdx, null); // Claim the destination was matched.
pm.update(1); advanceOrCancel(pm);
} }
} else { } else {
left.addAll(adds); left.addAll(adds);
} }
advanceOrCancel(pm);
} }
added = left; added = left;
@ -692,7 +716,8 @@ private static DiffEntry bestPathMatch(DiffEntry src, List<DiffEntry> list) {
@SuppressWarnings("unchecked") @SuppressWarnings("unchecked")
private HashMap<AbbreviatedObjectId, Object> populateMap( private HashMap<AbbreviatedObjectId, Object> populateMap(
List<DiffEntry> diffEntries, ProgressMonitor pm) { List<DiffEntry> diffEntries, ProgressMonitor pm)
throws CanceledException {
HashMap<AbbreviatedObjectId, Object> map = new HashMap<>(); HashMap<AbbreviatedObjectId, Object> map = new HashMap<>();
for (DiffEntry de : diffEntries) { for (DiffEntry de : diffEntries) {
Object old = map.put(id(de), de); Object old = map.put(id(de), de);
@ -706,7 +731,7 @@ private HashMap<AbbreviatedObjectId, Object> populateMap(
((List<DiffEntry>) old).add(de); ((List<DiffEntry>) old).add(de);
map.put(id(de), old); map.put(id(de), old);
} }
pm.update(1); advanceOrCancel(pm);
} }
return map; return map;
} }

View File

@ -52,6 +52,7 @@
import java.util.BitSet; import java.util.BitSet;
import java.util.List; import java.util.List;
import org.eclipse.jgit.api.errors.CanceledException;
import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.DiffEntry.ChangeType;
import org.eclipse.jgit.diff.SimilarityIndex.TableFullException; import org.eclipse.jgit.diff.SimilarityIndex.TableFullException;
import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.JGitText;
@ -128,7 +129,7 @@ void setRenameScore(int score) {
renameScore = score; renameScore = score;
} }
void compute(ProgressMonitor pm) throws IOException { void compute(ProgressMonitor pm) throws IOException, CanceledException {
if (pm == null) if (pm == null)
pm = NullProgressMonitor.INSTANCE; pm = NullProgressMonitor.INSTANCE;
@ -142,6 +143,9 @@ void compute(ProgressMonitor pm) throws IOException {
// we have looked at everything that is above our minimum score. // we have looked at everything that is above our minimum score.
// //
for (--mNext; mNext >= 0; mNext--) { for (--mNext; mNext >= 0; mNext--) {
if (pm.isCancelled()) {
throw new CanceledException(JGitText.get().renameCancelled);
}
long ent = matrix[mNext]; long ent = matrix[mNext];
int sIdx = srcFile(ent); int sIdx = srcFile(ent);
int dIdx = dstFile(ent); int dIdx = dstFile(ent);
@ -209,7 +213,8 @@ private static List<DiffEntry> compactDstList(List<DiffEntry> in) {
return r; return r;
} }
private int buildMatrix(ProgressMonitor pm) throws IOException { private int buildMatrix(ProgressMonitor pm)
throws IOException, CanceledException {
// Allocate for the worst-case scenario where every pair has a // Allocate for the worst-case scenario where every pair has a
// score that we need to consider. We might not need that many. // score that we need to consider. We might not need that many.
// //
@ -234,6 +239,10 @@ private int buildMatrix(ProgressMonitor pm) throws IOException {
SimilarityIndex s = null; SimilarityIndex s = null;
for (int dstIdx = 0; dstIdx < dsts.size(); dstIdx++) { for (int dstIdx = 0; dstIdx < dsts.size(); dstIdx++) {
if (pm.isCancelled()) {
throw new CanceledException(JGitText.get().renameCancelled);
}
DiffEntry dstEnt = dsts.get(dstIdx); DiffEntry dstEnt = dsts.get(dstIdx);
if (!isFile(dstEnt.newMode)) { if (!isFile(dstEnt.newMode)) {

View File

@ -640,6 +640,7 @@ public static JGitText get() {
/***/ public String renameBranchFailedBecauseTag; /***/ public String renameBranchFailedBecauseTag;
/***/ public String renameBranchFailedUnknownReason; /***/ public String renameBranchFailedUnknownReason;
/***/ public String renameBranchUnexpectedResult; /***/ public String renameBranchUnexpectedResult;
/***/ public String renameCancelled;
/***/ public String renameFileFailed; /***/ public String renameFileFailed;
/***/ public String renamesAlreadyFound; /***/ public String renamesAlreadyFound;
/***/ public String renamesBreakingModifies; /***/ public String renamesBreakingModifies;