Skip to content

Commit

Permalink
Replace DiffCommand with IndexDiff
Browse files Browse the repository at this point in the history
DiffCommand suffered from not taking smudge/clean filters into account
  • Loading branch information
Erik committed Sep 27, 2020
1 parent d9cf50a commit 1ae409e
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,21 @@
* - If you have up-to-date checking and want the best possible performance, use {@link #subtreeShaOf(Object, ObjectId)} to optimize up-to-date checks on a per-project basis.
*/
public abstract class GitRatchet<Project> implements AutoCloseable {

public boolean isClean(Project project, ObjectId treeSha, File file) throws IOException {
Repository repo = repositoryFor(project);
String relativePath = FileSignature.pathNativeToUnix(repo.getWorkTree().toPath().relativize(file.toPath()).toString());
return isClean(project, treeSha, relativePath);
}

/**
* This is the highest-level method, which all the others serve. Given the sha
* of a git tree (not a commit!), and the file in question, this method returns
* true if that file is clean relative to that tree. A naive implementation of this
* could be verrrry slow, so the rest of this is about speeding this up.
*/
public boolean isClean(Project project, ObjectId treeSha, File file) throws IOException {
public boolean isClean(Project project, ObjectId treeSha, String relativePath) throws IOException {
Repository repo = repositoryFor(project);
String path = FileSignature.pathNativeToUnix(repo.getWorkTree().toPath().relativize(file.toPath()).toString());

// TODO: should be cached-per-repo if it is thread-safe, or per-repo-per-thread if it is not
DirCache dirCache = repo.readDirCache();
Expand All @@ -75,7 +81,7 @@ public boolean isClean(Project project, ObjectId treeSha, File file) throws IOEx
treeWalk.addTree(new DirCacheIterator(dirCache));
treeWalk.addTree(new FileTreeIterator(repo));
treeWalk.setFilter(AndTreeFilter.create(
PathFilter.create(path),
PathFilter.create(relativePath),
new IndexDiffFilter(INDEX, WORKDIR)));

if (!treeWalk.next()) {
Expand All @@ -102,7 +108,7 @@ public boolean isClean(Project project, ObjectId treeSha, File file) throws IOEx
} else if (treeEqualsIndex) {
// this means they are all equal to each other, which should never happen
// the IndexDiffFilter should keep those out of the TreeWalk entirely
throw new IllegalStateException("Index status for " + file + " against treeSha " + treeSha + " is invalid.");
throw new IllegalStateException("Index status for " + relativePath + " against treeSha " + treeSha + " is invalid.");
} else {
// they are all unique
// we have to check manually
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.eclipse.aether.RepositorySystem;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.jgit.api.errors.GitAPIException;

import com.diffplug.spotless.Formatter;
import com.diffplug.spotless.LineEnding;
Expand Down Expand Up @@ -190,8 +189,6 @@ private List<File> collectFilesFromGit(FormatterFactory formatterFactory, String
.instance().getDirtyFiles(baseDir, ratchetFrom);
} catch (IOException e) {
throw new MojoExecutionException("Unable to scan file tree rooted at " + baseDir, e);
} catch (GitAPIException e) {
throw new MojoExecutionException("Error getting diff against 'ratchetFrom' setting '" + ratchetFrom + "'", e);
}

List<File> result = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;

import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.diff.DiffEntry;
import org.eclipse.jgit.lib.IndexDiff;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.treewalk.CanonicalTreeParser;
import org.eclipse.jgit.treewalk.FileTreeIterator;

import com.diffplug.spotless.extra.GitRatchet;

Expand Down Expand Up @@ -58,28 +56,44 @@ static GitRatchetMaven instance() {
return instance;
}

Iterable<String> getDirtyFiles(File baseDir, String ratchetFrom)
throws IOException, GitAPIException {
Iterable<String> getDirtyFiles(File baseDir, String ratchetFrom) throws IOException {
Repository repository = repositoryFor(baseDir);
ObjectId sha = rootTreeShaOf(baseDir, ratchetFrom);

ObjectReader oldReader = repository.newObjectReader();
CanonicalTreeParser oldTree = new CanonicalTreeParser();
oldTree.reset(oldReader, sha);

Git git = new Git(repository);
List<DiffEntry> diffs = git.diff()
.setShowNameAndStatusOnly(true)
.setOldTree(oldTree)
.call();
IndexDiff indexDiff = new IndexDiff(repository, sha, new FileTreeIterator(repository));
indexDiff.diff();

String workTreePath = repository.getWorkTree().getPath();
Path baseDirPath = Paths.get(baseDir.getPath());

return diffs.stream()
.map(DiffEntry::getNewPath)
.map(path -> Paths.get(workTreePath, path))
.map(path -> baseDirPath.relativize(path).toString())
Set<String> dirtyPaths = new HashSet<>(indexDiff.getChanged());
dirtyPaths.addAll(indexDiff.getAdded());
dirtyPaths.addAll(indexDiff.getConflicting());
dirtyPaths.addAll(indexDiff.getUntracked());

for (String path : indexDiff.getModified()) {
if (!dirtyPaths.add(path)) {
// File differs to index both in working tree and local repository,
// which means the working tree and local repository versions may be equal
if (isClean(baseDir, sha, path)) {
dirtyPaths.remove(path);
}
}
}
for (String path : indexDiff.getRemoved()) {
if (dirtyPaths.contains(path)) {
// A removed file can also be untracked, if a new file with the same name has been created.
// This file may be identical to the one in the local repository.
if (isClean(baseDir, sha, path)) {
dirtyPaths.remove(path);
}
}
}
// A file can be modified in the index but removed in the tree
dirtyPaths.removeAll(indexDiff.getMissing());

return dirtyPaths.stream()
.map(path -> baseDirPath.relativize(Paths.get(workTreePath, path)).toString())
.collect(Collectors.toList());
}
}

0 comments on commit 1ae409e

Please sign in to comment.