gnodet-bot commented on code in PR #1145:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1145#discussion_r4134065450
##########
pom.xml:
##########
@@ -192,6 +192,34 @@ under the License.
</plugin>
</plugins>
</pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <executions>
+ <execution>
+ <!--
+ Compile sources that require java.lang.classfile (JDK 24+)
separately
+ with release=24, so the main sources stay at the baseline Java
version.
+ The compiled classes land alongside the main classes in the
output directory.
+ BytecodeAnalyzer loads ClassfileClassAnalyzer reflectively at
runtime when
+ the running JVM is JDK 24 or later; AsmClassAnalyzer is used
otherwise.
+ -->
+ <id>compile-java24</id>
+ <goals>
+ <goal>compile</goal>
+ </goals>
+ <phase>compile</phase>
+ <configuration>
+ <compileSourceRoots>
+
<compileSourceRoot>${project.basedir}/src/main/java24</compileSourceRoot>
+ </compileSourceRoots>
+ <release>24</release>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
Review Comment:
🔴 **Critical:** This `compile-java24` execution is unconditional. The CI
matrix for this project tests on JDK 8, 21, and 25 (via
`apache/maven-gh-actions-shared`). JDK 8 and 21 do not support `--release 24`,
so `javac` will fail with an error like `error: release version 24 not
supported`.
This execution must be gated behind a JDK-version-activated profile:
```xml
<profiles>
<profile>
<id>java24+</id>
<activation>
<jdk>[24,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<executions>
<execution>
<id>compile-java24</id>
<goals><goal>compile</goal></goals>
<phase>compile</phase>
<configuration>
<compileSourceRoots>
<compileSourceRoot>${project.basedir}/src/main/java24</compileSourceRoot>
</compileSourceRoots>
<release>24</release>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
```
The reflective fallback in `BytecodeAnalyzer.selectAnalyzer()` already
handles the case where `ClassfileClassAnalyzer` is absent — it falls back to
ASM. So the plugin JAR just won't contain the java24 class when built on older
JDKs, which is fine.
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/AbiExtractor.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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 javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.Modifier;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.TypeKind;
+import javax.lang.model.type.TypeMirror;
+
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+/**
+ * Computes ABI (Application Binary Interface) fingerprints for Java types
+ * using the {@link javax.lang.model} element API.
+ *
+ * <p>The canonical form includes the type's modifiers, name, type parameters,
+ * superclass, interfaces, and all non-private members (fields, constructors,
+ * methods, nested types). For {@code static final} fields, the compile-time
+ * constant value is included so that changes to inlined constants are
detected.
+ *
+ * <p>The fingerprint is a truncated SHA-256 hash of this canonical form.
+ * Two types have the same fingerprint if and only if their public API surfaces
+ * are identical — method body changes do not affect it.
+ *
+ * @see CompilationAnalyzer
+ */
+public class AbiExtractor {
+
+ public static String computeFingerprint(TypeElement type) {
+ return sha256(canonicalForm(type));
+ }
+
+ public static String canonicalForm(TypeElement type) {
+ var sb = new StringBuilder();
+ appendType(sb, type, 0);
+ return sb.toString();
+ }
+
+ private static void appendType(StringBuilder sb, TypeElement type, int
indent) {
+ String prefix = " ".repeat(indent);
+
+ sb.append(prefix);
+ appendModifiers(sb, type.getModifiers());
+
sb.append(type.getKind().toString().toLowerCase(java.util.Locale.ROOT)).append('
');
+ sb.append(type.getQualifiedName());
+
+ var typeParams = type.getTypeParameters();
+ if (!typeParams.isEmpty()) {
+ sb.append('<');
+ sb.append(typeParams.stream()
+ .map(tp -> {
+ var tpSb = new StringBuilder(tp.getSimpleName());
+ var bounds = tp.getBounds().stream()
+ .filter(b ->
!"java.lang.Object".equals(b.toString()))
+ .toList();
+ if (!bounds.isEmpty()) {
+ tpSb.append(" extends ");
+ tpSb.append(
+
bounds.stream().map(TypeMirror::toString).collect(Collectors.joining(" & ")));
+ }
+ return tpSb.toString();
+ })
+ .collect(Collectors.joining(", ")));
+ sb.append('>');
+ }
+ sb.append('\n');
+
+ TypeMirror superclass = type.getSuperclass();
+ if (superclass.getKind() != TypeKind.NONE &&
!"java.lang.Object".equals(superclass.toString())) {
+ sb.append(prefix).append(" extends
").append(superclass).append('\n');
+ }
+
+ for (TypeMirror iface : type.getInterfaces()) {
+ sb.append(prefix).append(" implements
").append(iface).append('\n');
+ }
+
+ var members = type.getEnclosedElements().stream()
+ .filter(e -> !e.getModifiers().contains(Modifier.PRIVATE))
+ .sorted(Comparator.<Element, Integer>comparing(e -> switch
(e.getKind()) {
+ case FIELD, ENUM_CONSTANT -> 0;
+ case CONSTRUCTOR -> 1;
+ case METHOD -> 2;
+ default -> e.getKind().isClass() ||
e.getKind().isInterface() ? 3 : 4;
+ })
+ .thenComparing(e -> e.getSimpleName().toString())
+ .thenComparing(e -> e instanceof ExecutableElement ee
+ ? ee.getParameters().stream()
+ .map(p -> p.asType().toString())
+ .collect(Collectors.joining(","))
+ : ""))
+ .toList();
+
+ for (Element member : members) {
+ if (member instanceof VariableElement ve) {
+ appendField(sb, ve, indent + 1);
+ } else if (member instanceof ExecutableElement ee) {
+ appendMethod(sb, ee, indent + 1);
+ } else if (member instanceof TypeElement te) {
+ appendType(sb, te, indent + 1);
+ }
+ }
+ }
+
+ private static void appendField(StringBuilder sb, VariableElement field,
int indent) {
+ sb.append(" ".repeat(indent));
+ appendModifiers(sb, field.getModifiers());
+ sb.append(field.asType()).append(' ');
+ sb.append(field.getSimpleName());
+ Object constValue = field.getConstantValue();
+ if (constValue != null) {
+ if (constValue instanceof String s) {
+ sb.append(" = \"").append(s).append('"');
+ } else {
+ sb.append(" = ").append(constValue);
+ }
+ }
+ sb.append('\n');
+ }
+
+ private static void appendMethod(StringBuilder sb, ExecutableElement
method, int indent) {
+ sb.append(" ".repeat(indent));
+ appendModifiers(sb, method.getModifiers());
+
+ var typeParams = method.getTypeParameters();
+ if (!typeParams.isEmpty()) {
+ sb.append('<');
+ sb.append(
+ typeParams.stream().map(tp ->
tp.getSimpleName().toString()).collect(Collectors.joining(", ")));
+ sb.append("> ");
+ }
+
+ if (method.getKind() != ElementKind.CONSTRUCTOR) {
+ sb.append(method.getReturnType()).append(' ');
+ }
+
+ sb.append(method.getSimpleName()).append('(');
+ sb.append(
+ method.getParameters().stream().map(p ->
p.asType().toString()).collect(Collectors.joining(", ")));
+ sb.append(')');
+
+ var thrown = method.getThrownTypes();
+ if (!thrown.isEmpty()) {
+ sb.append(" throws ");
+
sb.append(thrown.stream().map(TypeMirror::toString).collect(Collectors.joining(",
")));
+ }
+ sb.append('\n');
+ }
+
+ private static void appendModifiers(StringBuilder sb, Set<Modifier>
modifiers) {
+ for (Modifier m : List.of(
+ Modifier.PUBLIC,
+ Modifier.PROTECTED,
+ Modifier.ABSTRACT,
+ Modifier.STATIC,
+ Modifier.FINAL,
+ Modifier.SYNCHRONIZED,
+ Modifier.NATIVE,
+ Modifier.STRICTFP,
+ Modifier.DEFAULT,
+ Modifier.SEALED,
+ Modifier.NON_SEALED)) {
+ if (modifiers.contains(m)) {
+ sb.append(m).append(' ');
+ }
+ }
+ }
+
+ private static String sha256(String input) {
+ try {
+ var md = MessageDigest.getInstance("SHA-256");
+ byte[] hash = md.digest(input.getBytes(StandardCharsets.UTF_8));
+ var hex = new StringBuilder();
+ for (byte b : hash) {
+ hex.append(String.format("%02x", b));
+ }
+ return hex.substring(0, 16);
+ } catch (java.security.NoSuchAlgorithmException e) {
+ throw new RuntimeException(e);
+ }
+ }
+}
Review Comment:
⚠️ **Duplication:** This `sha256()` method is functionally identical to
`Sha256.hash()` — same algorithm, same 16-char truncation. Use the shared
utility instead:
```suggestion
private static String sha256(String input) {
return Sha256.hash(input);
}
```
Or better yet, just call `Sha256.hash()` directly at the call site (line 55)
and remove this method entirely.
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java:
##########
@@ -0,0 +1,566 @@
+/*
+ * 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.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+
+import com.sun.source.util.JavacTask;
+
+/**
+ * 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 collects analysis data during compilation. Typical usage:
+ *
+ * <pre>{@code
+ * var abi = new AbiIncrementalBuild(outputDir);
+ * abi.setClasspathEntries(classpath);
+ * abi.setReactorModulePaths(reactorModules);
+ *
+ * Set<Path> toCompile = abi.initialize(allSourceFiles);
+ *
+ * while (!toCompile.isEmpty()) {
+ * JavacTask task = (JavacTask) compiler.getTask(..., toCompile, ...);
+ * abi.attachTo(task);
+ * if (!task.call()) break;
+ * toCompile = abi.processRound();
+ * }
+ *
+ * abi.finish();
+ * }</pre>
+ *
+ * <p>The engine persists its state as {@code .incremental-state} and writes
+ * an {@link AbiManifest} ({@code .abi-fingerprints}) in the output directory
+ * for downstream reactor modules.
+ *
+ * @see CompilationAnalyzer
+ * @see IncrementalState
+ */
+public class AbiIncrementalBuild {
+
+ 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 CompilationAnalyzer currentAnalyzer;
+ private boolean fullBuild;
+ private int totalSources;
+
+ public AbiIncrementalBuild(Path outputDir) {
+ this.outputDir = outputDir;
+ this.buildDir = outputDir.getParent() != null ? outputDir.getParent()
: outputDir;
+ this.stateFile = buildDir.resolve(".incremental-state");
+ }
+
+ /**
+ * Sets classpath entries for cross-module ABI tracking. Directory entries
+ * are checked for {@link AbiManifest} files; JAR entries use bytecode
+ * analysis as fallback.
+ */
+ public void setClasspathEntries(List<Path> entries) {
+ this.classpathEntries = entries;
+ }
+
+ /**
+ * Marks specific classpath entries as reactor modules. These are always
+ * checked for ABI changes (via manifest or bytecode).
+ */
+ public void setReactorModulePaths(Set<Path> paths) {
+ this.reactorModulePaths = paths;
+ }
+
+ /**
+ * Sets the annotation processor classpath for processor classification.
+ * Entries are scanned for {@code
META-INF/javaci/incremental.annotation.processors}
+ * and {@code META-INF/gradle/incremental.annotation.processors} to
determine
+ * whether each processor is {@link ProcessorType#ISOLATING},
+ * {@link ProcessorType#AGGREGATING}, or {@link ProcessorType#UNKNOWN}.
+ */
+ public void setProcessorPath(List<Path> processorPath) {
+ this.processorPath = processorPath;
+ this.processorClassification = new
ProcessorClassification(processorPath);
+ }
+
+ /**
+ * Initializes the incremental build by scanning source files and comparing
+ * against the previous build's state.
+ *
+ * @param allSourceFiles all source files in this module
+ * @return the set of files that need compilation (may be all files for a
+ * full build, a subset for incremental, or empty if up-to-date)
+ */
+ public Set<Path> initialize(List<Path> allSourceFiles) throws IOException {
+ Files.createDirectories(outputDir);
+
+ this.allSourceFiles = allSourceFiles;
+ totalSources = allSourceFiles.size();
+ allCompiled = new TreeSet<>();
+ previousState = IncrementalState.load(stateFile);
+ sourceMtimes = new LinkedHashMap<>();
+ sourceHashes = hashSourceFiles(allSourceFiles, previousState,
sourceMtimes);
+
+ if (previousState == null) {
+ return initFullBuild(allSourceFiles);
+ } else {
+ return initIncrementalBuild(allSourceFiles);
+ }
+ }
+
+ /**
+ * Attaches the ABI analyzer to a javac task. Must be called before
+ * {@code task.call()} on each compilation round.
+ */
+ public void attachTo(JavacTask task) {
+ currentAnalyzer = new CompilationAnalyzer(task);
+ task.addTaskListener(currentAnalyzer);
+ }
+
+ /**
+ * Processes the results of the last compilation round. Compares new ABI
+ * fingerprints against previous values and determines whether a cascade
+ * round is needed.
+ *
+ * @return the next set of files to compile (cascade consumers), or empty
+ * if the fixpoint has been reached
+ */
+ public Set<Path> processRound() {
+ if (currentAnalyzer == null) {
+ return Set.of();
+ }
+
+ var results = currentAnalyzer.getResults();
+
+ // Detect ABI changes
+ var abiChanged = new TreeSet<String>();
+ for (var result : results.values()) {
+ String prevAbi = previousState != null ?
previousState.getAbiFingerprint(result.qualifiedName()) : null;
+ if (prevAbi == null || !prevAbi.equals(result.abiFingerprint())) {
+ abiChanged.add(result.qualifiedName());
+ }
+ }
+
+ // Update state with this round's results
+ for (var entry : sourceHashes.entrySet()) {
+ if (allCompiled.contains(entry.getKey())) {
+ state.setSourceHash(entry.getKey(), entry.getValue());
+ }
+ }
+ for (var result : results.values()) {
+ state.setType(
+ result.qualifiedName(),
+ new IncrementalState.TypeInfo(
+ result.sourceFile(),
+ result.abiFingerprint(),
+ result.signatureDeps(),
+ result.implementationDeps(),
+ result.annotationTypes()));
+ }
+
+ if (fullBuild || abiChanged.isEmpty()) {
+ return Set.of();
+ }
+
+ // Cascade: find consumers of ABI-changed types
+ var abiCascade = new TreeSet<>(abiChanged);
+ for (String type : abiChanged) {
+ expandSignatureCascade(type, state, abiCascade);
+ }
+
+ var additionalFiles = new TreeSet<Path>();
+ for (String cascadedType : abiCascade) {
+ for (String consumer : state.getAllConsumers(cascadedType)) {
+ String sf = state.sourceFileFor(consumer);
+ if (sf != null && !allCompiled.contains(sf)) {
+ additionalFiles.add(Path.of(sf));
+ allCompiled.add(sf);
+ }
+ }
+ }
+
+ // Annotation processor cascade
+ additionalFiles.addAll(computeProcessorCascade());
+
+ return additionalFiles;
+ }
+
+ /**
+ * Finalizes the incremental build: saves state and writes the ABI manifest
+ * for downstream reactor modules.
+ */
+ public void finish() throws IOException {
+ if (state == null) {
+ return;
+ }
+
+ // Resolve and store external fingerprints
+ Set<String> externalDeps = state.getExternalDependencies();
+ if (!externalDeps.isEmpty()) {
+ var resolver = createResolver();
+ state.setExternalFingerprints(resolver.resolve(externalDeps));
+
state.setClasspathIdentities(resolver.computeCurrentJarIdentities());
+ }
+
+ // Persist source mtimes for the next build's mtime-first optimization
+ for (var entry : sourceMtimes.entrySet()) {
+ state.setSourceMtime(entry.getKey(), entry.getValue());
+ }
+
+ state.save(stateFile);
+ AbiManifest.write(buildDir.resolve(AbiManifest.FILENAME),
state.getAllAbiFingerprints());
+ }
+
+ /**
+ * Returns whether this was a full build (no previous state).
+ */
+ public boolean isFullBuild() {
+ return fullBuild;
+ }
+
+ /**
+ * Returns the total number of files compiled across all rounds.
+ */
+ public int compiledCount() {
+ return allCompiled.size();
+ }
+
+ /**
+ * Returns the total number of files that were unchanged.
+ */
+ public int unchangedCount() {
+ return totalSources - allCompiled.size();
+ }
+
+ // --- Initialization ---
+
+ private Set<Path> initFullBuild(List<Path> allSourceFiles) {
+ fullBuild = true;
+ state = IncrementalState.from(sourceHashes, Map.of());
+
+ var files = new TreeSet<Path>();
+ for (Path f : allSourceFiles) {
+ files.add(f);
+ allCompiled.add(f.toString());
+ }
+ return files;
+ }
+
+ private Set<Path> initIncrementalBuild(List<Path> allSourceFiles) {
+ fullBuild = false;
+ state = previousState.copy();
+
+ // Detect source changes
+ var changedFiles = new TreeSet<String>();
+ var newFiles = new TreeSet<String>();
+ var deletedFiles = new
TreeSet<>(previousState.getSourceHashes().keySet());
+
+ for (var entry : sourceHashes.entrySet()) {
+ String path = entry.getKey();
+ String hash = entry.getValue();
+ deletedFiles.remove(path);
+
+ String previousHash = previousState.getSourceHash(path);
+ if (previousHash == null) {
+ newFiles.add(path);
+ } else if (!hash.equals(previousHash)) {
+ changedFiles.add(path);
+ }
+ }
+
+ // Check external ABI changes
+ Set<String> externallyInvalidated = checkExternalAbiChanges();
+
+ if (changedFiles.isEmpty() && newFiles.isEmpty() &&
deletedFiles.isEmpty() && externallyInvalidated.isEmpty()) {
+ return Set.of();
+ }
+
+ // Build initial recompilation set
+ var toRecompile = new TreeSet<String>();
+ toRecompile.addAll(changedFiles);
+ toRecompile.addAll(newFiles);
+ toRecompile.addAll(externallyInvalidated);
+
+ // Consumers of deleted types
+ for (String deleted : deletedFiles) {
+ for (String type : previousState.getTypesFromSource(deleted)) {
+ for (String consumer : previousState.getAllConsumers(type)) {
+ String sf = previousState.sourceFileFor(consumer);
+ if (sf != null) {
+ toRecompile.add(sf);
+ }
+ }
+ deleteClassFile(type);
+ }
+ state.removeSource(deleted);
+ }
+
+ // Clear stale type entries for files about to be recompiled —
+ // handles cases where a source file previously defined multiple types
+ // but now defines fewer
+ for (String sourceFile : toRecompile) {
+ state.removeTypesForSource(sourceFile);
+ }
+
+ allCompiled.addAll(toRecompile);
+ var result = new TreeSet<Path>();
+ for (String s : toRecompile) {
+ result.add(Path.of(s));
+ }
+ return result;
+ }
+
+ // --- Annotation processor handling ---
+
+ /**
+ * Determines additional files to compile based on annotation processor
classification.
+ * Called during incremental builds when annotated sources are in the
compile set.
+ *
+ * <ul>
+ * <li>ISOLATING: no extra files needed (default, current behavior
works)</li>
+ * <li>AGGREGATING: all sources carrying the processor's trigger
annotations</li>
+ * <li>UNKNOWN: all sources (conservative full rebuild)</li>
+ * </ul>
+ */
+ private Set<Path> computeProcessorCascade() {
+ if (processorClassification == null) {
+ return Set.of();
+ }
+
+ // Collect annotation types from types we just compiled
+ var compiledAnnotations = new TreeSet<String>();
+ for (var entry : state.getTypes().entrySet()) {
+ if (allCompiled.contains(state.sourceFileFor(entry.getKey()))) {
+ compiledAnnotations.addAll(entry.getValue().annotationTypes());
+ }
+ }
+
+ if (compiledAnnotations.isEmpty()) {
+ return Set.of();
+ }
+
+ // Check if any compiled annotation triggers an AGGREGATING or UNKNOWN
processor
+ boolean hasUnknown = false;
+ boolean hasAggregating = false;
+
+ // Use worst-case classification from the processor path
+ var allAnnotations = state.getAllAnnotationTypes();
+ if (allAnnotations.isEmpty()) {
+ return Set.of();
+ }
+
+ // Check if any processors on the path are UNKNOWN or AGGREGATING
+ var classificationMap = processorClassification.getClassifications();
+ for (var entry : classificationMap.entrySet()) {
+ if (entry.getValue() == ProcessorType.UNKNOWN) {
+ hasUnknown = true;
+ } else if (entry.getValue() == ProcessorType.AGGREGATING) {
+ hasAggregating = true;
+ }
+ }
+
+ // If no processors are classified at all but processor path is set,
+ // we can't know what annotations they handle — conservative approach
+ if (classificationMap.isEmpty() && processorPath != null &&
!processorPath.isEmpty()) {
+ hasUnknown = true;
+ }
+
+ var additionalFiles = new TreeSet<Path>();
+
+ if (hasUnknown) {
+ // UNKNOWN: recompile all source files
+ for (Path sf : allSourceFiles) {
+ if (!allCompiled.contains(sf.toString())) {
+ additionalFiles.add(sf);
+ allCompiled.add(sf.toString());
+ }
+ }
+ } else if (hasAggregating) {
+ // AGGREGATING: recompile all annotated sources
+ Set<String> annotatedFiles =
state.getSourceFilesWithAnnotations(allAnnotations);
+ for (String sf : annotatedFiles) {
+ if (!allCompiled.contains(sf)) {
+ additionalFiles.add(Path.of(sf));
+ allCompiled.add(sf);
+ }
+ }
+ }
+ // ISOLATING: no extra files needed
+
+ return additionalFiles;
+ }
+
+ // --- External ABI tracking ---
+
+ private Set<String> checkExternalAbiChanges() {
+ var invalidated = new TreeSet<String>();
+ Set<String> externalDeps = state.getExternalDependencies();
+ if (externalDeps.isEmpty()) {
+ return invalidated;
+ }
+
+ var resolver = createResolver();
+ Map<String, String> currentFingerprints =
resolver.resolve(externalDeps);
+ Map<String, String> storedFingerprints =
state.getExternalFingerprints();
+
+ var changedExternalTypes = new TreeSet<String>();
+ for (var entry : currentFingerprints.entrySet()) {
+ String stored = storedFingerprints.get(entry.getKey());
+ if (stored == null || !stored.equals(entry.getValue())) {
+ changedExternalTypes.add(entry.getKey());
+ }
+ }
+
+ if (!changedExternalTypes.isEmpty()) {
+ for (String changedType : changedExternalTypes) {
+ for (String consumer : state.getAllConsumers(changedType)) {
+ String sf = state.sourceFileFor(consumer);
+ if (sf != null) {
+ invalidated.add(sf);
+ }
+ }
+ }
+ }
+
+ state.setExternalFingerprints(currentFingerprints);
+ state.setClasspathIdentities(resolver.computeCurrentJarIdentities());
+ return invalidated;
+ }
+
+ private ExternalAbiResolver createResolver() {
+ var resolver =
+ new ExternalAbiResolver(classpathEntries != null ?
classpathEntries : List.of(), reactorModulePaths);
+ if (previousState != null) {
+ resolver.setCachedState(previousState.getExternalFingerprints(),
previousState.getClasspathIdentities());
+ }
+ return resolver;
+ }
+
+ // --- Utility ---
+
+ private void expandSignatureCascade(String type, IncrementalState state,
Set<String> result) {
+ for (String consumer : state.getSignatureConsumers(type)) {
+ if (result.add(consumer)) {
+ expandSignatureCascade(consumer, state, result);
+ }
+ }
+ }
+
+ // TODO: inner classes use $ in file names (Foo$Bar.class) but qualified
names use dots.
+ // javac's ANALYZE fires for top-level types, so inner class names should
not appear here,
+ // but this needs verification for nested/local class edge cases.
+ private void deleteClassFile(String qualifiedName) {
+ Path classFile = outputDir.resolve(qualifiedName.replace('.', '/') +
".class");
+ try {
+ Files.deleteIfExists(classFile);
+ // Also clean up inner/nested class files (Foo$Bar.class,
Foo$Bar$Baz.class, etc.)
+ // These are generated by javac alongside the top-level class file.
+ Path classDir = classFile.getParent();
+ String simplePrefix =
classFile.getFileName().toString().replace(".class", "$");
+ if (Files.isDirectory(classDir)) {
+ try (var stream = Files.list(classDir)) {
+ stream.filter(p ->
p.getFileName().toString().startsWith(simplePrefix)
+ &&
p.getFileName().toString().endsWith(".class"))
+ .forEach(p -> {
+ try {
+ Files.deleteIfExists(p);
+ } catch (IOException ex) {
+ // Best effort — stale inner class files
are harmless
+ }
+ });
+ }
+ }
+ } catch (IOException e) {
+ throw new UncheckedIOException("Failed to delete stale class file:
" + classFile, e);
+ }
+ }
+
+ /**
+ * Computes SHA-256 hashes for the given source files, using an mtime-first
+ * short-circuit: if a file's last-modified time matches the value stored
in
+ * {@code prev}, its previously stored hash is reused without reading the
+ * file's content. This avoids redundant I/O on the common no-change case,
+ * particularly beneficial for large source trees.
+ *
+ * @param files the source files to process
+ * @param prev previous build's state (may be {@code null})
+ * @param mtimes output map populated with each file's observed mtime
(millis)
+ * @return map from file path string to SHA-256 content hash
+ */
+ private static Map<String, String> hashSourceFiles(
+ List<Path> files, IncrementalState prev, Map<String, Long> mtimes)
throws IOException {
+ var hashes = new LinkedHashMap<String, String>();
+ for (Path file : files) {
+ String path = file.toString();
+ BasicFileAttributes attrs = Files.readAttributes(file,
BasicFileAttributes.class);
+ long mtime = attrs.lastModifiedTime().toMillis();
+ mtimes.put(path, mtime);
+
+ if (prev != null) {
+ var storedMtime = prev.getSourceMtime(path);
+ if (storedMtime.isPresent() && storedMtime.getAsLong() ==
mtime) {
+ String storedHash = prev.getSourceHash(path);
+ if (storedHash != null) {
+ // mtime unchanged — reuse stored hash, skip reading
file content
+ hashes.put(path, storedHash);
+ continue;
+ }
+ }
+ }
+
+ hashes.put(path, sha256(Files.readAllBytes(file)));
+ }
+ return hashes;
+ }
+
+ private static String sha256(byte[] content) {
Review Comment:
⚠️ **Same duplication:** This `sha256(byte[])` method also duplicates the
hashing logic. The `Sha256` utility class only has `hash(String)`, but this
method hashes `byte[]` and returns the full 64-char hex (not truncated to 16
chars like `Sha256.hash()`). The inconsistency is intentional here (source
content hashes vs ABI fingerprints), but it would be cleaner to centralize both
variants in `Sha256` with clear naming:
- `Sha256.fingerprint(String)` → 16-char truncated (for ABI)
- `Sha256.digest(byte[])` → full hex (for content hashing)
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java:
##########
@@ -0,0 +1,377 @@
+/*
+ * 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.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.OptionalLong;
+import java.util.Set;
+import java.util.TreeSet;
+
+/**
+ * Persistent state for incremental compilation, storing per-source-file
content
+ * hashes and per-type metadata (ABI fingerprint, signature dependencies, and
+ * implementation dependencies).
+ *
+ * <p>Serialized as a compact binary format via {@link
java.io.DataOutputStream}
+ * and stored alongside the class output as {@code .incremental-state}. The
state
+ * enables the incremental engine to detect which files changed, whether their
+ * ABI is affected, and which consumers need recompilation.
+ *
+ * <p>Consumer lookup methods ({@link #getSignatureConsumers},
+ * {@link #getImplementationConsumers}, {@link #getAllConsumers}) support the
+ * cascade logic: signature consumers are followed transitively (their ABI may
+ * change), while implementation consumers are recompiled directly but do not
+ * cascade further.
+ *
+ * @see AbiIncrementalBuild
+ */
+public class IncrementalState {
+
+ private static final int VERSION = 5;
+
+ private final Map<String, String> sourceHashes = new LinkedHashMap<>();
+ private final Map<String, Long> sourceMtimes = new LinkedHashMap<>();
+ private final Map<String, TypeInfo> types = new LinkedHashMap<>();
+ private final Map<String, String> externalFingerprints = new
LinkedHashMap<>();
+ private final Map<String, String> classpathIdentities = new
LinkedHashMap<>();
+
+ public record TypeInfo(
+ String sourceFile,
+ String abiFingerprint,
+ Set<String> signatureDeps,
+ Set<String> implementationDeps,
+ Set<String> annotationTypes) {}
+
+ public String getSourceHash(String path) {
+ return sourceHashes.get(path);
+ }
+
+ public Map<String, String> getSourceHashes() {
+ return Collections.unmodifiableMap(sourceHashes);
+ }
+
+ public TypeInfo getType(String qualifiedName) {
+ return types.get(qualifiedName);
+ }
+
+ public Map<String, TypeInfo> getTypes() {
+ return Collections.unmodifiableMap(types);
+ }
+
+ public String getAbiFingerprint(String qualifiedName) {
+ TypeInfo info = types.get(qualifiedName);
+ return info != null ? info.abiFingerprint() : null;
+ }
+
+ public void setSourceHash(String path, String hash) {
+ sourceHashes.put(path, hash);
+ }
+
+ /**
+ * Returns the last-modified time (in milliseconds) recorded for the given
+ * source file in the previous build, or {@link OptionalLong#empty()} if
not
+ * stored (e.g. first build or state format upgrade).
+ */
+ public OptionalLong getSourceMtime(String path) {
+ Long mtime = sourceMtimes.get(path);
+ return mtime != null ? OptionalLong.of(mtime) : OptionalLong.empty();
+ }
+
+ public void setSourceMtime(String path, long mtime) {
+ sourceMtimes.put(path, mtime);
+ }
+
+ public void setType(String qualifiedName, TypeInfo info) {
+ types.put(qualifiedName, info);
+ }
+
+ public void removeSource(String path) {
+ sourceHashes.remove(path);
+ removeTypesForSource(path);
+ }
+
+ /**
+ * Removes all type entries associated with the given source file.
+ * Used to clear stale types before recompilation — a source file
+ * that previously defined types A and B but now only defines A
+ * would otherwise retain phantom type B in the state.
+ */
+ public void removeTypesForSource(String sourceFile) {
+ types.entrySet().removeIf(e ->
e.getValue().sourceFile().equals(sourceFile));
+ }
+
+ public List<String> getTypesFromSource(String sourceFile) {
+ return types.entrySet().stream()
+ .filter(e -> e.getValue().sourceFile().equals(sourceFile))
+ .map(Map.Entry::getKey)
+ .toList();
+ }
+
+ public Set<String> getSignatureConsumers(String type) {
+ var result = new TreeSet<String>();
+ for (var entry : types.entrySet()) {
+ if (entry.getValue().signatureDeps().contains(type)) {
+ result.add(entry.getKey());
+ }
+ }
+ return result;
+ }
+
+ public Set<String> getImplementationConsumers(String type) {
+ var result = new TreeSet<String>();
+ for (var entry : types.entrySet()) {
+ if (entry.getValue().implementationDeps().contains(type)) {
+ result.add(entry.getKey());
+ }
+ }
+ return result;
+ }
+
+ public Set<String> getAllConsumers(String type) {
+ var result = getSignatureConsumers(type);
+ result.addAll(getImplementationConsumers(type));
+ return result;
+ }
+
+ public String sourceFileFor(String typeName) {
+ TypeInfo info = types.get(typeName);
+ return info != null ? info.sourceFile() : null;
+ }
+
+ public Map<String, String> getExternalFingerprints() {
+ return Collections.unmodifiableMap(externalFingerprints);
+ }
+
Review Comment:
💡 **Shallow copy of mutable sets in `copy()`:** `TypeInfo` is a record with
`Set<String>` fields. When `copy()` does `copy.types.putAll(this.types)`, the
`TypeInfo` instances are shared between original and copy since records are
immutable references but their contained sets may be mutable `TreeSet`s.
If any code path ever calls `removeTypesForSource()` on the copy and then
modifies the sets of a remaining `TypeInfo`, the original's data would be
corrupted. The current code doesn't hit this because `TypeInfo` sets are never
mutated after construction, but it's a latent correctness hazard.
Consider either:
1. Making `TypeInfo` sets defensive copies in the constructor:
`Set.copyOf(signatureDeps)`
2. Or deep-copying in `copy()`
##########
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 javaci.</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 javaci.</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);
+ }
+
+ /**
+ * Resolves ABI fingerprints for the given set of type names.
+ *
+ * @param typeNames fully qualified type names to resolve
+ * @return map from type name to ABI fingerprint (types not found on the
+ * classpath are omitted)
+ */
+ public Map<String, String> resolve(Set<String> typeNames) {
+ if (typeNames.isEmpty()) {
+ return Map.of();
+ }
+
+ var result = new HashMap<String, String>();
+ var remaining = new LinkedHashSet<>(typeNames);
+
+ // Strategy 1 & 2: read from manifests in directory classpath entries
+ resolveFromManifests(remaining, result);
+ remaining.removeAll(result.keySet());
+
+ // Strategy 3: compute from bytecode for anything still unresolved
+ if (!remaining.isEmpty()) {
+ resolveFromBytecode(remaining, result);
+ }
+
+ return result;
+ }
+
+ /**
+ * Computes identity strings for all JAR entries on the classpath.
+ * The identity is {@code size:lastModifiedMillis}.
+ */
+ public Map<String, String> computeCurrentJarIdentities() {
+ var identities = new LinkedHashMap<String, String>();
+ for (Path entry : classpathEntries) {
+ if (isJarFile(entry) && Files.exists(entry)) {
+ String id = jarIdentity(entry);
+ if (id != null) {
+ identities.put(entry.toString(), id);
+ }
+ }
+ }
+ return identities;
+ }
+
+ private void resolveFromManifests(Set<String> typeNames, Map<String,
String> result) {
+ if (manifestCache == null) {
+ manifestCache = new HashMap<>();
+ for (Path entry : classpathEntries) {
+ if (Files.isDirectory(entry)) {
+ // Manifest is in the build directory (parent of classes
dir)
+ Path parent = entry.getParent();
+ if (parent != null) {
+ Path manifestFile =
parent.resolve(AbiManifest.FILENAME);
+ Map<String, String> manifest =
AbiManifest.read(manifestFile);
+ if (!manifest.isEmpty()) {
+ manifestCache.put(entry, manifest);
Review Comment:
💡 **JAR identity based on size+mtime:** This caching strategy can produce
false positives when:
1. A JAR is rebuilt deterministically (same content, same mtime via
`SOURCE_DATE_EPOCH` or similar)
2. A JAR is replaced atomically and the FS rounds mtime to the same second
Both are edge cases. For most Maven builds this is fine because JAR rebuilds
update mtimes. Just noting that an alternative like a content hash (MD5/SHA-1
of the JAR header) would be more robust if this ever becomes an issue.
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/DependencyScanner.java:
##########
@@ -0,0 +1,198 @@
+/*
+ * 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 javax.lang.model.element.Element;
+import javax.lang.model.element.ExecutableElement;
+import javax.lang.model.element.Modifier;
+import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.VariableElement;
+
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.ImportTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.VariableTree;
+import com.sun.source.util.TreePath;
+import com.sun.source.util.TreePathScanner;
+import com.sun.source.util.Trees;
+
+/**
+ * Walks the AST after ANALYZE to extract type dependencies, classifying each
+ * as signature-level (appears in public API) or implementation-level
(body-only).
+ */
+public class DependencyScanner extends TreePathScanner<Void, Void> {
+
+ private final Trees trees;
+ private final Set<String> signatureDeps = new TreeSet<>();
+ private final Set<String> implementationDeps = new TreeSet<>();
+ private int bodyDepth = 0;
+ private boolean inPrivateMember = false;
+
+ public DependencyScanner(Trees trees) {
+ this.trees = trees;
+ }
+
+ public Set<String> getSignatureDeps() {
+ return Collections.unmodifiableSet(signatureDeps);
+ }
+
+ public Set<String> getImplementationDeps() {
+ return Collections.unmodifiableSet(implementationDeps);
+ }
+
+ @Override
+ public Void visitImport(ImportTree node, Void p) {
+ // Skip imports — they don't create dependencies, the actual usage
sites do.
+ // Without this, all imported types appear as signature deps
regardless of
+ // where they're actually used.
+ return null;
+ }
+
+ @Override
+ public Void visitMethod(MethodTree node, Void p) {
+ Element el = resolveElement(getCurrentPath());
+ boolean wasPrivate = inPrivateMember;
+ if (el != null && el.getModifiers().contains(Modifier.PRIVATE)) {
+ inPrivateMember = true;
+ }
+
+ // Signature parts: return type, parameters, throws, type params,
annotations
+ scan(node.getModifiers(), p);
+ scan(node.getTypeParameters(), p);
+ scan(node.getReturnType(), p);
+ scan(node.getParameters(), p);
+ scan(node.getThrows(), p);
+ scan(node.getDefaultValue(), p);
+
+ // Body: implementation context
+ if (node.getBody() != null) {
+ bodyDepth++;
+ scan(node.getBody(), p);
+ bodyDepth--;
+ }
+
+ inPrivateMember = wasPrivate;
+ return null;
Review Comment:
💡 **Lambda handling gap:** `visitMethod` correctly increments `bodyDepth`
for method bodies. But `LambdaExpressionTree` is not overridden — lambda bodies
are traversed by the default scanner without adjusting `bodyDepth`.
This matters when a lambda appears in a signature-level position that the
scanner hasn't entered a body for yet. For example, a constant field
initializer with a lambda:
```java
public static final Comparator<Foo> CMP = (a, b) -> new Helper().compare(a,
b);
```
Here `Helper` would be classified as an impl dep (field initializers bump
bodyDepth via `visitVariable`), so the lambda case is partially covered. But in
annotation values or other edge cases, the missing override could misclassify.
This is conservative (over-cascading, not under-cascading) so it won't cause
missed recompilations, but adding:
```java
@Override
public Void visitLambdaExpression(LambdaExpressionTree node, Void p) {
bodyDepth++;
var result = super.visitLambdaExpression(node, p);
bodyDepth--;
return result;
}
```
would make the classification more precise.
--
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]
