Make rebuild-on-change the default incremental compilation strategy

[gnodet]

Problem

The current default incremental compilation configuration for Java >= 23 / no annotation processor projects is options,dependencies,sources. This allows partial builds — only modified source files are passed to javac. While faster, this is unsafe: if Foo.java changes a method signature, Bar.java (which calls Foo) is not recompiled, potentially leaving a stale Bar.class that causes NoSuchMethodError at runtime.

This is the same correctness risk that made useIncrementalCompilation=false "not recommended" in 3.x.

Notably, the annotation-processor path already defaults to rebuild-on-change — so the behavior was inconsistent: safe with processors, potentially unsafe without.

Change

Add rebuild-on-change to the base default, making it options,dependencies,sources,rebuild-on-change for all projects regardless of annotation processor presence.

Users who want faster (but potentially unsafe) per-file recompilation can still opt in explicitly with:

<incrementalCompilation>options,dependencies,sources</incrementalCompilation>

Relationship to PR #1123

This was identified during the review discussion on #1123, where the Javadoc for incrementalCompilation was being clarified. The Javadoc explicitly documents the limitation: "the current compiler-plugin does not detect structural changes other than file addition or removal". Making the default safe is the natural complement to that documentation.

Fixes MCOMPILER-563 (partial — correctness concern).

[gnodet-bot]

Sound change — making the default safe-by-default is the right call, and the opt-out path is clearly documented.

One Javadoc grammar nit inline.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[desruisseaux]

Just a clarification on the following:

Notably, the annotation-processor path already defaults to rebuild-on-change — so the behavior was inconsistent: safe with processors, potentially unsafe without.

In my understanding, the rational for rebuilding all classes when annotation processing is present was not for safety. It is because annotation processor can collect information about annotated elements, and therefore may have incomplete information if we don't rebuild all. Example: a @IncludeMeInSomeList annotation and a processor which, after compilation, generates a file with all elements having this annotation. The processor needs all source files at compilation time even if these files are not impacted by any change. Even if we implemented an accurate tracking of dependencies ("IDE-style incremental build"), we would still need to rebuild all files when an annotation processor is present.

[gnodet]

Just a clarification on the following:

Notably, the annotation-processor path already defaults to rebuild-on-change — so the behavior was inconsistent: safe with processors, potentially unsafe without.

In my understanding, the rational for rebuilding all classes when annotation processing is present was not for safety. It is because annotation processor can collect information about annotated elements, and therefore may have incomplete information if we don't rebuild all. Example: a @IncludeMeInSomeList annotation and a processor which, after compilation, generates a file with all elements having this annotation. The processor needs all source files at compilation time even if these files are not impacted by any change. Even if we implemented an accurate tracking of dependencies ("IDE-style incremental build"), we would still need to rebuild all files when an annotation processor is present.

Yes, the only way is to have the annotation processors provide some meta-information about their processing and whether they are safe to not rebuild all. This is what is implemented with

maven-compiler-plugin/src/main/java/org/apache/maven/plugin/compiler/incremental/ProcessorType.java

Line 29 in 56e40dd

public enum ProcessorType {

.

However, the main point is that I think this is wrong to provide a default behaviour which is unsafe in the default basic use case.

[gnodet-bot]

The Javadoc update in this PR is partially correct but misses one case introduced by PR #1136 (merged Oct 4).

amendincrementalCompilation() has two branches, not one:

1. Uncertain case (Java < 23, proc unset, no processor path): adds REBUILD_ON_ADD + REBUILD_ON_CHANGE to the base → result matches "same as above with the addition of rebuild-on-add" ✅
2. Confirmed case (proc explicitly set to non-none, or Java ≥ 23 with explicit processor paths): calls aspects.clear() then sets NONE → incremental compilation is disabled entirely, not "same as above + additions" ❌

The updated Javadoc only describes the uncertain sub-case. It says the annotation-processor default is "same as above with the addition of rebuild-on-add", which is inaccurate when a processor is confirmed — in that branch the result is NONE, not a superset of the base config.

Suggestion: Rebase on master to pick up #1136, then expand the annotation-processor paragraph to cover both sub-cases:

• When the processor is uncertain (Java < 23, no explicit proc path): options,dependencies,sources,rebuild-on-change,rebuild-on-add
• When the processor is confirmed (explicit proc, or Java ≥ 23 with processor paths): none (full incremental disabled)

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

The Javadoc update in this PR is partially correct but misses one case introduced by PR #1136 (merged Oct 4).

amendincrementalCompilation() has two branches, not one:

1. Uncertain case (Java < 23, proc unset, no processor path): adds REBUILD_ON_ADD + REBUILD_ON_CHANGE to the base → result matches "same as above with the addition of rebuild-on-add" ✅
2. Confirmed case (proc explicitly set to non-none, or Java ≥ 23 with explicit processor paths): calls aspects.clear() then sets NONE → incremental compilation is disabled entirely, not "same as above + additions" ❌

The updated Javadoc only describes the uncertain sub-case. It says the annotation-processor default is "same as above with the addition of rebuild-on-add", which is inaccurate when a processor is confirmed — in that branch the result is NONE, not a superset of the base config.

Suggestion: Rebase on master to pick up #1136, then expand the annotation-processor paragraph to cover both sub-cases:

• When the processor is uncertain (Java < 23, no explicit proc path): options,dependencies,sources,rebuild-on-change,rebuild-on-add
• When the processor is confirmed (explicit proc, or Java ≥ 23 with processor paths): none (full incremental disabled)

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

The PR correctly changes the no-processor default to include rebuild-on-change, but the final Javadoc paragraph (unchanged from the old code) is now stale and internally inconsistent with the code above it.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

⚠️ Stale Javadoc — proc=none no longer restores per-file recompilation

This paragraph survived unchanged from the old code, but is now incorrect after the base-default change at line 645.

Previously, proc=none caused hasAnnotationProcessor() to return false, which selected the no-processor default of options,dependencies,sources — no rebuild-on-change, so per-file recompilation worked.

With this PR, the no-processor default is now options,dependencies,sources,rebuild-on-change (line 645). Setting proc=none still picks the no-processor branch, but that branch now includes rebuild-on-change, so a full rebuild is triggered on every source change.

The IT diff confirms this: incremental-proc-none-per-file was updated to explicitly set <incrementalCompilation>options,dependencies,sources</incrementalCompilation> because proc=none alone is no longer sufficient, with the IT description stating: "users who want faster (but potentially unsafe) per-file compilation must opt in explicitly."

The paragraph should be updated to remove the proc=none shortcut and only recommend the explicit incrementalCompilation property:

Suggested change

	* by setting {@link #proc} to {@code "none"} (or by setting this {@code incrementalCompilation} property explicitly).</p>
	* Projects on Java &lt; 23 that use no annotation processor can restore per-file recompilation
	* by setting this {@code incrementalCompilation} property explicitly to {@code "options,dependencies,sources"}.</p>

[gnodet-bot]

The PR correctly changes the no-processor default to include rebuild-on-change, but the final Javadoc paragraph (unchanged from the old code) is now stale and internally inconsistent with the code above it.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

⚠️ Stale Javadoc — proc=none no longer restores per-file recompilation

This paragraph survived unchanged from the old code, but is now incorrect after the base-default change at line 645.

Previously, proc=none caused hasAnnotationProcessor() to return false, which selected the no-processor default of options,dependencies,sources — no rebuild-on-change, so per-file recompilation worked.

With this PR, the no-processor default is now options,dependencies,sources,rebuild-on-change (line 645). Setting proc=none still picks the no-processor branch, but that branch now includes rebuild-on-change, so a full rebuild is triggered on every source change.

The IT diff confirms this: incremental-proc-none-per-file was updated to explicitly set <incrementalCompilation>options,dependencies,sources</incrementalCompilation> because proc=none alone is no longer sufficient, with the IT description stating: "users who want faster (but potentially unsafe) per-file compilation must opt in explicitly."

The paragraph should be updated to remove the proc=none shortcut and only recommend the explicit incrementalCompilation property:

Suggested change

	* by setting {@link #proc} to {@code "none"} (or by setting this {@code incrementalCompilation} property explicitly).</p>
	* Projects on Java &lt; 23 that use no annotation processor can restore per-file recompilation
	* by setting this {@code incrementalCompilation} property explicitly to {@code "options,dependencies,sources"}.</p>