gnodet-bot commented on code in PR #1155: URL: https://github.com/apache/maven-compiler-plugin/pull/1155#discussion_r4186108144
########## src/it/abi-incremental-cascade/pom.xml: ########## @@ -0,0 +1,67 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <groupId>org.apache.maven.plugins.compiler.it</groupId> + <artifactId>abi-incremental-cascade</artifactId> + <version>1.0-SNAPSHOT</version> + + <description>IT: ABI change should cascade to consumers (recompile Service when Model API changes).</description> + + <properties> + <maven.compiler.release>17</maven.compiler.release> + <maven.compiler.incrementalStrategy>graph</maven.compiler.incrementalStrategy> Review Comment: π Same as `abi-incremental-basic` β this IT is named `abi-incremental-cascade` but sets `graph`. The cascade IT's `verify.groovy` doesn't check for the ABI manifest, so it won't fail, but the IT name is misleading. If the intent is to test ABI-specific cascade behavior (sig-only cascades vs. all-dep cascades), this should also be `abi`. ########## src/it/abi-incremental-basic/pom.xml: ########## @@ -0,0 +1,66 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <groupId>org.apache.maven.plugins.compiler.it</groupId> + <artifactId>abi-incremental-basic</artifactId> + <version>1.0-SNAPSHOT</version> + + <description>IT: ABI-based incremental compilation β body-only change should only recompile the changed file.</description> + + <properties> + <maven.compiler.release>17</maven.compiler.release> + <maven.compiler.incrementalStrategy>graph</maven.compiler.incrementalStrategy> Review Comment: π΄ **Bug:** This IT sets `incrementalStrategy=graph`, but `verify.groovy` (lines 40-44) asserts that `target/abi-fingerprints` exists. With the refactoring in `6ffe5a6`, `ToolExecutor.setAbiTracking()` is only `true` when `incrementalStrategy=abi`. When set to `graph`, `finish()` skips the `AbiManifest.write()` call, so the manifest won't exist and the IT assertion will fail. Either: 1. Change this to `abi` so the IT actually tests ABI-fingerprint tracking (matching the IT name `abi-incremental-basic`), or 2. Remove the manifest assertions from `verify.groovy` if the intent is to test graph-only behavior here. Option 1 seems correct given the IT name and description: ```suggestion <maven.compiler.incrementalStrategy>abi</maven.compiler.incrementalStrategy> ``` ########## src/main/java/org/apache/maven/plugin/compiler/incremental/IncrementalState.java: ########## @@ -0,0 +1,621 @@ +/* + * 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. + * + * <p>Two levels of type metadata are supported, corresponding to the two incremental + * strategies: + * <ul> + * <li>{@link GraphTypeInfo} β used by the {@code graph} strategy. Stores class-level + * dependency references (all types referenced in the classfile constant pool), without + * distinguishing signature from implementation dependencies, and without ABI fingerprints.</li> + * <li>{@link AbiTypeInfo} β used by the {@code abi} strategy. Extends {@code GraphTypeInfo} + * with fine-grained {@code signatureDeps}/{@code implementationDeps} sets and an ABI + * fingerprint, enabling more precise cascade decisions and cross-module manifest writing.</li> + * </ul> + * + * <p>Serialized as a compact binary format via {@link DataOutputStream} and stored alongside + * the class output as {@code .incremental-state}. A one-byte tag discriminates the two + * {@code TypeInfo} variants: {@code 0} = {@code GraphTypeInfo}, {@code 1} = {@code AbiTypeInfo}. + * + * <p>Consumer lookup methods ({@link #getSignatureConsumers}, {@link #getImplementationConsumers}, + * {@link #getAllConsumers}) support the cascade logic. For {@code GraphTypeInfo}, all consumers + * are stored as "signature consumers" (single index, no distinction). For {@code AbiTypeInfo}, + * signature and implementation consumers are indexed separately. + * + * @see GraphIncrementalBuild + */ +public class IncrementalState { + + private static final int VERSION = 2; + + /** Serialization tag for {@link GraphTypeInfo}. */ + private static final byte TAG_GRAPH = 0; + + /** Serialization tag for {@link AbiTypeInfo}. */ + private static final byte TAG_ABI = 1; + + 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<>(); + /** + * For {@link GraphTypeInfo}: stores all consumers (no sig/impl distinction). + * For {@link AbiTypeInfo}: stores only signature consumers. + */ + private final Map<String, Set<String>> signatureConsumersIndex = new LinkedHashMap<>(); + /** Only populated for {@link AbiTypeInfo} entries. */ + private final Map<String, Set<String>> implementationConsumersIndex = new LinkedHashMap<>(); + private final Map<String, Set<String>> sourceToTypesIndex = new LinkedHashMap<>(); + private String configHash = ""; + + // ----------------------------------------------------------------------- + // TypeInfo sealed hierarchy + // ----------------------------------------------------------------------- + + /** + * Per-type metadata stored by the incremental engine. + * + * <p>Use {@code switch} on the concrete type to distinguish the two strategies: + * <pre>{@code + * switch (info) { + * case GraphTypeInfo g -> // class-level deps only + * case AbiTypeInfo a -> // fine-grained sig/impl deps + ABI fingerprint + * } + * }</pre> + */ + public sealed interface TypeInfo permits GraphTypeInfo, AbiTypeInfo { + /** Path to the source file that defines this type. */ + String sourceFile(); + + /** All class-level dependency references (constant pool {@code ClassEntry} names). */ + Set<String> classDeps(); + + /** Annotation types applied to this type or its members. */ + Set<String> annotationTypes(); + + /** Java module name, or empty string if non-modular or unnamed module. */ + String moduleName(); + } + + /** + * Type metadata for the {@code graph} incremental strategy. + * + * <p>Stores all class-level references without distinguishing public API (signature) + * from method-body references (implementation). No ABI fingerprint is computed. + * + * @param sourceFile path to the source file that defines this type + * @param classDeps all types referenced in the classfile (constant pool class entries), + * regardless of where the reference appears + * @param annotationTypes annotation types applied to this type or its members + * @param moduleName Java module name (empty string if non-modular or unnamed) + */ + public record GraphTypeInfo(String sourceFile, Set<String> classDeps, Set<String> annotationTypes, String moduleName) + implements TypeInfo {} + + /** + * Type metadata for the {@code abi} incremental strategy. + * + * <p>Extends {@link GraphTypeInfo} with fine-grained dependency sets and an ABI fingerprint, + * enabling more precise cascade decisions (only signature consumers cascade transitively) + * and cross-module incremental detection via {@link AbiManifest}. + * + * @param sourceFile path to the source file that defines this type + * @param classDeps all types referenced in the classfile (union of sig + impl deps) + * @param annotationTypes annotation types applied to this type or its members + * @param moduleName Java module name (empty string if non-modular or unnamed) + * @param signatureDeps types in the public API surface (extends/implements, method/field + * descriptors of non-private members, exception types, annotations) + * @param implementationDeps types referenced only in method bodies or private members + * @param abiFingerprint 16-character hex SHA-256 prefix of the ABI canonical form + */ + public record AbiTypeInfo( + String sourceFile, + Set<String> classDeps, + Set<String> annotationTypes, + String moduleName, + Set<String> signatureDeps, + Set<String> implementationDeps, + String abiFingerprint) + implements TypeInfo {} + + // ----------------------------------------------------------------------- + // Source hash accessors + // ----------------------------------------------------------------------- + + public String getSourceHash(String path) { + return sourceHashes.get(path); + } + + public Map<String, String> getSourceHashes() { + return Collections.unmodifiableMap(sourceHashes); + } + + 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); + } + + // ----------------------------------------------------------------------- + // TypeInfo accessors + // ----------------------------------------------------------------------- + + public TypeInfo getType(String qualifiedName) { + return types.get(qualifiedName); + } + + public Map<String, TypeInfo> getTypes() { + return Collections.unmodifiableMap(types); + } + + /** + * Returns the ABI fingerprint for the given type, or {@code null} if the type + * is not stored with {@link AbiTypeInfo} (i.e. the {@code graph} strategy is in use). + */ + public String getAbiFingerprint(String qualifiedName) { + TypeInfo info = types.get(qualifiedName); + return info instanceof AbiTypeInfo abi ? abi.abiFingerprint() : null; + } + + public void setType(String qualifiedName, TypeInfo info) { + TypeInfo old = types.put(qualifiedName, info); + updateInvertedIndex(qualifiedName, old, 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 -> { + if (e.getValue().sourceFile().equals(sourceFile)) { + updateInvertedIndex(e.getKey(), e.getValue(), null); + return true; + } + return false; + }); + } + + public List<String> getTypesFromSource(String sourceFile) { + Set<String> indexed = sourceToTypesIndex.get(sourceFile); + return indexed != null ? List.copyOf(indexed) : List.of(); + } + + public String sourceFileFor(String typeName) { + TypeInfo info = types.get(typeName); + return info != null ? info.sourceFile() : null; + } + + // ----------------------------------------------------------------------- + // Consumer index accessors + // ----------------------------------------------------------------------- + + /** + * Returns the set of types that have {@code type} in their signature dependencies. + * + * <p>For the {@code graph} strategy ({@link GraphTypeInfo}), this returns all consumers + * (no sig/impl distinction is made). For the {@code abi} strategy ({@link AbiTypeInfo}), + * this returns only signature consumers (those that cascade transitively). + */ + public Set<String> getSignatureConsumers(String type) { + return Collections.unmodifiableSet(signatureConsumersIndex.getOrDefault(type, Collections.emptySet())); + } + + /** + * Returns the set of types that have {@code type} in their implementation dependencies only. + * + * <p>Only populated for the {@code abi} strategy ({@link AbiTypeInfo}). Always empty for + * the {@code graph} strategy. + */ + public Set<String> getImplementationConsumers(String type) { + return Collections.unmodifiableSet(implementationConsumersIndex.getOrDefault(type, Collections.emptySet())); + } + + /** + * Returns all consumers of {@code type} (union of signature and implementation consumers). + */ + public Set<String> getAllConsumers(String type) { + var result = new TreeSet<>(getSignatureConsumers(type)); + result.addAll(getImplementationConsumers(type)); + return result; + } + + // ----------------------------------------------------------------------- + // External dependency / fingerprint accessors + // ----------------------------------------------------------------------- + + public Map<String, String> getExternalFingerprints() { + return Collections.unmodifiableMap(externalFingerprints); + } + + public void setExternalFingerprints(Map<String, String> fingerprints) { + externalFingerprints.clear(); + externalFingerprints.putAll(fingerprints); + } + + public Map<String, String> getClasspathIdentities() { + return Collections.unmodifiableMap(classpathIdentities); + } + + public void setClasspathIdentities(Map<String, String> identities) { + classpathIdentities.clear(); + classpathIdentities.putAll(identities); + } + + /** + * Returns the compilation context hash, capturing external configuration + * such as module-info-patch.maven file content. + */ + public String getConfigHash() { + return configHash; + } + + public void setConfigHash(String hash) { + this.configHash = hash != null ? hash : ""; + } + + // ----------------------------------------------------------------------- + // Aggregate queries + // ----------------------------------------------------------------------- + + /** + * Returns the source files of all types carrying any of the given annotations. + */ + public Set<String> getSourceFilesWithAnnotations(Set<String> annotationTypes) { + var result = new TreeSet<String>(); + for (TypeInfo info : types.values()) { + for (String ann : info.annotationTypes()) { + if (annotationTypes.contains(ann)) { + result.add(info.sourceFile()); + break; + } + } + } + return result; + } + + /** + * Returns all annotation types used across all types in this module. + */ + public Set<String> getAllAnnotationTypes() { + var result = new TreeSet<String>(); + for (TypeInfo info : types.values()) { + result.addAll(info.annotationTypes()); + } + return result; + } + + /** + * Returns the set of type names that appear in dependency sets but are not + * defined in this module (no {@link TypeInfo} entry). These are types from + * the classpath β other reactor modules or external libraries. + * + * <p>Uses {@link TypeInfo#classDeps()} which is available for both strategies. + */ + public Set<String> getExternalDependencies() { + var external = new TreeSet<String>(); + for (TypeInfo info : types.values()) { + for (String dep : info.classDeps()) { + if (!types.containsKey(dep)) { + external.add(dep); + } + } + } + return external; + } + + /** + * Returns the current ABI fingerprints for all types stored as {@link AbiTypeInfo}, + * suitable for writing to an {@link AbiManifest}. + * + * <p>Types stored as {@link GraphTypeInfo} (i.e. compiled with the {@code graph} strategy) + * are not included β they have no ABI fingerprint. + */ + public Map<String, String> getAllAbiFingerprints() { + var result = new LinkedHashMap<String, String>(); + for (var entry : types.entrySet()) { + if (entry.getValue() instanceof AbiTypeInfo abi) { + result.put(entry.getKey(), abi.abiFingerprint()); + } + } + return result; + } + + // ----------------------------------------------------------------------- + // Copy / factory + // ----------------------------------------------------------------------- + + public IncrementalState copy() { + var copy = new IncrementalState(); + copy.sourceHashes.putAll(this.sourceHashes); + copy.sourceMtimes.putAll(this.sourceMtimes); + copy.types.putAll(this.types); + copy.externalFingerprints.putAll(this.externalFingerprints); + copy.classpathIdentities.putAll(this.classpathIdentities); + copy.configHash = this.configHash; + copy.buildInvertedIndex(); + return copy; + } + + // ----------------------------------------------------------------------- + // Inverted index maintenance + // ----------------------------------------------------------------------- + + private void buildInvertedIndex() { + signatureConsumersIndex.clear(); + implementationConsumersIndex.clear(); + sourceToTypesIndex.clear(); + for (var entry : types.entrySet()) { + String consumer = entry.getKey(); + TypeInfo info = entry.getValue(); + indexDepsFor(consumer, null, info); + } + } + + private void updateInvertedIndex(String typeName, TypeInfo oldInfo, TypeInfo newInfo) { + // Remove old entries + if (oldInfo != null) { + Set<String> deps = depsForIndex(oldInfo); + for (String dep : deps) { + Set<String> consumers = signatureConsumersIndex.get(dep); + if (consumers != null) { + consumers.remove(typeName); + } + } + if (oldInfo instanceof AbiTypeInfo oldAbi) { + for (String dep : oldAbi.implementationDeps()) { + Set<String> consumers = implementationConsumersIndex.get(dep); + if (consumers != null) { + consumers.remove(typeName); + } + } + } + Set<String> oldSources = sourceToTypesIndex.get(oldInfo.sourceFile()); + if (oldSources != null) { + oldSources.remove(typeName); + } + } + // Add new entries + indexDepsFor(typeName, null, newInfo); + } + + /** + * Indexes dependency edges for {@code typeName} from {@code info}. + * Pass {@code null} for {@code info} to skip (no-op, used during removal). + */ + private void indexDepsFor(String typeName, Object unused, TypeInfo info) { Review Comment: π Dead parameter: `Object unused` is always passed as `null` (lines 406 and 434). Looks like a refactoring artifact β the method signature should be `indexDepsFor(String typeName, TypeInfo info)`. ```suggestion private void indexDepsFor(String typeName, TypeInfo info) { ``` ########## src/main/java/org/apache/maven/plugin/compiler/incremental/GraphIncrementalBuild.java: ########## @@ -0,0 +1,938 @@ +/* + * 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.ArrayDeque; +import java.util.ArrayList; +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; + +/** + * Dependency-graph-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 + * class-level dependency graph. Typical usage: + * + * {@snippet : + * var build = new GraphIncrementalBuild(outputDir); + * build.setProcessorPath(processorPath); + * build.setConfigHash(configHash); + * + * Set<Path> toCompile = build.initialize(allSourceFiles); + * + * while (!toCompile.isEmpty()) { + * compiler.compile(toCompile); // any compiler, any mode + * toCompile = build.processCompiledClasses(toCompile); + * } + * + * build.finish(); + * } + * + * <p>After each compilation pass, {@link #processCompiledClasses(Set)} scans + * the freshly produced {@code .class} files, updates the dependency graph, + * and returns any additional source files that must be compiled in the next + * pass (cascade due to changed classes, or newly discovered dependencies). + * The loop converges in at most 2β3 passes in practice. + * + * <p>The engine persists its state as {@code 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). + * + * @see IncrementalState + */ +public class GraphIncrementalBuild { + + /** Prefix used to distinguish module-info entries from regular type entries in the state. */ + static final String MODULE_PREFIX = BytecodeAnalyzer.MODULE_PREFIX; + + private final Path outputDir; + private final Path buildDir; + private final Path stateFile; + private List<Path> classpathEntries; + private Set<Path> reactorModulePaths; + private boolean abiTracking; + 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 Map<Path, String> outputClassIndex; + + public GraphIncrementalBuild(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. + // Include the output directory name (e.g. "classes", "test-classes") to avoid + // collisions between compile and testCompile executions. + String outputDirName = outputDir.getFileName().toString(); + Path mavenStatus = buildDir.resolve("maven-status").resolve("maven-compiler-plugin"); + this.stateFile = mavenStatus.resolve(outputDirName).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; + } + + /** + * Enables ABI tracking mode ({@code abi} strategy). When {@code true}, the + * engine computes ABI fingerprints and fine-grained {@code signatureDeps}/ + * {@code implementationDeps} for each compiled type, and writes an + * {@link AbiManifest} on {@link #finish()}. When {@code false} (default, + * {@code graph} strategy), only class-level dependency references are collected. + */ + public void setAbiTracking(boolean abiTracking) { + this.abiTracking = abiTracking; + } + + /** + * Sets the annotation processor classpath for processor classification. + * Entries are scanned for {@code META-INF/maven/compiler/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); + } + + /** + * Indicates that class files are written under module-name subdirectories + * of the output directory (MODULE_SOURCE hierarchy). When set, stored module + * names are used as path prefixes when deleting class files. + */ + public void setUseModulePrefixedPaths(boolean useModulePrefixedPaths) { + this.useModulePrefixedPaths = useModulePrefixedPaths; + } + + /** + * 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 : ""; + } + + /** + * 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) { + rebuildCause = "no previous build state"; + return initFullBuild(allSourceFiles); + } else if (!configHash.equals(previousState.getConfigHash())) { + rebuildCause = "compilation configuration changed (module-info-patch.maven or compiler options)"; + return initFullBuild(allSourceFiles); + } else { + return initIncrementalBuild(allSourceFiles); + } + } + + /** + * Processes the {@code .class} files produced by the last compilation pass. + * Scans each class file to extract the dependency graph, records changes, + * and returns any additional source files that must be compiled in the next pass. + * + * <p>The cascade logic: any compiled class whose content changed (new or modified) + * triggers recompilation of all source files that depend on it (signature or + * implementation consumers). + * + * @param compiledSourceFiles the source files that were passed to the compiler in this round + * @return additional source files to compile (may be empty when fixpoint is reached) + * @throws IOException if reading {@code .class} files fails + */ + public Set<Path> processCompiledClasses(Set<Path> compiledSourceFiles) throws IOException { + // Reset the class index so it is rebuilt fresh for each compilation round + outputClassIndex = null; + + // Scan .class files for the types produced from the compiled source files + var results = new LinkedHashMap<String, SourceFileAnalysis>(); + for (Path sourceFile : compiledSourceFiles) { + collectClassAnalyses(sourceFile, results); + } + + // All compiled types cascade to their consumers (any change triggers recompilation) + var changedTypes = new TreeSet<>(results.keySet()); + + // Update incremental 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()) { + String moduleName = useModulePrefixedPaths ? result.moduleName() : ""; + IncrementalState.TypeInfo typeInfo; + if (abiTracking) { + typeInfo = new IncrementalState.AbiTypeInfo( + result.sourceFile(), + result.classDeps(), + result.annotationTypes(), + moduleName, + result.signatureDeps(), + result.implementationDeps(), + result.abiFingerprint()); + } else { + typeInfo = new IncrementalState.GraphTypeInfo( + result.sourceFile(), result.classDeps(), result.annotationTypes(), moduleName); + } + state.setType(result.qualifiedName(), typeInfo); + } + + if (fullBuild || changedTypes.isEmpty()) { + return Set.of(); + } + + // Detect module name changes β require a full rebuild + if (previousState != null && hasModuleNameChanged(state, previousState)) { + return forceFullRebuild(); + } + + // Cascade: find all consumers of changed types (both signature and implementation) + var cascade = new TreeSet<>(changedTypes); + for (String type : changedTypes) { + expandSignatureCascade(type, state, cascade); + } + + var additionalFiles = new TreeSet<Path>(); + for (String cascadedType : cascade) { + 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; + } + + /** + * Scans the output directory for {@code .class} files produced from the given source file, + * analyzes each one with {@link BytecodeAnalyzer}, and accumulates {@link SourceFileAnalysis} + * records into {@code results}. + * + * <p>The sourceβclass mapping is reconstructed by looking up types previously recorded for + * this source file in the previous state, and by scanning the output directory for class + * files whose name prefix matches the source file's simple name. This covers both primary + * classes and inner/anonymous classes ({@code Foo$Bar.class}). + */ + private void collectClassAnalyses(Path sourceFile, Map<String, SourceFileAnalysis> results) throws IOException { + String sourceFilePath = sourceFile.toString(); + String simpleSourceName = sourceFile.getFileName().toString(); // e.g. "Model.java" + + // Map from class file path β pre-computed analysis (null if not yet analyzed). + // Reuses the analysis from the package-dir walk to avoid double BytecodeAnalyzer.analyze calls. + var classFileCache = new LinkedHashMap<Path, BytecodeAnalyzer.ClassAnalysis>(); + + // Types previously tracked for this source file β their .class files may have moved + if (previousState != null) { + for (String type : previousState.getTypesFromSource(sourceFilePath)) { + Path classFile = classFileFor(type, previousState.getType(type)); + if (Files.exists(classFile)) { + classFileCache.put(classFile, null); + } + // Also scan for inner classes (Foo$Bar.class etc.) + var innerFiles = new TreeSet<Path>(); + addInnerClassFiles(classFile, innerFiles); + for (Path inner : innerFiles) { + classFileCache.putIfAbsent(inner, null); + } + } + } + + // Walk output directory entries for this source's simple name + // (handles new types introduced in this compilation) + Path packageDir = inferPackageDir(sourceFile); + if (packageDir != null && Files.isDirectory(packageDir)) { + try (var stream = Files.list(packageDir)) { + stream.filter(p -> p.toString().endsWith(".class")).forEach(cf -> { + try { + // Match by SourceFile attribute β covers primary class, inner/anonymous + // classes (Foo$Bar.class), AND package-private secondary top-level classes + // (FooHelper in Foo.java), without cross-package simple-name collisions. + var a = BytecodeAnalyzer.analyze(cf); + if (simpleSourceName.equals(a.sourceFileName())) { + classFileCache.put(cf, a); // cache the analysis for reuse below + } + } catch (IOException e) { + // best effort β skip unreadable class files + } + }); + } + } else { + // No previous state for this source (full build or new file in incremental build): + // use the cached class index keyed by SourceFile attribute value (simple source name, + // e.g. "Foo.java"). This correctly associates package-private secondary types and + // avoids cross-package simple-name collisions. + for (var entry : getFullBuildClassIndex().entrySet()) { + if (simpleSourceName.equals(entry.getValue())) { + classFileCache.putIfAbsent(entry.getKey(), null); + } + } + } + + // Special case: module-info.class β check both flat and module-prefixed locations + if (sourceFile.getFileName().toString().equals("module-info.java")) { + // Flat layout: outputDir/module-info.class + Path flat = outputDir.resolve("module-info.class"); + if (Files.exists(flat)) { + classFileCache.putIfAbsent(flat, null); + } + // MODULE_SOURCE layout: outputDir/<moduleName>/module-info.class + if (useModulePrefixedPaths && previousState != null) { + // Find the module name from previous state + for (String type : previousState.getTypesFromSource(sourceFile.toString())) { + if (type.startsWith(MODULE_PREFIX)) { + String modName = type.substring(MODULE_PREFIX.length()); + Path modInfo = outputDir.resolve(modName).resolve("module-info.class"); + if (Files.exists(modInfo)) { + classFileCache.putIfAbsent(modInfo, null); + } + break; + } + } + } + } + + // Final pass: analyze each class file (reusing cached analysis where available) + for (var cacheEntry : classFileCache.entrySet()) { + Path classFile = cacheEntry.getKey(); + if (!Files.exists(classFile)) { + continue; + } + try { + if (abiTracking) { + // Full ABI analysis: sig/impl deps + fingerprint + var analysis = + cacheEntry.getValue() != null ? cacheEntry.getValue() : BytecodeAnalyzer.analyze(classFile); + String sourceFilePath2 = analysis.isModuleInfo() + ? sourceFilePath + : resolveSourceFile(analysis.className(), sourceFilePath); + var sfa = new SourceFileAnalysis( + analysis.className(), + sourceFilePath2, + unionDeps(analysis.signatureTypes(), analysis.implementationTypes()), + analysis.signatureTypes(), + analysis.implementationTypes(), + analysis.abiFingerprint(), + analysis.abiCanonical(), + analysis.annotationTypes(), + analysis.moduleName()); + results.put(analysis.className(), sfa); + } else { + // Graph analysis: class-level deps only (constant pool scan) + // We still need className, sourceFileName, moduleName, annotationTypes + // from a lightweight analysis β use analyzeGraph for deps but full analyze Review Comment: π Misleading comment: says "use analyzeGraph for deps" but the code at line 399 still calls `BytecodeAnalyzer.analyze(classFile)` (full analysis), not the new `analyzeGraph()`. The comment should reflect the actual implementation β the full analysis is used because `className`, `sourceFileName`, `moduleName`, and `annotationTypes` are also needed, and using a single full parse is simpler than combining a lightweight dep scan with a separate metadata extraction. ```suggestion // Graph analysis: class-level deps only (constant pool scan) // We still need className, sourceFileName, moduleName, annotationTypes // from the full analysis β one parse of the classfile via the classfile API // provides both deps (union of sig+impl) and metadata. ``` -- 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]
