dweiss commented on code in PR #14824: URL: https://github.com/apache/lucene/pull/14824#discussion_r2159163732
########## build-tools/build-infra/src/main/java/org/apache/lucene/gradle/plugins/spotless/ParentGoogleJavaFormatTask.java: ########## @@ -0,0 +1,69 @@ +package org.apache.lucene.gradle.plugins.spotless; + +import com.google.googlejavaformat.java.Formatter; +import com.google.googlejavaformat.java.JavaFormatterOptions; +import org.gradle.api.DefaultTask; +import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.file.FileType; +import org.gradle.api.file.ProjectLayout; +import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.Internal; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.PathSensitive; +import org.gradle.api.tasks.PathSensitivity; +import org.gradle.work.ChangeType; +import org.gradle.work.FileChange; +import org.gradle.work.Incremental; +import org.gradle.work.InputChanges; +import org.gradle.workers.WorkQueue; +import org.gradle.workers.WorkerExecutor; + +import javax.inject.Inject; +import java.io.File; +import java.util.List; +import java.util.stream.StreamSupport; + +abstract class ParentGoogleJavaFormatTask extends DefaultTask { + @Incremental + @InputFiles + @PathSensitive(PathSensitivity.RELATIVE) + public abstract ConfigurableFileCollection getSourceFiles(); + + @OutputFile + public abstract RegularFileProperty getOutputChangeListFile(); + + @Inject + protected abstract WorkerExecutor getWorkerExecutor(); + + public ParentGoogleJavaFormatTask(ProjectLayout layout, String gjfTask) { + getOutputChangeListFile().convention(layout.getBuildDirectory().file("gjf-" + gjfTask + ".txt")); + } + + protected static Formatter getFormatter() { + JavaFormatterOptions options = + JavaFormatterOptions.builder() + .style(JavaFormatterOptions.Style.GOOGLE) + .formatJavadoc(true) + .reorderModifiers(true) + .build(); + return new Formatter(options); + } + + protected List<File> getIncrementalBatch(InputChanges inputChanges) { + return StreamSupport.stream(inputChanges.getFileChanges(getSourceFiles()).spliterator(), false) + .filter(fileChange -> { + return fileChange.getFileType() == FileType.FILE && + (fileChange.getChangeType() == ChangeType.ADDED || + fileChange.getChangeType() == ChangeType.MODIFIED); + }) + .map(FileChange::getFile) + .toList(); + } + + @Internal + protected WorkQueue getWorkQueue() { + // TODO: maybe fork a separate jvm so that we can pass open-module settings there and fine-tune the jvm for the task? + return getWorkerExecutor().noIsolation(); + } Review Comment: I'd love to fork a separate jvm here - where we can set some options that open up modules, etc. The problem is - there is no way to control how many isolated processes this forks. On high-end multi-core machines I see dozens of forked java processes - this doesn't make sense and is super heavy. Is there a way to control the concurrency of the isolated worker executor, @breskeby ? ########## build-tools/build-infra/src/main/java/org/apache/lucene/gradle/plugins/spotless/CheckGoogleJavaFormatTask.java: ########## @@ -0,0 +1,195 @@ +package org.apache.lucene.gradle.plugins.spotless; + +import com.github.difflib.text.DiffRow; +import com.github.difflib.text.DiffRowGenerator; +import com.google.googlejavaformat.java.FormatterException; +import com.google.googlejavaformat.java.ImportOrderer; +import com.google.googlejavaformat.java.JavaFormatterOptions; +import com.google.googlejavaformat.java.RemoveUnusedImports; +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import javax.inject.Inject; +import org.gradle.api.GradleException; +import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.file.FileSystemOperations; +import org.gradle.api.file.ProjectLayout; +import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.tasks.TaskAction; +import org.gradle.work.DisableCachingByDefault; +import org.gradle.work.InputChanges; +import org.gradle.workers.WorkAction; +import org.gradle.workers.WorkParameters; +import org.gradle.workers.WorkQueue; + +@DisableCachingByDefault +public abstract class CheckGoogleJavaFormatTask extends ParentGoogleJavaFormatTask { + + @Inject + protected abstract FileSystemOperations getFilesystemOperations(); + + @Inject + public CheckGoogleJavaFormatTask(ProjectLayout layout) { + super(layout, "check"); + } + + @TaskAction + public void formatSources(InputChanges inputChanges) throws IOException { + WorkQueue workQueue = getWorkQueue(); + List<File> sourceFiles = getIncrementalBatch(inputChanges); + if (sourceFiles.isEmpty()) { + return; + } + + getLogger() + .info( + "Will check formatting of {} source {} in this run.", + sourceFiles.size(), + sourceFiles.size() == 1 ? "file" : "files"); + + int outputSeq = 0; + File tempDir = new File(getTemporaryDir(), "changes"); + if (Files.exists(tempDir.toPath())) { + getFilesystemOperations().delete(spec -> spec.delete(tempDir)); + } + Files.createDirectory(tempDir.toPath()); + + for (var sourceFile : sourceFiles) { + final int seq = outputSeq; + workQueue.submit( + CheckGoogleJavaFormatAction.class, + params -> { + params.getTargetFiles().setFrom(List.of(sourceFile)); + params.getOutputFile().set(new File(tempDir, "gjf-check-" + seq + ".txt")); + }); + outputSeq++; + } + + workQueue.await(); + + // Check if there are any changes. + try (Stream<Path> stream = Files.list(tempDir.toPath()).sorted()) { + var allChanges = stream.toList(); + StringBuilder sb = new StringBuilder(); + for (Path changeset : allChanges) { + if (sb.length() > 1024 * 1024 * 2) { + sb.append("(...and more files, omitted)"); + break; + } + + sb.append(Files.readString(changeset)); + sb.append("\n"); + } Review Comment: @breskeby - how do you deal with worker api actions that produce something which the task should check/aggregate? The above solution seems... crude at best. It works but I am not happy with it. I also couldn't find any examples of worker api actions returning or producing something the source task should consume. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org