gnodet-bot commented on code in PR #1155:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1155#discussion_r4181252010
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -682,6 +682,31 @@ final Charset charset() {
@Parameter(property = "maven.compiler.useIncrementalCompilation")
protected Boolean useIncrementalCompilation;
+ /**
+ * The strategy to use for incremental compilation.
+ * <ul>
+ * <li>{@code timestamp} (default) — the existing timestamp-based
strategy from
+ * {@link IncrementalBuild}. Detects changes by comparing source
file modification
+ * times and triggers full rebuilds when files are added/removed or
dependencies change.
+ * Respects {@link #incrementalCompilation} aspects, {@code
staleMillis}, and
+ * {@code incrementalExcludes}.</li>
+ * <li>{@code graph} — dependency-graph-based strategy. Tracks
class-level dependencies
+ * by analysing bytecode after each compilation pass. When a source
file changes, only
+ * the source files that transitively depend on any of its classes
are recompiled. Any
+ * change to a class (API or implementation) cascades to all its
consumers.
+ * Full JPMS support including {@code module-info.java} and
+ * {@code module-info-patch.maven} tracking.
+ * <p>Note: the graph strategy has its own change detection (content
hashing) and does
+ * not use {@code staleMillis}, {@code incrementalExcludes}, or the
+ * {@link #incrementalCompilation} aspects. Setting {@link
#useIncrementalCompilation}
+ * to {@code false} disables this strategy and forces a full
rebuild.</p></li>
+ * </ul>
+ *
+ * @since 4.0.0-beta-7
+ */
+ @Parameter(property = "maven.compiler.incrementalStrategy", defaultValue =
"timestamp")
+ protected String incrementalStrategy;
+
/**
* Returns the configuration of the incremental compilation.
* If the argument is null or blank, then this method applies
Review Comment:
💡 The Javadoc for `incrementalStrategy` documents `graph` and `timestamp`,
but the dispatch in `compile()` checks for `"graph"` while the actual strategy
name used by the ABI engine is `"abi"` (the IT properties use
`maven.compiler.incrementalStrategy=abi`). This means the user-facing property
value and the Javadoc are inconsistent:
- The ITs set `abi`
- The Javadoc says `graph`
- The dispatch checks `graph`
Users who read the IT examples will try `abi` and get the timestamp strategy
silently. Should the check in `compile()` accept both `graph` and `abi`, or
should the ITs use `graph`?
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java:
##########
@@ -0,0 +1,893 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.plugin.compiler.incremental;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+
+/**
+ * ABI-fingerprint-driven incremental build engine, designed for embedding
+ * in maven-compiler-plugin alongside the existing timestamp-based
+ * {@code IncrementalBuild}.
+ *
+ * <p>The plugin drives compilation; this class determines <em>what</em> to
+ * compile and performs post-compilation bytecode analysis to build the
+ * dependency graph and ABI fingerprints. Typical usage:
+ *
+ * <pre>{@code
+ * var abi = new AbiIncrementalBuild(outputDir);
+ * abi.setClasspathEntries(classpath);
+ * abi.setReactorModulePaths(reactorModules);
+ * abi.setProcessorPath(processorPath);
+ * abi.setConfigHash(configHash);
+ *
+ * Set<Path> toCompile = abi.initialize(allSourceFiles);
+ *
+ * while (!toCompile.isEmpty()) {
+ * compiler.compile(toCompile); // any compiler, any mode
+ * toCompile = abi.processCompiledClasses(toCompile);
+ * }
+ *
+ * abi.finish();
+ * }</pre>
+ *
+ * <p>After each compilation pass, {@link #processCompiledClasses(Set)} scans
+ * the freshly produced {@code .class} files, updates the dependency graph and
+ * ABI fingerprints, and returns any additional files that must be compiled in
+ * the next pass (cascade due to ABI changes, or newly discovered
dependencies).
+ * The loop converges in at most 2–3 passes in practice.
+ *
+ * <p>The engine persists its state as {@code .abi-incremental-state} in the
+ * {@code target/maven-status/maven-compiler-plugin/<outputDirName>/}
directory (outside the class
+ * output directory so it is not packaged into JARs) and writes an {@link
AbiManifest}
+ * ({@code .abi-fingerprints}) in the build directory for downstream reactor
modules.
+ *
+ * @see IncrementalState
+ */
+public class AbiIncrementalBuild {
+
+ /** Prefix used to distinguish module-info entries from regular type
entries in the state. */
+ static final String MODULE_PREFIX = "module:";
+
+ private final Path outputDir;
+ private final Path buildDir;
+ private final Path stateFile;
+ private List<Path> classpathEntries;
+ private Set<Path> reactorModulePaths;
+ private List<Path> processorPath;
+ private ProcessorClassification processorClassification;
+
+ private IncrementalState previousState;
+ private IncrementalState state;
+ private Map<String, String> sourceHashes;
+ private Map<String, Long> sourceMtimes;
+ private List<Path> allSourceFiles;
+ private Set<String> allCompiled;
+ private boolean fullBuild;
+ private boolean useModulePrefixedPaths;
+ private String configHash = "";
+ private String rebuildCause;
+ private int totalSources;
+ /** Lazily populated on full builds; maps each output class file to its
simple top-level class name. */
+ private java.util.Map<Path, String> outputClassIndex;
+
+ public AbiIncrementalBuild(Path outputDir) {
+ this.outputDir = outputDir;
+ this.buildDir = outputDir.getParent() != null ? outputDir.getParent()
: outputDir;
+ // Store state outside the output directory so it is not included in
the JAR.
+ // Use the same maven-status convention as the timestamp-based
strategy.
Review Comment:
📝 Minor: `outputClassIndex` is declared as `java.util.Map<Path, String>`
with the fully-qualified type instead of using the import at line 34 (`import
java.util.Map`). Consistent with the rest of the file would be `Map<Path,
String>`.
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/ExternalAbiResolver.java:
##########
@@ -0,0 +1,252 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.plugin.compiler.incremental;
+
+import java.io.IOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Resolves ABI fingerprints for types defined outside the current compilation
+ * module — in other reactor modules or in external library JARs.
+ *
+ * <p>Resolution uses a three-strategy cascade:
+ * <ol>
+ * <li><b>Manifest (option 1):</b> If a classpath directory contains an
+ * {@value AbiManifest#FILENAME} file, fingerprints are read from it.
+ * This is the fast path for reactor modules compiled with
maven-compiler-plugin.</li>
+ * <li><b>Reactor metadata (option 2):</b> The caller can mark specific
+ * classpath entries as reactor modules via {@code reactorModulePaths}.
+ * These directories are scanned for class files when no manifest is
+ * present.</li>
+ * <li><b>Bytecode fallback (option 3):</b> For any type not resolved above,
+ * the resolver searches all classpath entries (directories and JARs) and
+ * computes the ABI fingerprint from bytecode via {@link
BytecodeAnalyzer}.
+ * This works with any dependency, including third-party JARs that were
+ * not built with maven-compiler-plugin.</li>
+ * </ol>
+ *
+ * <p>JAR entries are cached by identity (path + size + last-modified-time).
+ * When a JAR has not changed since the last build and a stored fingerprint
+ * exists for the requested type, the stored fingerprint is reused without
+ * opening the JAR.
+ *
+ * @see AbiManifest
+ * @see AbiIncrementalBuild
+ */
+public class ExternalAbiResolver {
+
+ private static final Logger LOGGER =
Logger.getLogger(ExternalAbiResolver.class.getName());
+
+ private final List<Path> classpathEntries;
+ private final Set<Path> reactorModulePaths;
+ private Map<Path, Map<String, String>> manifestCache;
+
+ private Map<String, String> previousFingerprints = Map.of();
+ private Set<String> unchangedJars = Set.of();
+
+ public ExternalAbiResolver(List<Path> classpathEntries, Set<Path>
reactorModulePaths) {
+ this.classpathEntries = classpathEntries != null ? classpathEntries :
List.of();
+ this.reactorModulePaths = reactorModulePaths != null ?
reactorModulePaths : Set.of();
+ }
+
+ /**
+ * Configures JAR caching. Fingerprints for types found in unchanged JARs
+ * are reused from the previous build without re-opening the JAR.
+ *
+ * @param previousFingerprints external fingerprints from the previous
build
+ * @param storedJarIdentities JAR identities ({@code path -> size:mtime})
+ * from the previous build
+ */
+ public void setCachedState(Map<String, String> previousFingerprints,
Map<String, String> storedJarIdentities) {
+ this.previousFingerprints = previousFingerprints != null ?
previousFingerprints : Map.of();
+ this.unchangedJars = computeUnchangedJars(storedJarIdentities);
+ }
+
+ /**
Review Comment:
📝 `java.util.logging.Logger` is used here instead of the SLF4J-based logger
(`MessageBuilder`/`Log`) used elsewhere in the plugin. Since this class is in
the `incremental` package that's otherwise free of Maven API dependencies, this
is defensible — but worth noting that log output from `resolveFromBytecode`
failures will go to JUL rather than the Maven logger, which means it won't
appear in `-X` debug output unless JUL is bridged.
##########
src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java:
##########
@@ -915,6 +919,243 @@ private static boolean removeFirsts(Deque<Path> paths,
Integer count) {
}
}
+ /**
+ * Compiles using the ABI-fingerprint incremental strategy. This method
handles the full
+ * lifecycle: determining what to compile, running javac, cascading on ABI
changes,
+ * and persisting state.
+ *
+ * @param compiler the compiler
+ * @param configuration the options to give to the Java compiler
+ * @param mojo the MOJO for configuration access
+ * @throws IOException if an error occurred while reading or writing a file
+ * @throws MojoException if the compilation failed
+ */
+ void compileWithAbiIncremental(JavaCompiler compiler, final Options
configuration, final AbstractCompilerMojo mojo)
+ throws IOException {
+ var abiBuild = new AbiIncrementalBuild(outputDirectory);
+
+ // Collect annotation processor path for processor classification
+ var processorPaths = new ArrayList<Path>();
+ for (var entry : dependencies.entrySet()) {
+ if (entry.getKey() instanceof JavaPathType type) {
+ var location = type.location();
+ if (location.isPresent()
+ && (location.get() ==
StandardLocation.ANNOTATION_PROCESSOR_PATH
+ || location.get() ==
StandardLocation.ANNOTATION_PROCESSOR_MODULE_PATH)) {
+ processorPaths.addAll(entry.getValue());
+ }
+ }
+ }
+ if (!processorPaths.isEmpty()) {
+ abiBuild.setProcessorPath(processorPaths);
+ }
+
+ // Collect classpath entries for cross-module ABI tracking.
+ // Directory entries are checked for AbiManifest files (other reactor
modules compiled
+ // with the ABI strategy); JAR entries use bytecode analysis as
fallback.
+ var classpathPaths = new ArrayList<Path>();
+ var reactorPaths = new java.util.LinkedHashSet<Path>();
+ for (var entry : dependencies.entrySet()) {
+ if (entry.getKey() instanceof JavaPathType type) {
+ var location = type.location();
+ if (location.isPresent()
+ && (location.get() == StandardLocation.CLASS_PATH
+ || location.get() ==
StandardLocation.MODULE_PATH)) {
+ classpathPaths.addAll(entry.getValue());
+ if (location.get() == StandardLocation.MODULE_PATH) {
+ for (Path p : entry.getValue()) {
+ if (java.nio.file.Files.isDirectory(p)) {
+ reactorPaths.add(p);
Review Comment:
📝 `computeConfigHash` returns `""` when only the options prefix `opts:...|;`
is present (since `digest.isEmpty()` will be `false` — the `opts:` line was
already appended). That means the `digest.isEmpty()` guard at line 378 of the
diff is dead code when `configuration.options` is non-empty (which is always
true in practice since javac always has at least `-d`). Not a bug — the hash
still works correctly — but the guard could be simplified.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
