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

Reply via email to