diff --git a/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java b/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java index b71731b4e5..5130760091 100644 --- a/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java +++ b/lib-extra/src/main/java/com/diffplug/spotless/extra/GitRatchet.java @@ -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 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 relativePathUnix) 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(); @@ -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(relativePathUnix), new IndexDiffFilter(INDEX, WORKDIR))); if (!treeWalk.next()) { @@ -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 " + relativePathUnix + " against treeSha " + treeSha + " is invalid."); } else { // they are all unique // we have to check manually @@ -135,7 +141,7 @@ private static boolean worktreeIsCleanCheckout(TreeWalk treeWalk) { * builds and submodules, it's quite possible that a single Gradle project will span across multiple git repositories. * We cache the Repository for every Project in `gitRoots`, and use dynamic programming to populate it. */ - private Repository repositoryFor(Project project) throws IOException { + protected Repository repositoryFor(Project project) throws IOException { Repository repo = gitRoots.get(project); if (repo == null) { if (isGitRoot(getDir(project))) { diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 1bcd662201..61318f4136 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`). ## [Unreleased] +### Fixed +Improve speed of plugin when using `` ([#701](https://github.com/diffplug/spotless/pull/706)). ## [2.4.1] - 2020-09-18 ### Fixed diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java index 38de3f5aa3..b7fd05451c 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/AbstractSpotlessMojo.java @@ -19,6 +19,7 @@ import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -30,6 +31,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; @@ -38,11 +40,11 @@ import org.codehaus.plexus.resource.ResourceManager; import org.codehaus.plexus.resource.loader.FileResourceLoader; import org.codehaus.plexus.util.FileUtils; +import org.codehaus.plexus.util.MatchPatterns; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.repository.RemoteRepository; -import com.diffplug.common.collect.Iterables; import com.diffplug.spotless.Formatter; import com.diffplug.spotless.LineEnding; import com.diffplug.spotless.Provisioner; @@ -137,38 +139,23 @@ public final void execute() throws MojoExecutionException { } private void execute(FormatterFactory formatterFactory) throws MojoExecutionException { - List files = collectFiles(formatterFactory); FormatterConfig config = getFormatterConfig(); - Optional ratchetFrom = formatterFactory.ratchetFrom(config); - Iterable toFormat; - if (!ratchetFrom.isPresent()) { - toFormat = files; - } else { - toFormat = Iterables.filter(files, GitRatchetMaven.instance().isGitDirty(baseDir, ratchetFrom.get())); - } + List files = collectFiles(formatterFactory, config); + try (Formatter formatter = formatterFactory.newFormatter(files, config)) { - process(toFormat, formatter); + process(files, formatter); } } - private List collectFiles(FormatterFactory formatterFactory) throws MojoExecutionException { - Set configuredIncludes = formatterFactory.includes(); - Set configuredExcludes = formatterFactory.excludes(); - - Set includes = configuredIncludes.isEmpty() ? formatterFactory.defaultIncludes() : configuredIncludes; - if (includes.isEmpty()) { - throw new MojoExecutionException("You must specify some files to include, such as 'src/**'"); - } - - Set excludes = new HashSet<>(FileUtils.getDefaultExcludesAsList()); - excludes.add(withTrailingSeparator(buildDir.toString())); - excludes.addAll(configuredExcludes); - - String includesString = String.join(",", includes); - String excludesString = String.join(",", excludes); - + private List collectFiles(FormatterFactory formatterFactory, FormatterConfig config) throws MojoExecutionException { + Optional ratchetFrom = formatterFactory.ratchetFrom(config); try { - final List files = FileUtils.getFiles(baseDir, includesString, excludesString); + final List files; + if (ratchetFrom.isPresent()) { + files = collectFilesFromGit(formatterFactory, ratchetFrom.get()); + } else { + files = collectFilesFromFormatterFactory(formatterFactory); + } if (filePatterns == null || filePatterns.isEmpty()) { return files; } @@ -189,10 +176,68 @@ private List collectFiles(FormatterFactory formatterFactory) throws MojoEx } } + private List collectFilesFromGit(FormatterFactory formatterFactory, String ratchetFrom) throws MojoExecutionException { + MatchPatterns includePatterns = MatchPatterns.from( + withNormalizedFileSeparators(getIncludes(formatterFactory))); + MatchPatterns excludePatterns = MatchPatterns.from( + withNormalizedFileSeparators(getExcludes(formatterFactory))); + + Iterable dirtyFiles; + try { + dirtyFiles = GitRatchetMaven + .instance().getDirtyFiles(baseDir, ratchetFrom); + } catch (IOException e) { + throw new MojoExecutionException("Unable to scan file tree rooted at " + baseDir, e); + } + + List result = new ArrayList<>(); + for (String file : withNormalizedFileSeparators(dirtyFiles)) { + if (includePatterns.matches(file, true)) { + if (!excludePatterns.matches(file, true)) { + result.add(new File(baseDir.getPath(), file)); + } + } + } + return result; + } + + private List collectFilesFromFormatterFactory(FormatterFactory formatterFactory) + throws MojoExecutionException, IOException { + String includesString = String.join(",", getIncludes(formatterFactory)); + String excludesString = String.join(",", getExcludes(formatterFactory)); + + return FileUtils.getFiles(baseDir, includesString, excludesString); + } + + private Iterable withNormalizedFileSeparators(Iterable patterns) { + return StreamSupport.stream(patterns.spliterator(), true) + .map(pattern -> pattern.replace('/', File.separatorChar)) + .map(pattern -> pattern.replace('\\', File.separatorChar)) + .collect(Collectors.toSet()); + } + private static String withTrailingSeparator(String path) { return path.endsWith(File.separator) ? path : path + File.separator; } + private Set getIncludes(FormatterFactory formatterFactory) throws MojoExecutionException { + Set configuredIncludes = formatterFactory.includes(); + Set includes = configuredIncludes.isEmpty() ? formatterFactory.defaultIncludes() : configuredIncludes; + if (includes.isEmpty()) { + throw new MojoExecutionException("You must specify some files to include, such as 'src/**'"); + } + return includes; + } + + private Set getExcludes(FormatterFactory formatterFactory) { + Set configuredExcludes = formatterFactory.excludes(); + + Set excludes = new HashSet<>(FileUtils.getDefaultExcludesAsList()); + excludes.add(withTrailingSeparator(buildDir.toString())); + excludes.addAll(configuredExcludes); + return excludes; + } + private FormatterConfig getFormatterConfig() { ArtifactResolver resolver = new ArtifactResolver(repositorySystem, repositorySystemSession, repositories, getLog()); Provisioner provisioner = MavenProvisioner.create(resolver); diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/GitRatchetMaven.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/GitRatchetMaven.java index a759f05b06..f68ac295f6 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/GitRatchetMaven.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/GitRatchetMaven.java @@ -17,11 +17,17 @@ import java.io.File; import java.io.IOException; -import java.util.function.Predicate; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; +import org.eclipse.jgit.lib.IndexDiff; import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.treewalk.FileTreeIterator; -import com.diffplug.common.base.Errors; import com.diffplug.spotless.extra.GitRatchet; final class GitRatchetMaven extends GitRatchet { @@ -50,15 +56,44 @@ static GitRatchetMaven instance() { return instance; } - /** A predicate which returns only the "git dirty" files. */ - Predicate isGitDirty(File baseDir, String ratchetFrom) { + Iterable getDirtyFiles(File baseDir, String ratchetFrom) throws IOException { + Repository repository = repositoryFor(baseDir); ObjectId sha = rootTreeShaOf(baseDir, ratchetFrom); - return file -> { - try { - return !isClean(baseDir, sha, file); - } catch (IOException e) { - throw Errors.asRuntime(e); + + IndexDiff indexDiff = new IndexDiff(repository, sha, new FileTreeIterator(repository)); + indexDiff.diff(); + + String workTreePath = repository.getWorkTree().getPath(); + Path baseDirPath = Paths.get(baseDir.getPath()); + + Set 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()); } }