Require a DiffConfig when creating a FollowFilter
The various rename detection options are an inherent part of the filter, similar to the path being followed. This fixes a potential NPE when a RevWalk with a FollowFilter is created without a Repository, since the old code path tried to get the DiffConfig from the RevWalk's possibly-missing repository. Change-Id: Idb273d5a92849b42935ac14eed73b796b80aad50
This commit is contained in:
parent
b8e763fc19
commit
0bdf030b26
|
@ -50,6 +50,7 @@
|
||||||
|
|
||||||
import org.kohsuke.args4j.Argument;
|
import org.kohsuke.args4j.Argument;
|
||||||
import org.kohsuke.args4j.Option;
|
import org.kohsuke.args4j.Option;
|
||||||
|
import org.eclipse.jgit.diff.DiffConfig;
|
||||||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||||
import org.eclipse.jgit.lib.Constants;
|
import org.eclipse.jgit.lib.Constants;
|
||||||
import org.eclipse.jgit.lib.ObjectId;
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
|
@ -117,9 +118,7 @@ void enableBoundary(final boolean on) {
|
||||||
}
|
}
|
||||||
|
|
||||||
@Option(name = "--follow", metaVar = "metaVar_path")
|
@Option(name = "--follow", metaVar = "metaVar_path")
|
||||||
void follow(final String path) {
|
private String followPath;
|
||||||
pathFilter = FollowFilter.create(path);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Argument(index = 0, metaVar = "metaVar_commitish")
|
@Argument(index = 0, metaVar = "metaVar_commitish")
|
||||||
private final List<RevCommit> commits = new ArrayList<RevCommit>();
|
private final List<RevCommit> commits = new ArrayList<RevCommit>();
|
||||||
|
@ -150,11 +149,14 @@ protected void run() throws Exception {
|
||||||
for (final RevSort s : sorting)
|
for (final RevSort s : sorting)
|
||||||
walk.sort(s, true);
|
walk.sort(s, true);
|
||||||
|
|
||||||
if (pathFilter instanceof FollowFilter)
|
if (pathFilter == TreeFilter.ALL) {
|
||||||
walk.setTreeFilter(pathFilter);
|
if (followPath != null)
|
||||||
else if (pathFilter != TreeFilter.ALL)
|
walk.setTreeFilter(FollowFilter.create(followPath,
|
||||||
|
db.getConfig().get(DiffConfig.KEY)));
|
||||||
|
} else if (pathFilter != TreeFilter.ALL) {
|
||||||
walk.setTreeFilter(AndTreeFilter.create(pathFilter,
|
walk.setTreeFilter(AndTreeFilter.create(pathFilter,
|
||||||
TreeFilter.ANY_DIFF));
|
TreeFilter.ANY_DIFF));
|
||||||
|
}
|
||||||
|
|
||||||
if (revLimiter.size() == 1)
|
if (revLimiter.size() == 1)
|
||||||
walk.setRevFilter(revLimiter.get(0));
|
walk.setRevFilter(revLimiter.get(0));
|
||||||
|
|
|
@ -47,8 +47,10 @@
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
|
import org.eclipse.jgit.diff.DiffConfig;
|
||||||
import org.eclipse.jgit.diff.DiffEntry;
|
import org.eclipse.jgit.diff.DiffEntry;
|
||||||
import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
|
import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
|
||||||
|
import org.eclipse.jgit.lib.Config;
|
||||||
import org.junit.Assert;
|
import org.junit.Assert;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
@ -74,7 +76,8 @@ public void setUp() throws Exception {
|
||||||
}
|
}
|
||||||
|
|
||||||
protected FollowFilter follow(final String followPath) {
|
protected FollowFilter follow(final String followPath) {
|
||||||
FollowFilter followFilter = FollowFilter.create(followPath);
|
FollowFilter followFilter =
|
||||||
|
FollowFilter.create(followPath, new Config().get(DiffConfig.KEY));
|
||||||
followFilter.setRenameCallback(diffCollector);
|
followFilter.setRenameCallback(diffCollector);
|
||||||
rw.setTreeFilter(followFilter);
|
rw.setTreeFilter(followFilter);
|
||||||
return followFilter;
|
return followFilter;
|
||||||
|
|
|
@ -119,6 +119,8 @@ public class DiffFormatter {
|
||||||
|
|
||||||
private ObjectReader reader;
|
private ObjectReader reader;
|
||||||
|
|
||||||
|
private DiffConfig diffCfg;
|
||||||
|
|
||||||
private int context = 3;
|
private int context = 3;
|
||||||
|
|
||||||
private int abbreviationLength = 7;
|
private int abbreviationLength = 7;
|
||||||
|
@ -173,6 +175,7 @@ public void setRepository(Repository repository) {
|
||||||
|
|
||||||
db = repository;
|
db = repository;
|
||||||
reader = db.newObjectReader();
|
reader = db.newObjectReader();
|
||||||
|
diffCfg = db.getConfig().get(DiffConfig.KEY);
|
||||||
|
|
||||||
ContentSource cs = ContentSource.create(reader);
|
ContentSource cs = ContentSource.create(reader);
|
||||||
source = new ContentSource.Pair(cs, cs);
|
source = new ContentSource.Pair(cs, cs);
|
||||||
|
@ -534,7 +537,7 @@ private List<DiffEntry> updateFollowFilter(List<DiffEntry> files) {
|
||||||
String oldPath = ((FollowFilter) pathFilter).getPath();
|
String oldPath = ((FollowFilter) pathFilter).getPath();
|
||||||
for (DiffEntry ent : files) {
|
for (DiffEntry ent : files) {
|
||||||
if (isRename(ent) && ent.getNewPath().equals(oldPath)) {
|
if (isRename(ent) && ent.getNewPath().equals(oldPath)) {
|
||||||
pathFilter = FollowFilter.create(ent.getOldPath());
|
pathFilter = FollowFilter.create(ent.getOldPath(), diffCfg);
|
||||||
return Collections.singletonList(ent);
|
return Collections.singletonList(ent);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -111,7 +111,7 @@ private int sortOf(ChangeType changeType) {
|
||||||
|
|
||||||
private boolean done;
|
private boolean done;
|
||||||
|
|
||||||
private final Repository repo;
|
private final ObjectReader objectReader;
|
||||||
|
|
||||||
/** Similarity score required to pair an add/delete as a rename. */
|
/** Similarity score required to pair an add/delete as a rename. */
|
||||||
private int renameScore = 60;
|
private int renameScore = 60;
|
||||||
|
@ -136,11 +136,20 @@ private int sortOf(ChangeType changeType) {
|
||||||
* the repository to use for rename detection
|
* the repository to use for rename detection
|
||||||
*/
|
*/
|
||||||
public RenameDetector(Repository repo) {
|
public RenameDetector(Repository repo) {
|
||||||
this.repo = repo;
|
this(repo.newObjectReader(), repo.getConfig().get(DiffConfig.KEY));
|
||||||
|
}
|
||||||
|
|
||||||
DiffConfig cfg = repo.getConfig().get(DiffConfig.KEY);
|
/**
|
||||||
|
* Create a new rename detector with a specified reader and diff config.
|
||||||
|
*
|
||||||
|
* @param reader
|
||||||
|
* reader to obtain objects from the repository with.
|
||||||
|
* @param cfg
|
||||||
|
* diff config specifying rename detection options.
|
||||||
|
*/
|
||||||
|
public RenameDetector(ObjectReader reader, DiffConfig cfg) {
|
||||||
|
objectReader = reader.newReader();
|
||||||
renameLimit = cfg.getRenameLimit();
|
renameLimit = cfg.getRenameLimit();
|
||||||
|
|
||||||
reset();
|
reset();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -310,11 +319,10 @@ public List<DiffEntry> compute() throws IOException {
|
||||||
*/
|
*/
|
||||||
public List<DiffEntry> compute(ProgressMonitor pm) throws IOException {
|
public List<DiffEntry> compute(ProgressMonitor pm) throws IOException {
|
||||||
if (!done) {
|
if (!done) {
|
||||||
ObjectReader reader = repo.newObjectReader();
|
|
||||||
try {
|
try {
|
||||||
return compute(reader, pm);
|
return compute(objectReader, pm);
|
||||||
} finally {
|
} finally {
|
||||||
reader.release();
|
objectReader.release();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return Collections.unmodifiableList(entries);
|
return Collections.unmodifiableList(entries);
|
||||||
|
|
|
@ -45,6 +45,7 @@
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
|
||||||
|
import org.eclipse.jgit.diff.DiffConfig;
|
||||||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||||
import org.eclipse.jgit.errors.MissingObjectException;
|
import org.eclipse.jgit.errors.MissingObjectException;
|
||||||
import org.eclipse.jgit.treewalk.TreeWalk;
|
import org.eclipse.jgit.treewalk.TreeWalk;
|
||||||
|
@ -77,20 +78,25 @@ public class FollowFilter extends TreeFilter {
|
||||||
* the path to filter on. Must not be the empty string. All
|
* the path to filter on. Must not be the empty string. All
|
||||||
* trailing '/' characters will be trimmed before string's length
|
* trailing '/' characters will be trimmed before string's length
|
||||||
* is checked or is used as part of the constructed filter.
|
* is checked or is used as part of the constructed filter.
|
||||||
|
* @param cfg
|
||||||
|
* diff config specifying rename detection options.
|
||||||
* @return a new filter for the requested path.
|
* @return a new filter for the requested path.
|
||||||
* @throws IllegalArgumentException
|
* @throws IllegalArgumentException
|
||||||
* the path supplied was the empty string.
|
* the path supplied was the empty string.
|
||||||
|
* @since 3.0
|
||||||
*/
|
*/
|
||||||
public static FollowFilter create(String path) {
|
public static FollowFilter create(String path, DiffConfig cfg) {
|
||||||
return new FollowFilter(PathFilter.create(path));
|
return new FollowFilter(PathFilter.create(path), cfg);
|
||||||
}
|
}
|
||||||
|
|
||||||
private final PathFilter path;
|
private final PathFilter path;
|
||||||
|
final DiffConfig cfg;
|
||||||
|
|
||||||
private RenameCallback renameCallback;
|
private RenameCallback renameCallback;
|
||||||
|
|
||||||
FollowFilter(final PathFilter path) {
|
FollowFilter(final PathFilter path, final DiffConfig cfg) {
|
||||||
this.path = path;
|
this.path = path;
|
||||||
|
this.cfg = cfg;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @return the path this filter matches. */
|
/** @return the path this filter matches. */
|
||||||
|
@ -112,7 +118,7 @@ public boolean shouldBeRecursive() {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public TreeFilter clone() {
|
public TreeFilter clone() {
|
||||||
return new FollowFilter(path.clone());
|
return new FollowFilter(path.clone(), cfg);
|
||||||
}
|
}
|
||||||
|
|
||||||
@SuppressWarnings("nls")
|
@SuppressWarnings("nls")
|
||||||
|
|
|
@ -46,15 +46,15 @@
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
|
import org.eclipse.jgit.diff.DiffConfig;
|
||||||
import org.eclipse.jgit.diff.DiffEntry;
|
import org.eclipse.jgit.diff.DiffEntry;
|
||||||
import org.eclipse.jgit.diff.RenameDetector;
|
|
||||||
import org.eclipse.jgit.diff.DiffEntry.ChangeType;
|
import org.eclipse.jgit.diff.DiffEntry.ChangeType;
|
||||||
|
import org.eclipse.jgit.diff.RenameDetector;
|
||||||
import org.eclipse.jgit.errors.CorruptObjectException;
|
import org.eclipse.jgit.errors.CorruptObjectException;
|
||||||
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
|
||||||
import org.eclipse.jgit.errors.MissingObjectException;
|
import org.eclipse.jgit.errors.MissingObjectException;
|
||||||
import org.eclipse.jgit.errors.StopWalkException;
|
import org.eclipse.jgit.errors.StopWalkException;
|
||||||
import org.eclipse.jgit.lib.ObjectId;
|
import org.eclipse.jgit.lib.ObjectId;
|
||||||
import org.eclipse.jgit.lib.Repository;
|
|
||||||
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
import org.eclipse.jgit.revwalk.filter.RevFilter;
|
||||||
import org.eclipse.jgit.treewalk.TreeWalk;
|
import org.eclipse.jgit.treewalk.TreeWalk;
|
||||||
import org.eclipse.jgit.treewalk.filter.TreeFilter;
|
import org.eclipse.jgit.treewalk.filter.TreeFilter;
|
||||||
|
@ -82,10 +82,7 @@ class RewriteTreeFilter extends RevFilter {
|
||||||
|
|
||||||
private final TreeWalk pathFilter;
|
private final TreeWalk pathFilter;
|
||||||
|
|
||||||
private final Repository repository;
|
|
||||||
|
|
||||||
RewriteTreeFilter(final RevWalk walker, final TreeFilter t) {
|
RewriteTreeFilter(final RevWalk walker, final TreeFilter t) {
|
||||||
repository = walker.repository;
|
|
||||||
pathFilter = new TreeWalk(walker.reader);
|
pathFilter = new TreeWalk(walker.reader);
|
||||||
pathFilter.setFilter(t);
|
pathFilter.setFilter(t);
|
||||||
pathFilter.setRecursive(t.shouldBeRecursive());
|
pathFilter.setRecursive(t.shouldBeRecursive());
|
||||||
|
@ -142,7 +139,7 @@ public boolean include(final RevWalk walker, final RevCommit c)
|
||||||
// commit. We need to update our filter to its older
|
// commit. We need to update our filter to its older
|
||||||
// name, if we can discover it. Find out what that is.
|
// name, if we can discover it. Find out what that is.
|
||||||
//
|
//
|
||||||
updateFollowFilter(trees);
|
updateFollowFilter(trees, ((FollowFilter) tw.getFilter()).cfg);
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
@ -235,7 +232,7 @@ public boolean requiresCommitBody() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
private void updateFollowFilter(ObjectId[] trees)
|
private void updateFollowFilter(ObjectId[] trees, DiffConfig cfg)
|
||||||
throws MissingObjectException, IncorrectObjectTypeException,
|
throws MissingObjectException, IncorrectObjectTypeException,
|
||||||
CorruptObjectException, IOException {
|
CorruptObjectException, IOException {
|
||||||
TreeWalk tw = pathFilter;
|
TreeWalk tw = pathFilter;
|
||||||
|
@ -244,14 +241,14 @@ private void updateFollowFilter(ObjectId[] trees)
|
||||||
tw.reset(trees);
|
tw.reset(trees);
|
||||||
|
|
||||||
List<DiffEntry> files = DiffEntry.scan(tw);
|
List<DiffEntry> files = DiffEntry.scan(tw);
|
||||||
RenameDetector rd = new RenameDetector(repository);
|
RenameDetector rd = new RenameDetector(tw.getObjectReader(), cfg);
|
||||||
rd.addAll(files);
|
rd.addAll(files);
|
||||||
files = rd.compute();
|
files = rd.compute();
|
||||||
|
|
||||||
TreeFilter newFilter = oldFilter;
|
TreeFilter newFilter = oldFilter;
|
||||||
for (DiffEntry ent : files) {
|
for (DiffEntry ent : files) {
|
||||||
if (isRename(ent) && ent.getNewPath().equals(oldFilter.getPath())) {
|
if (isRename(ent) && ent.getNewPath().equals(oldFilter.getPath())) {
|
||||||
newFilter = FollowFilter.create(ent.getOldPath());
|
newFilter = FollowFilter.create(ent.getOldPath(), cfg);
|
||||||
RenameCallback callback = oldFilter.getRenameCallback();
|
RenameCallback callback = oldFilter.getRenameCallback();
|
||||||
if (callback != null) {
|
if (callback != null) {
|
||||||
callback.renamed(ent);
|
callback.renamed(ent);
|
||||||
|
|
Loading…
Reference in New Issue