GH-438: Split out concern of merging sources and imports to the build…

… orchestrator
ascopes · Nov 10, 2024 · 3a0608d · 3a0608d
1 parent d4cf813
commit 3a0608d
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 49 deletions.
diff --git a/...main/java/io/github/ascopes/protobufmavenplugin/generation/ProtobufBuildOrchestrator.java b/...main/java/io/github/ascopes/protobufmavenplugin/generation/ProtobufBuildOrchestrator.java
@@ -28,8 +28,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
-import java.util.LinkedHashSet;
-import java.util.stream.Collectors;
 import javax.inject.Inject;
 import javax.inject.Named;
 import org.apache.maven.execution.MavenSession;
@@ -104,11 +102,8 @@ public boolean generate(GenerationRequest request) throws ResolutionException, I
 
     var argLineBuilder = new ArgLineBuilder(protocPath)
         .fatalWarnings(request.isFatalWarnings())
-        .importPaths(
-            projectInputs.getImports().stream()
-                .map(SourceListing::getSourceRoot)
-                .collect(Collectors.toCollection(LinkedHashSet::new))
-        );
+        .importPaths(projectInputs.getCompilableSources())
+        .importPaths(projectInputs.getDependencySources());
 
     request.getEnabledLanguages()
         .forEach(language -> argLineBuilder.generateCodeFor(
@@ -120,13 +115,7 @@ public boolean generate(GenerationRequest request) throws ResolutionException, I
     // GH-269: Add the plugins after the enabled languages to support generated code injection
     argLineBuilder.plugins(resolvedPlugins, request.getOutputDirectory());
 
-    var sourceFiles = projectInputs.getCompilableSources()
-        .stream()
-        .map(SourceListing::getSourceProtoFiles)
-        .flatMap(Collection::stream)
-        .collect(Collectors.toCollection(LinkedHashSet::new));
-
-    var argLine = argLineBuilder.compile(sourceFiles);
+    var argLine = argLineBuilder.compile(projectInputs.getCompilableSources());
 
     if (!commandLineExecutor.execute(argLine)) {
       return false;

diff --git a/...plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java b/...plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java
@@ -158,7 +158,7 @@ public AbstractGenerateMojo() {
    *       property - optional.</li>
    * </ul>
    *
-   * <p>On Linux, MacOS, and other POSIX-like systems, resolution looks for an executable
+   * <p>On Linux, macOS, and other POSIX-like systems, resolution looks for an executable
    * binary matching the exact name in any directory in the {@code $PATH} environment variable.
    *
    * <p>On Windows, the case-insensitive {@code %PATH%} environment variable is searched for an
@@ -551,7 +551,7 @@ public AbstractGenerateMojo() {
    * {@code libpthread} that can result in {@code SIGSYS} (Bad System Call) being raised.
    *
    *
-   * <p>Path resolution on Linux, Mac OS, and other POSIX-like systems, resolution looks
+   * <p>Path resolution on Linux, macOS, and other POSIX-like systems, resolution looks
    * for an executable binary matching the exact name in any directory in the {@code $PATH}
    * environment variable.
    *

diff --git a/...ven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/ArgLineBuilder.java b/...ven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/ArgLineBuilder.java
@@ -18,6 +18,7 @@
 
 import io.github.ascopes.protobufmavenplugin.generation.Language;
 import io.github.ascopes.protobufmavenplugin.plugins.ResolvedProtocPlugin;
+import io.github.ascopes.protobufmavenplugin.sources.SourceListing;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -29,6 +30,7 @@
  *
  * @author Ashley Scopes
  */
+@SuppressWarnings("UnusedReturnValue")
 public final class ArgLineBuilder {
 
   private final Path protocPath;
@@ -43,7 +45,7 @@ public ArgLineBuilder(Path protocPath) {
     importPaths = new ArrayList<>();
   }
 
-  public List<String> compile(Collection<Path> sourcesToCompile) {
+  public List<String> compile(Collection<SourceListing> sourceListings) {
     if (targets.isEmpty()) {
       throw new IllegalStateException("No output target operations were provided");
     }
@@ -59,8 +61,10 @@ public List<String> compile(Collection<Path> sourcesToCompile) {
       target.addArgsTo(args);
     }
 
-    for (var sourceToCompile : sourcesToCompile) {
-      args.add(sourceToCompile.toString());
+    for (var sourceListing : sourceListings) {
+      for (var sourcePath : sourceListing.getSourceProtoFiles()) {
+        args.add(sourcePath.toString());
+      }
     }
 
     for (var importPath : importPaths) {
@@ -81,8 +85,11 @@ public ArgLineBuilder generateCodeFor(Language language, Path outputPath, boolea
     return this;
   }
 
-  public ArgLineBuilder importPaths(Collection<Path> importPaths) {
-    this.importPaths.addAll(importPaths);
+  public ArgLineBuilder importPaths(Collection<SourceListing> importPathListings) {
+    for (var importPathListing : importPathListings) {
+      importPaths.add(importPathListing.getSourceRoot());
+    }
+
     return this;
   }
 

diff --git a/...ugin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/ProjectInputListing.java b/...ugin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/ProjectInputListing.java
@@ -29,5 +29,5 @@
 public interface ProjectInputListing {
   Collection<SourceListing> getCompilableSources();
 
-  Collection<SourceListing> getImports();
+  Collection<SourceListing> getDependencySources();
 }
diff --git a/...gin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/ProjectInputResolver.java b/...gin/src/main/java/io/github/ascopes/protobufmavenplugin/sources/ProjectInputResolver.java
@@ -20,7 +20,6 @@
 import io.github.ascopes.protobufmavenplugin.generation.GenerationRequest;
 import io.github.ascopes.protobufmavenplugin.utils.ResolutionException;
 import java.util.Collection;
-import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.inject.Inject;
@@ -54,12 +53,10 @@ public ProjectInputResolver(
   public ProjectInputListing resolveProjectInputs(
       GenerationRequest request
   ) throws ResolutionException {
-    var compilableSources = resolveCompilableSources(request);
-    var imports = resolveImports(request, compilableSources);
-
+    // XXX: We might want to run these two resolution steps in parallel in the future.
     return ImmutableProjectInputListing.builder()
-        .compilableSources(compilableSources)
-        .imports(imports)
+        .compilableSources(resolveCompilableSources(request))
+        .dependencySources(resolveDependencySources(request))
         .build();
   }
 
@@ -88,7 +85,9 @@ private Collection<SourceListing> resolveCompilableSources(
         filter
     );
 
-    var sourcePaths = concat(sourcePathsListings, sourceDependencyListings);
+    var sourcePaths = Stream
+        .concat(sourcePathsListings.stream(), sourceDependencyListings.stream())
+        .collect(Collectors.toUnmodifiableList());
 
     var sourceFileCount = sourcePaths.stream()
         .mapToInt(sourcePath -> sourcePath.getSourceProtoFiles().size())
@@ -103,9 +102,8 @@ private Collection<SourceListing> resolveCompilableSources(
     return sourcePaths;
   }
 
-  private Collection<SourceListing> resolveImports(
-      GenerationRequest request,
-      Collection<SourceListing> knownSourceListings
+  private Collection<SourceListing> resolveDependencySources(
+      GenerationRequest request
   ) throws ResolutionException {
     var artifactPaths = artifactPathResolver.resolveDependencies(
         request.getImportDependencies(),
@@ -117,25 +115,11 @@ private Collection<SourceListing> resolveImports(
 
     var filter = new SourceGlobFilter();
 
-    var importListings = sourceResolver.resolveSources(
-        concat(request.getImportPaths(), artifactPaths),
-        filter
-    );
-
-    // Use the source paths here as well and use them first to give them precedence. This works
-    // around GH-172 where we can end up with different versions on the import and source paths
-    // depending on how dependency conflicts arise.
-    return Stream.concat(knownSourceListings.stream(), importListings.stream())
-        .distinct()
+    var importPaths = Stream
+        .concat(request.getImportPaths().stream(), artifactPaths.stream())
         .collect(Collectors.toUnmodifiableList());
-  }
 
-  @SafeVarargs
-  @SuppressWarnings("varargs")
-  private static <T> List<T> concat(Collection<? extends T>... collections) {
-    return Stream.of(collections)
-        .flatMap(Collection::stream)
-        .collect(Collectors.toUnmodifiableList());
+    return sourceResolver.resolveSources(importPaths, filter);
   }
 
   private static String pluralize(int count, String name) {