[WIP] Include existing output directories on the compiler classpath

[goutamadwant]

Related to #1036.

Draft for design review. A known correctness regression blocks merging this change.

The current compiler plugin omits the output directory from the classpath during full compilation. This prevents Java sources from resolving classes written earlier by Kotlin, in both main and test compilation.

This draft adds the output directory to the nonmodular classpath and covers main/test compilation with embedded and forked javac. The four native regressions fail on unchanged production code, and the reporter's mixed Kotlin/Java project passes with the initial implementation.

Broader validation exposed stale Java bytecode reuse: after removing a source or a secondary class declaration, a full rebuild can incorrectly succeed by resolving the old class file from the output directory. The existing source-to-output cache cannot identify every javac-produced class. Reliable output ownership and cleanup are needed before enabling this behavior generally.

The annotation-processor fixture also needed to disable processing while compiling its own provider, whose service descriptor is copied before its class exists. This is restricted to the provider module; the consuming module still exercises annotation processing. Compiler 3.15 reproduces the same provider self-discovery failure.

Validation evidence:

• Initial clean verify passed 20 tests on Java 17 and Java 21.
• The reporter project passed normal and forked clean builds, with four tests each.
• The initial full integration run reported 79 passed, two failed, and two skipped. The processor fixture was subsequently corrected, but the stale-class regression remains and no passing final full integration run is claimed.
• A separate reproduction confirms incorrect success after removing a package-private class declaration from a source file that remains present.

Proposed next step: agree on compiler-output ownership tracking, including partial/full builds, failed compilations, cache migration, multiple executions and multi-release outputs, while preserving classes generated by earlier compilers.

• The draft addresses one issue.
• The description and commit explain the change and its remaining blocker.
• Native regression tests fail without the runtime change.
• Final mvn verify validation after resolving the blocker.
• mvn -Prun-its verify passes.
• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004.
• File an Apache Individual Contributor License Agreement, as applicable.

[gnodet-bot]

The direction is right — putting outputDirectory on the classpath unconditionally for non-modular builds is the correct fix for the Kotlin→Java cross-compilation problem. Two observations beyond what @desruisseaux already noted.

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

[gnodet-bot]

⚠️ Import ordering: javax.tools.ToolProvider is placed before the java.* block, which violates the project's import ordering convention (java.* → javax.* → third-party). Move it after the java.util.Map import.

Suggested change

	import java.io.File;
	import java.io.IOException;
	import java.io.UncheckedIOException;
	import java.net.URI;
	import java.nio.file.Files;
	import java.nio.file.Path;
	import java.time.Instant;
	import java.time.temporal.ChronoUnit;
	import java.util.ArrayList;
	import java.util.HashMap;
	import java.util.List;
	import java.util.Map;
	import javax.tools.ToolProvider;

[goutamadwant]

The current repository import convention places javax.* before java.* (for example, SourceDirectory and ToolExecutor), and Spotless reports this file clean. I am keeping the existing order. Let me know @gnodet

[gnodet-bot]

⚠️ Missing regression test for the stale-class problem: The test currently only covers the happy path — a class pre-compiled before the mojo runs is visible via the classpath during the mojo's own compilation. This proves outputDirectory is on the classpath, but it does not cover the correctness regression the PR description warns about.

The regression case is: MainHelper.java is compiled in one build, then MainHelper.java is deleted, then the mojo runs a full clean build — MainHelper.class should be absent from the output (or the build should fail because Consumer.java can no longer resolve it). Without this test, the known stale-class regression ships untested.

Suggested additional test (schematic):

private static void compileWithDeletedSource(AbstractCompilerMojo mojo) throws Exception {
    Path output = Files.createDirectories(mojo.getOutputDirectory());
    // Step 1: pre-populate a stale class file (simulating a previous build artifact)
    Path helper = mojo.basedir.resolve("StaleHelper.java");
    Files.writeString(helper, "public class StaleHelper {}");
    ToolProvider.getSystemJavaCompiler().run(null, null, null, "-d", output.toString(), helper.toString());
    Files.delete(helper); // source is gone — class file is stale
    // Step 2: run the mojo — Consumer.java does NOT reference StaleHelper
    Files.deleteIfExists(output.resolve("Consumer.class"));
    mojo.execute();
    // Stale class MUST be removed or the build must fail
    assertFalse("Stale StaleHelper.class should not survive a full rebuild",
            Files.isRegularFile(output.resolve("StaleHelper.class")));
}

This is exactly the scenario described in the PR body ("after removing a source or a secondary class declaration, a full rebuild can incorrectly succeed by resolving the old class file from the output directory"). If the regression is a known blocker that prevents merging, capturing it as a failing test would make the blocker explicit and trackable.

[goutamadwant]

Agreed that this needs a regression. I reproduced the deleted-source case with the existing mcompiler-21_class-remove integration test and a removed secondary top-level class case. The cleanup design still has ownership gaps, so I marked the PR Draft and described the cases needing maintainer direction here: #1127 (comment).

[goutamadwant]

I reproduced the stale-class failure in mcompiler-21_class-remove and a second case where a source keeps its primary class but removes a secondary top-level class. I've marked this PR Draft while resolving output ownership.

A cleanup based on inferred source paths misses classes when the declared package differs from the source path. Scanning class files by SourceFile also misses -g:none output and can conflate same-named sources from different roots. The none/classes modes and a missing incremental cache leave no prior-source record. I don't want to ship a cleanup that can still compile against stale bytecode or delete another source's output.

@desruisseaux, would you favor persisting exact Java output ownership across embedded and forked compilation, with an explicit policy for builds whose prior ownership is unknown?

[desruisseaux]

would you favor persisting exact Java output ownership across embedded and forked compilation, with an explicit policy for builds whose prior ownership is unknown?

Correct me if I'm wrong, but it seems to me that this is a request for better incremental compilation, isn't it? The current incremental compilation algorithms are known to have holes. This is not a regression compared to 3.x, which had the same holes.

The above AI analysis give the impression that incremental compilation was the whole point of this pull request, while I think that the point was to allow a Java project to build when some classes have been compiled by Kotlin. I suggest to stick to that goal, in which case this pull request seems about complete to me, and defer better incremental compilation to later. We would need to do that for Java first, and then in a second step see how it could be applied to Kotlin too.