You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/09/30 13:33:15 UTC
(maven-compiler-plugin) 01/01: Address review comments on ABI incremental strategy
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch feature/abi-incremental
in repository https://gitbox.apache.org/repos/asf/maven-compiler-plugin.git
commit e583f7d38b1d59187d33a7c770bc6aaa86aee656
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Wed Sep 30 13:32:39 2026 +0000
Address review comments on ABI incremental strategy
---
.../apache/maven/plugin/compiler/ToolExecutor.java | 26 +++++--
.../maven/plugin/compiler/ToolExecutorForTest.java | 10 +++
.../plugin/compiler/incremental/AbiExtractor.java | 5 +-
.../compiler/incremental/AbiIncrementalBuild.java | 8 +--
.../compiler/incremental/DependencyScanner.java | 11 +++
.../compiler/incremental/IncrementalState.java | 84 +++++++++++++++++-----
6 files changed, 116 insertions(+), 28 deletions(-)
diff --git a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java
index f9fe55e1..043f5c2b 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java
@@ -935,11 +935,21 @@ public class ToolExecutor {
if (compiler instanceof ForkedTool) {
logger.warn("ABI incremental strategy is not supported with forked compilation."
+ " Falling back to full compilation.");
+ // ABI analysis requires in-process javac (JavacTask). Fall back to the
+ // standard incremental strategy instead of attempting ABI analysis.
+ var compilerOutput = new StringWriter();
+ applyIncrementalBuild(mojo, configuration);
+ boolean success = compile(compiler, configuration, compilerOutput);
+ String output = compilerOutput.toString();
+ if (!output.isBlank()) {
+ logger.warn(output);
+ }
+ if (!success) {
+ throw new CompilationFailureException("Compilation failed (forked, non-ABI fallback).");
+ }
+ return;
}
var abiBuild = new AbiIncrementalBuild(outputDirectory);
- if (compiler instanceof ForkedTool) {
- abiBuild.invalidate();
- }
// Collect classpath entries for cross-module tracking
var classpathPaths = new ArrayList<Path>();
@@ -983,7 +993,7 @@ public class ToolExecutor {
}
// Hash module-info-patch.maven files for config change detection
- abiBuild.setConfigHash(computeModuleInfoPatchHash());
+ abiBuild.setConfigHash(computeConfigHash(configuration));
// Collect all source file paths
var allSourcePaths = new ArrayList<Path>();
@@ -1057,8 +1067,14 @@ public class ToolExecutor {
}
}
- private String computeModuleInfoPatchHash() {
+ private String computeConfigHash(Options configuration) {
var digest = new StringBuilder();
+
+ // Include compiler options in the config hash so changes to -source/-target/-release
+ // etc. trigger a full rebuild under the ABI strategy.
+ String optionsRepr = String.join("|", configuration.options);
+ digest.append("opts:").append(optionsRepr).append(';');
+
for (SourceDirectory source : sourceDirectories) {
Path patchFile = source.root.resolve(ModuleInfoPatch.FILENAME);
if (Files.isRegularFile(patchFile)) {
diff --git a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutorForTest.java b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutorForTest.java
index 56658822..72b50b9e 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutorForTest.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutorForTest.java
@@ -401,6 +401,16 @@ class ToolExecutorForTest extends ToolExecutor {
}
}
+ /**
+ * @hidden
+ */
+ @Override
+ void compileWithAbiIncremental(JavaCompiler compiler, Options configuration, AbstractCompilerMojo mojo)
+ throws IOException {
+ addModuleOptions(configuration); // Effective only once.
+ super.compileWithAbiIncremental(compiler, configuration, mojo);
+ }
+
/**
* Returns the output directory of the main classes. This is the directory to prepend to
* the class-path or module-path before to compile the classes managed by this executor.
diff --git a/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiExtractor.java b/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiExtractor.java
index 5c100e27..ce93ae70 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiExtractor.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiExtractor.java
@@ -167,11 +167,12 @@ public class AbiExtractor {
var tpSb = new StringBuilder(tp.getSimpleName());
var bounds = tp.getBounds().stream()
.filter(b -> !"java.lang.Object".equals(b.toString()))
+ .map(TypeMirror::toString)
+ .sorted()
.toList();
if (!bounds.isEmpty()) {
tpSb.append(" extends ");
- tpSb.append(
- bounds.stream().map(TypeMirror::toString).collect(Collectors.joining(" & ")));
+ tpSb.append(String.join(" & ", bounds));
}
return tpSb.toString();
})
diff --git a/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java b/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java
index f969194f..5d995485 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/incremental/AbiIncrementalBuild.java
@@ -137,9 +137,9 @@ public class AbiIncrementalBuild {
}
/**
- * Sets a hash of compilation context configuration (e.g. module-info-patch
- * files). If this hash differs from the previous build, a full rebuild
- * is triggered.
+ * Sets a hash of compilation context configuration (e.g. compiler options
+ * and module-info-patch files). If this hash differs from the previous
+ * build, a full rebuild is triggered.
*/
public void setConfigHash(String hash) {
this.configHash = hash != null ? hash : "";
@@ -167,7 +167,7 @@ public class AbiIncrementalBuild {
rebuildCause = "no previous build state";
return initFullBuild(allSourceFiles);
} else if (!configHash.equals(previousState.getConfigHash())) {
- rebuildCause = "compilation configuration changed (module-info-patch.maven)";
+ rebuildCause = "compilation configuration changed (module-info-patch.maven or compiler options)";
return initFullBuild(allSourceFiles);
} else {
return initIncrementalBuild(allSourceFiles);
diff --git a/src/main/java/org/apache/maven/plugin/compiler/incremental/DependencyScanner.java b/src/main/java/org/apache/maven/plugin/compiler/incremental/DependencyScanner.java
index aa63f1e6..b6bba866 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/incremental/DependencyScanner.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/incremental/DependencyScanner.java
@@ -28,6 +28,7 @@ import java.util.Collections;
import java.util.Set;
import java.util.TreeSet;
+import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.IdentifierTree;
@@ -137,6 +138,16 @@ public class DependencyScanner extends TreePathScanner<Void, Void> {
return super.visitBlock(node, p);
}
+ @Override
+ public Void visitAnnotation(AnnotationTree node, Void p) {
+ // Record the annotation type itself
+ recordReference(getCurrentPath());
+ // Visit annotation arguments — this catches types used in annotation values
+ // such as @Foo(SomeEnum.VALUE) or @Foo(SomeType.class), where SomeEnum/SomeType
+ // need to be tracked as dependencies even if not in any method signature.
+ return super.visitAnnotation(node, p);
+ }
+
@Override
public Void visitIdentifier(IdentifierTree node, Void p) {
recordReference(getCurrentPath());
diff --git a/src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java b/src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java
index 15a92e9e..031ce4b1 100644
--- a/src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java
+++ b/src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java
@@ -60,6 +60,8 @@ public class IncrementalState {
private final Map<String, TypeInfo> types = new LinkedHashMap<>();
private final Map<String, String> externalFingerprints = new LinkedHashMap<>();
private final Map<String, String> classpathIdentities = new LinkedHashMap<>();
+ private final Map<String, Set<String>> signatureConsumersIndex = new LinkedHashMap<>();
+ private final Map<String, Set<String>> implementationConsumersIndex = new LinkedHashMap<>();
private String configHash = "";
public record TypeInfo(
@@ -110,7 +112,8 @@ public class IncrementalState {
}
public void setType(String qualifiedName, TypeInfo info) {
- types.put(qualifiedName, info);
+ TypeInfo old = types.put(qualifiedName, info);
+ updateInvertedIndex(qualifiedName, old, info);
}
public void removeSource(String path) {
@@ -125,7 +128,13 @@ public class IncrementalState {
* would otherwise retain phantom type B in the state.
*/
public void removeTypesForSource(String sourceFile) {
- types.entrySet().removeIf(e -> e.getValue().sourceFile().equals(sourceFile));
+ types.entrySet().removeIf(e -> {
+ if (e.getValue().sourceFile().equals(sourceFile)) {
+ updateInvertedIndex(e.getKey(), e.getValue(), null);
+ return true;
+ }
+ return false;
+ });
}
public List<String> getTypesFromSource(String sourceFile) {
@@ -136,27 +145,15 @@ public class IncrementalState {
}
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;
+ return Collections.unmodifiableSet(signatureConsumersIndex.getOrDefault(type, Collections.emptySet()));
}
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;
+ return Collections.unmodifiableSet(implementationConsumersIndex.getOrDefault(type, Collections.emptySet()));
}
public Set<String> getAllConsumers(String type) {
- var result = getSignatureConsumers(type);
+ var result = new TreeSet<>(getSignatureConsumers(type));
result.addAll(getImplementationConsumers(type));
return result;
}
@@ -265,6 +262,7 @@ public class IncrementalState {
copy.externalFingerprints.putAll(this.externalFingerprints);
copy.classpathIdentities.putAll(this.classpathIdentities);
copy.configHash = this.configHash;
+ copy.buildInvertedIndex();
return copy;
}
@@ -282,9 +280,60 @@ public class IncrementalState {
result.annotationTypes(),
result.moduleName()));
}
+ state.buildInvertedIndex();
return state;
}
+ private void buildInvertedIndex() {
+ signatureConsumersIndex.clear();
+ implementationConsumersIndex.clear();
+ for (var entry : types.entrySet()) {
+ String consumer = entry.getKey();
+ TypeInfo info = entry.getValue();
+ for (String dep : info.signatureDeps()) {
+ signatureConsumersIndex
+ .computeIfAbsent(dep, k -> new TreeSet<>())
+ .add(consumer);
+ }
+ for (String dep : info.implementationDeps()) {
+ implementationConsumersIndex
+ .computeIfAbsent(dep, k -> new TreeSet<>())
+ .add(consumer);
+ }
+ }
+ }
+
+ private void updateInvertedIndex(String typeName, TypeInfo oldInfo, TypeInfo newInfo) {
+ // Remove old entries
+ if (oldInfo != null) {
+ for (String dep : oldInfo.signatureDeps()) {
+ Set<String> consumers = signatureConsumersIndex.get(dep);
+ if (consumers != null) {
+ consumers.remove(typeName);
+ }
+ }
+ for (String dep : oldInfo.implementationDeps()) {
+ Set<String> consumers = implementationConsumersIndex.get(dep);
+ if (consumers != null) {
+ consumers.remove(typeName);
+ }
+ }
+ }
+ // Add new entries
+ if (newInfo != null) {
+ for (String dep : newInfo.signatureDeps()) {
+ signatureConsumersIndex
+ .computeIfAbsent(dep, k -> new TreeSet<>())
+ .add(typeName);
+ }
+ for (String dep : newInfo.implementationDeps()) {
+ implementationConsumersIndex
+ .computeIfAbsent(dep, k -> new TreeSet<>())
+ .add(typeName);
+ }
+ }
+ }
+
public void save(Path file) throws IOException {
Files.createDirectories(file.getParent());
try (var out = new DataOutputStream(new BufferedOutputStream(Files.newOutputStream(file)))) {
@@ -347,6 +396,7 @@ public class IncrementalState {
readStringMap(in, state.externalFingerprints);
readStringMap(in, state.classpathIdentities);
state.configHash = in.readUTF();
+ state.buildInvertedIndex();
return state;
} catch (IOException e) {
return null;