From da254106a737ed719e84b5a5e43d5e53149b8937 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 21 Jul 2018 21:50:30 +0200 Subject: [PATCH] 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 --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../eclipse/jgit/blame/BlameGenerator.java | 26 +++++++-- .../org/eclipse/jgit/diff/DiffFormatter.java | 7 ++- .../org/eclipse/jgit/diff/RenameDetector.java | 57 +++++++++++++------ .../jgit/diff/SimilarityRenameDetector.java | 13 ++++- .../org/eclipse/jgit/internal/JGitText.java | 1 + 6 files changed, 80 insertions(+), 25 deletions(-) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 826be11d5..2150d5ce3 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -579,6 +579,7 @@ remoteNameCantBeNull=Remote name can't be null. renameBranchFailedBecauseTag=Can not rename as Ref {0} is a tag renameBranchFailedUnknownReason=Rename failed with unknown reason renameBranchUnexpectedResult=Unexpected rename result {0} +renameCancelled=Rename detection was cancelled renameFileFailed=Could not rename file {0} to {1} renamesAlreadyFound=Renames have already been found. renamesBreakingModifies=Breaking apart modified file pairs diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java index 9cec64567..823494166 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/blame/BlameGenerator.java @@ -52,6 +52,7 @@ import java.util.Collections; 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.ReverseCandidate; import org.eclipse.jgit.blame.ReverseWalk.ReverseCommit; @@ -627,9 +628,15 @@ private boolean processOne(Candidate n) throws IOException { if (n.sourceCommit == null) return result(n); - DiffEntry r = findRename(parent, n.sourceCommit, n.sourcePath); - if (r == null) + DiffEntry r; + try { + r = findRename(parent, n.sourceCommit, n.sourcePath); + if (r == null) { + return result(n); + } + } catch (CanceledException e) { return result(n); + } if (0 == r.getOldId().prefixCompare(n.sourceBlob)) { // A 100% rename without any content change can also @@ -687,7 +694,8 @@ private boolean split(Candidate parent, Candidate source) return false; } - private boolean processMerge(Candidate n) throws IOException { + private boolean processMerge(Candidate n) + throws IOException { int pCnt = n.getParentCount(); // 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) continue; - DiffEntry r = findRename(parent, n.sourceCommit, n.sourcePath); - if (r == null) + DiffEntry r; + try { + r = findRename(parent, n.sourceCommit, n.sourcePath); + if (r == null) { + continue; + } + } catch (CanceledException e) { continue; + } if (n instanceof ReverseCandidate) { if (ids == null) @@ -1021,7 +1035,7 @@ private static final boolean isFile(int rawMode) { } private DiffEntry findRename(RevCommit parent, RevCommit commit, - PathFilter path) throws IOException { + PathFilter path) throws IOException, CanceledException { if (renameDetector == null) return null; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java index 7aaa50030..b70ff4b2b 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/DiffFormatter.java @@ -62,6 +62,7 @@ import java.util.Collections; import java.util.List; +import org.eclipse.jgit.api.errors.CanceledException; import org.eclipse.jgit.diff.DiffAlgorithm.SupportedAlgorithm; import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.dircache.DirCacheIterator; @@ -577,7 +578,11 @@ private List detectRenames(List files) throws IOException { renameDetector.reset(); renameDetector.addAll(files); - return renameDetector.compute(reader, progressMonitor); + try { + return renameDetector.compute(reader, progressMonitor); + } catch (CanceledException e) { + return Collections.emptyList(); + } } private boolean isAdd(List files) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java index 7bb217d04..8336cf249 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/RenameDetector.java @@ -55,6 +55,7 @@ import java.util.HashMap; import java.util.List; +import org.eclipse.jgit.api.errors.CanceledException; import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.SimilarityIndex.TableFullException; import org.eclipse.jgit.internal.JGitText; @@ -320,7 +321,11 @@ public void add(DiffEntry entry) { * file contents cannot be read from the repository. */ public List compute() throws IOException { - return compute(NullProgressMonitor.INSTANCE); + try { + return compute(NullProgressMonitor.INSTANCE); + } catch (CanceledException e) { + return Collections.emptyList(); + } } /** @@ -332,8 +337,11 @@ public List compute() throws IOException { * representing all files that have been changed. * @throws java.io.IOException * file contents cannot be read from the repository. + * @throws CanceledException + * if rename detection was cancelled */ - public List compute(ProgressMonitor pm) throws IOException { + public List compute(ProgressMonitor pm) + throws IOException, CanceledException { if (!done) { try { return compute(objectReader, pm); @@ -355,9 +363,11 @@ public List compute(ProgressMonitor pm) throws IOException { * representing all files that have been changed. * @throws java.io.IOException * file contents cannot be read from the repository. + * @throws CanceledException + * if rename detection was cancelled */ public List compute(ObjectReader reader, ProgressMonitor pm) - throws IOException { + throws IOException, CanceledException { final ContentSource cs = ContentSource.create(reader); return compute(new ContentSource.Pair(cs, cs), pm); } @@ -373,9 +383,11 @@ public List compute(ObjectReader reader, ProgressMonitor pm) * representing all files that have been changed. * @throws java.io.IOException * file contents cannot be read from the repository. + * @throws CanceledException + * if rename detection was cancelled */ public List compute(ContentSource.Pair reader, ProgressMonitor pm) - throws IOException { + throws IOException, CanceledException { if (!done) { done = true; @@ -415,8 +427,15 @@ public void reset() { 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) - throws IOException { + throws IOException, CanceledException { ArrayList newEntries = new ArrayList<>(entries.size()); pm.beginTask(JGitText.get().renamesBreakingModifies, entries.size()); @@ -437,13 +456,13 @@ private void breakModifies(ContentSource.Pair reader, ProgressMonitor pm) } else { newEntries.add(e); } - pm.update(1); + advanceOrCancel(pm); } entries = newEntries; } - private void rejoinModifies(ProgressMonitor pm) { + private void rejoinModifies(ProgressMonitor pm) throws CanceledException { HashMap nameMap = new HashMap<>(); ArrayList newAdded = new ArrayList<>(added.size()); @@ -452,7 +471,7 @@ private void rejoinModifies(ProgressMonitor pm) { for (DiffEntry src : deleted) { nameMap.put(src.oldPath, src); - pm.update(1); + advanceOrCancel(pm); } for (DiffEntry dst : added) { @@ -468,7 +487,7 @@ private void rejoinModifies(ProgressMonitor pm) { } else { newAdded.add(dst); } - pm.update(1); + advanceOrCancel(pm); } added = newAdded; @@ -498,7 +517,7 @@ private int calculateModifyScore(ContentSource.Pair reader, DiffEntry d) private void findContentRenames(ContentSource.Pair reader, ProgressMonitor pm) - throws IOException { + throws IOException, CanceledException { int cnt = Math.max(added.size(), deleted.size()); if (getRenameLimit() == 0 || cnt <= getRenameLimit()) { SimilarityRenameDetector d; @@ -516,7 +535,7 @@ private void findContentRenames(ContentSource.Pair reader, } @SuppressWarnings("unchecked") - private void findExactRenames(ProgressMonitor pm) { + private void findExactRenames(ProgressMonitor pm) throws CanceledException { pm.beginTask(JGitText.get().renamesFindingExact, // added.size() + added.size() + deleted.size() + added.size() * deleted.size()); @@ -562,7 +581,7 @@ private void findExactRenames(ProgressMonitor pm) { } else { left.add(a); } - pm.update(1); + advanceOrCancel(pm); } for (List adds : nonUniqueAdds) { @@ -604,6 +623,10 @@ private void findExactRenames(ProgressMonitor pm) { int score = SimilarityRenameDetector.nameScore(addedName, deletedName); matrix[mNext] = SimilarityRenameDetector.encode(score, delIdx, addIdx); mNext++; + if (pm.isCancelled()) { + throw new CanceledException( + JGitText.get().renameCancelled); + } } } @@ -617,7 +640,7 @@ private void findExactRenames(ProgressMonitor pm) { DiffEntry a = adds.get(addIdx); if (a == null) { - pm.update(1); + advanceOrCancel(pm); continue; // was already matched earlier } @@ -635,11 +658,12 @@ private void findExactRenames(ProgressMonitor pm) { entries.add(DiffEntry.pair(type, d, a, 100)); adds.set(addIdx, null); // Claim the destination was matched. - pm.update(1); + advanceOrCancel(pm); } } else { left.addAll(adds); } + advanceOrCancel(pm); } added = left; @@ -692,7 +716,8 @@ private static DiffEntry bestPathMatch(DiffEntry src, List list) { @SuppressWarnings("unchecked") private HashMap populateMap( - List diffEntries, ProgressMonitor pm) { + List diffEntries, ProgressMonitor pm) + throws CanceledException { HashMap map = new HashMap<>(); for (DiffEntry de : diffEntries) { Object old = map.put(id(de), de); @@ -706,7 +731,7 @@ private HashMap populateMap( ((List) old).add(de); map.put(id(de), old); } - pm.update(1); + advanceOrCancel(pm); } return map; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java index 653658be3..71e391c73 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/diff/SimilarityRenameDetector.java @@ -52,6 +52,7 @@ import java.util.BitSet; import java.util.List; +import org.eclipse.jgit.api.errors.CanceledException; import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.diff.SimilarityIndex.TableFullException; import org.eclipse.jgit.internal.JGitText; @@ -128,7 +129,7 @@ void setRenameScore(int score) { renameScore = score; } - void compute(ProgressMonitor pm) throws IOException { + void compute(ProgressMonitor pm) throws IOException, CanceledException { if (pm == null) pm = NullProgressMonitor.INSTANCE; @@ -142,6 +143,9 @@ void compute(ProgressMonitor pm) throws IOException { // we have looked at everything that is above our minimum score. // for (--mNext; mNext >= 0; mNext--) { + if (pm.isCancelled()) { + throw new CanceledException(JGitText.get().renameCancelled); + } long ent = matrix[mNext]; int sIdx = srcFile(ent); int dIdx = dstFile(ent); @@ -209,7 +213,8 @@ private static List compactDstList(List in) { 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 // 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; for (int dstIdx = 0; dstIdx < dsts.size(); dstIdx++) { + if (pm.isCancelled()) { + throw new CanceledException(JGitText.get().renameCancelled); + } + DiffEntry dstEnt = dsts.get(dstIdx); if (!isFile(dstEnt.newMode)) { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index a3cbc22e8..2373b97bd 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -640,6 +640,7 @@ public static JGitText get() { /***/ public String renameBranchFailedBecauseTag; /***/ public String renameBranchFailedUnknownReason; /***/ public String renameBranchUnexpectedResult; + /***/ public String renameCancelled; /***/ public String renameFileFailed; /***/ public String renamesAlreadyFound; /***/ public String renamesBreakingModifies;