[MCOMPILER-578] Track outputs across compiler executions - #1091
Open
wilx wants to merge 3 commits into
Open
Conversation
Run clean compile followed by compile and verify that Java 8 classes and the Java 11 module descriptor retain their configured class-file versions. The test exposes the incremental compilation failure before the production fix.
Record the last compiler execution that processed each mapped output in the shared plugin context. Recompile when a different execution overlaps that output while allowing repeated invocations of the same execution to remain up to date. Fixes apache#788
wilx
marked this pull request as ready for review
July 28, 2026 17:47
elharo
changed the title
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR fixes incremental-compilation correctness when multiple compiler executions share an output directory but use different release levels, by tracking which execution last produced each output within the Maven session.
Changes:
- Introduces a per-project, per-session output registry keyed by
goal@executionIdto detect overlapping outputs between executions. - Updates
AbstractCompilerMojoincremental rebuild-cause detection to force recompilation when another execution last produced an expected output. - Adds a new Invoker IT (
MCOMPILER-578) and unit tests for the output registry behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java | Computes expected outputs and triggers rebuilds when outputs overlap across executions; records processed outputs in plugin context. |
| src/main/java/org/apache/maven/plugin/compiler/CompilationOutputRegistry.java | New registry to map expected output paths to the last goal@executionId that processed them. |
| src/test/java/org/apache/maven/plugin/compiler/CompilationOutputRegistryTest.java | New unit tests validating registry semantics across executions and disjoint outputs. |
| src/it/MCOMPILER-578/pom.xml | New integration test project with multiple compile executions using different --release values into the same output dir. |
| src/it/MCOMPILER-578/verify.groovy | Validates resulting classfile major versions for Java 8 class and Java 11 module-info.class. |
| src/it/MCOMPILER-578/invoker.properties | Runs clean compile then compile to exercise incremental behavior. |
| src/it/MCOMPILER-578/src/main/java/org/example/Example.java | Minimal class compiled by different executions. |
| src/it/MCOMPILER-578/src/main/java/org/example/package-info.java | Ensures package presence without generating missing package-info class. |
| src/it/MCOMPILER-578/src/main/java/module-info.java | Module descriptor compiled with Java 11 settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1619
to
+1625
| private boolean hasPreviouslyCompiledOutput(String compilerExecution, Set<Path> outputs) { | ||
| Optional<Path> output = CompilationOutputRegistry.find(getPluginContext(), compilerExecution, outputs); | ||
| if (output.isPresent() && (getLog().isDebugEnabled() || showCompilationChanges)) { | ||
| getLog().info("\tOutput from another compiler execution: " + output.get()); | ||
| } | ||
| return output.isPresent(); | ||
| } |
Comment on lines
+67
to
+78
| for (String sourceRoot : sourceRoots) { | ||
| Path root = Paths.get(sourceRoot).toAbsolutePath().normalize(); | ||
| for (File source : sources) { | ||
| Path path = source.toPath().toAbsolutePath().normalize(); | ||
| if (path.startsWith(root)) { | ||
| for (File output : mapping.getTargetFiles( | ||
| outputDirectory, root.relativize(path).toString())) { | ||
| outputs.add(output.toPath().toAbsolutePath().normalize()); | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Is this really something people do? SMH
Comment on lines
+22
to
+24
| def exampleMajorVersion = exampleClass.bytes[7] & 0xFF | ||
| // major_version: 52 = Java 8, from the base-compile execution. | ||
| assert exampleMajorVersion == 52 |
Comment on lines
+28
to
+30
| def moduleInfoMajorVersion = moduleInfoClass.bytes[7] & 0xFF | ||
| // major_version: 55 = Java 11, from the base-modules-compile execution. | ||
| assert moduleInfoMajorVersion == 55 |
Comment on lines
+38
to
+40
| private static final Path OUTPUT = Paths.get("target/classes/Example.class"); | ||
|
|
||
| private static final Path OTHER_OUTPUT = Paths.get("target/classes/Other.class"); |
Contributor
Author
|
I have pushed a fix for all of the review comments. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keep the final class files deterministic when multiple compiler executions use different release levels and share an
output directory.
Root cause
An earlier execution can overwrite an existing class file without changing its filename.
IncrementalBuildHelpercompares input and output file names, so a later execution can incorrectly consider its source up to date even though
a different execution produced the shared output.
Changes
clean compilefollowed bycompileand verifies the Java 8 classes andJava 11 module descriptor.
goal@executionIdthat processed each mapped output in Maven's per-project plugin context.execution to remain up to date.
Validation
mvn -Prun-its verify: 77 passed, 0 failed, 5 environment-specific skips.MCOMPILER-525andMCOMPILER-578Invoker tests: passed.git diff --check: passed.Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[MCOMPILER-XXX] - Fixes bug in ApproximateQuantiles,where you replace
MCOMPILER-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean verifyto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the
Apache License Version 2.0, January 2004
In any other case, please file an
Apache Individual Contributor License Agreement.
Fixes #788