Merge pull request #23913 Backport "Fix incremental compilation with …

…modules" to 7.6.1

Backport from #23119 to 7.6.1.

Also backport from #23251 to 7.6.1

Fixes #23224

Co-authored-by: Anže Sodja <asodja@gradle.com>

subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc

@@ -509,6 +509,26 @@ To help you understand how incremental compilation works, the following provides
 * Having a source structure that does not match the package names, while legal for compilation, might end up causing trouble in the toolchain.
     Even more if annotation processing and <<build_cache.adoc#build_cache,caching>> are involved.
 
+Additionally, Gradle might temporarily change the output location while running incremental compilation.
+This might affect some annotation processors that inspect output locations or accept file paths as arguments (e.g., `-XepExcludedPaths` in Error Prone).
+There are two options:
+
+ - Disable `incremental after failure` by setting link:{javadocPath}/org/gradle/api/tasks/compile/CompileOptions.html#getIncrementalAfterFailure--[`options.incrementalAfterFailure`] to `false` on the JavaCompile task.
+-  Keep `incremental after failure` enabled by instead passing a temporary output value to the annotation processor as a parameter so that the annotation processor is aware of the temporary path. 
+
+In the case of Error Prone, `-XepExcludedPaths` must be set. Given an existing value of `-XepExcludedPaths=.\*/build/generated/.*`, the updated value would be
+`-XepExcludedPaths=.\*/(build/generated|compileJava/compileTransaction/annotation-output)/.*`.
+
+The following table shows the mapping from default output location to temporary output location:
+
+.Compiler output to temporary output location
+|===
+| JavaCompile location property | Default output location | Temporary output location
+| `destinationDirectory` | `build/classes/java/main` | `build/tmp/<task-name>/compileTransaction/compile-output`
+| `options.generatedSourceOutputDirectory` | `build/generated/sources/annotationProcessor/java/main` | `build/tmp/<task-name>/compileTransaction/annotation-output`
+| `options.headerOutputDirectory` | `build/generated/sources/headers/java/main` | `build/tmp/<task-name>/compileTransaction/header-output`
+|===
+
 [[sec:incremental_annotation_processing]]
 == Incremental annotation processing
 

subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc

@@ -173,6 +173,13 @@ Use `implementation.bundle(libs.bundles.testing)` instead.
 
 For more information, see the updated <<jvm_test_suite_plugin.adoc#sec:declare_an_additional_test_suite, declare an additional test suite>> example in the JVM Test Suite Plugin section of the user guide and the link:{groovyDslPath}/org.gradle.api.artifacts.dsl.DependencyAdder.html[`DependencyAdder`] page in the DSL reference.
 
+==== Incremental compilation temporarily changes the output location
+
+Incremental Java and Groovy compilation may now change the compiler output location.
+This might affect some annotation processors that allow users to wire some action to file paths (e.g. `-XepExcludedPaths` in Error Prone).
+This behaviour can be disabled by setting `options.incrementalAfterFailure` to `false`.
+Please refer to the <<java_plugin#sec:incremental_compilation_known_issues, userguide section about known incremental compilation issues>> for more details.
+
 === Deprecations
 
 [[invalid_toolchain_specification_deprecation]]

subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java

@@ -262,6 +262,7 @@ private GroovyJavaJointCompileSpec createSpec() {
         spec.setSourcesRoots(sourceRoots);
         spec.setSourceFiles(stableSourcesAsFileTree);
         spec.setDestinationDir(getDestinationDirectory().getAsFile().get());
+        spec.setOriginalDestinationDir(spec.getDestinationDir());
         spec.setWorkingDir(getProjectLayout().getProjectDirectory().getAsFile());
         spec.setTempDir(getTemporaryDir());
         spec.setCompileClasspath(ImmutableList.copyOf(determineGroovyCompileClasspath()));

subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy

@@ -19,6 +19,8 @@ package org.gradle.java.compile.incremental
 import org.gradle.api.internal.cache.StringInterner
 import org.gradle.api.internal.tasks.compile.incremental.recomp.PreviousCompilationAccess
 import org.gradle.integtests.fixtures.CompiledLanguage
+import org.gradle.util.Requires
+import org.gradle.util.TestPrecondition
 import spock.lang.Issue
 
 import static org.junit.Assume.assumeFalse
@@ -222,6 +224,56 @@ abstract class BaseIncrementalCompilationAfterFailureIntegrationTest extends Abs
 
 class JavaIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrementalCompilationAfterFailureIntegrationTest {
     CompiledLanguage language = CompiledLanguage.JAVA
+
+    @Requires(TestPrecondition.JDK9_OR_LATER)
+    def "incremental compilation after failure works with modules #description"() {
+        file("impl/build.gradle") << """
+            def layout = project.layout
+            tasks.compileJava {
+                modularity.inferModulePath = $inferModulePath
+                options.compilerArgs.addAll($compileArgs)
+                doFirst {
+                    $doFirst
+                }
+            }
+        """
+        source "package a; import b.B; public class A {}",
+            "package b; public class B {}",
+            "package c; public class C {}"
+        file("src/main/${language.name}/module-info.${language.name}").text = """
+            module impl {
+                exports a;
+                exports b;
+                exports c;
+            }
+        """
+        succeeds language.compileTaskName
+        outputs.recompiledClasses("A", "B", "C", "module-info")
+
+        when:
+        outputs.snapshot {
+            source "package a; import b.B; public class A { void m1() {}; }",
+                "package b; import a.A; public class B { A m1() { return new B(); } }"
+        }
+
+        then:
+        fails language.compileTaskName
+
+        when:
+        outputs.snapshot {
+            source "package a; import b.B; public class A { void m1() {}; }",
+                "package b; import a.A; public class B { A m1() { return new A(); } }"
+        }
+        succeeds language.compileTaskName
+
+        then:
+        outputs.recompiledClasses("A", "B", "module-info")
+
+        where:
+        description                 | inferModulePath | compileArgs                                                  | doFirst
+        "with inferred module-path" | "true"          | "[]"                                                         | ""
+        "with manual module-path"   | "false"         | "[\"--module-path=\${classpath.join(File.pathSeparator)}\"]" | "classpath = layout.files()"
+    }
 }
 
 class GroovyIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrementalCompilationAfterFailureIntegrationTest {

subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy

@@ -20,6 +20,7 @@ package org.gradle.java.compile.incremental
 import org.gradle.integtests.fixtures.CompiledLanguage
 import org.gradle.util.Requires
 import org.gradle.util.TestPrecondition
+import spock.lang.Issue
 
 abstract class CrossTaskIncrementalJavaCompilationIntegrationTest extends AbstractCrossTaskIncrementalCompilationIntegrationTest {
     CompiledLanguage language = CompiledLanguage.JAVA
@@ -72,6 +73,103 @@ abstract class CrossTaskIncrementalJavaCompilationIntegrationTest extends Abstra
         result.hasErrorOutput("package a is not visible")
     }
 
+    @Requires(TestPrecondition.JDK9_OR_LATER)
+    @Issue("https://github.com/gradle/gradle/issues/23067")
+    def "incremental compilation works with modules #description"() {
+        file("impl/build.gradle") << """
+            def layout = project.layout
+            tasks.compileJava {
+                modularity.inferModulePath = $inferModulePath
+                options.compilerArgs.addAll($compileArgs)
+                doFirst {
+                    $doFirst
+                }
+            }
+        """
+        source api: ["package a; public class A {}"]
+        file("api/src/main/${language.name}/module-info.${language.name}").text = """
+            module api {
+                exports a;
+            }
+        """
+        source impl: [
+            "package b; import a.A; import c.C; public class B extends A {}",
+            "package c; public class C {}",
+            "package c.d; public class D {}"
+        ]
+        file("impl/src/main/${language.name}/module-info.${language.name}").text = """
+            module impl {
+                requires api;
+                exports b;
+                exports c;
+                exports c.d;
+            }
+        """
+        succeeds "impl:${language.compileTaskName}"
+
+        when:
+        impl.snapshot { source api: "package a; public class A { void m1() {} }" }
+
+        then:
+        succeeds "impl:${language.compileTaskName}", "--info"
+        impl.recompiledClasses("B", "module-info")
+
+        where:
+        description                 | inferModulePath | compileArgs                                                  | doFirst
+        "with inferred module-path" | "true"          | "[]"                                                         | ""
+        "with manual module-path"   | "false"         | "[\"--module-path=\${classpath.join(File.pathSeparator)}\"]" | "classpath = layout.files()"
+    }
+
+    @Requires(TestPrecondition.JDK9_OR_LATER)
+    def "incremental compilation works for multi-module project with manual module paths"() {
+        file("impl/build.gradle") << """
+            def layout = project.layout
+            tasks.compileJava {
+                modularity.inferModulePath = false
+                options.compilerArgs << "--module-path=\${classpath.join(File.pathSeparator)}" \
+                    << "--module-source-path" << file("src/main/$languageName")
+                doFirst {
+                    classpath = layout.files()
+                }
+            }
+        """
+        source api: "package a; public class A {}"
+        def moduleInfo = file("api/src/main/${language.name}/module-info.${language.name}")
+        moduleInfo.text = """
+            module api {
+                exports a;
+            }
+        """
+        file("impl/src/main/${language.name}/my.module.first/b/B.java").text = "package b; import a.A; public class B extends A {}"
+        file("impl/src/main/${language.name}/my.module.first/module-info.${language.name}").text = """
+            module my.module.first {
+                requires api;
+                exports b;
+            }
+        """
+        file("impl/src/main/${language.name}/my.module.second/c/C.java").text = "package c; import b.B; class C extends B {}"
+        file("impl/src/main/${language.name}/my.module.second/module-info.${language.name}").text = """
+            module my.module.second {
+                requires my.module.first;
+            }
+        """
+        file("impl/src/main/${language.name}/my.module.unrelated/unrelated/Unrelated.java").text = "package unrelated; class Unrelated {}"
+        file("impl/src/main/${language.name}/my.module.unrelated/module-info.${language.name}").text = """
+            module my.module.unrelated {
+                exports unrelated;
+            }
+        """
+        succeeds "impl:${language.compileTaskName}"
+
+        when:
+        impl.snapshot { source api: "package a; public class A { public void m1() {} }" }
+
+        then:
+        succeeds "impl:${language.compileTaskName}"
+        // We recompile all module-info.java also for unrelated modules, but we don't recompile unrelated classes
+        impl.recompiledFqn("my.module.first.b.B", "my.module.second.c.C", "my.module.first.module-info", "my.module.second.module-info", "my.module.unrelated.module-info")
+    }
+
     @Requires(TestPrecondition.JDK9_OR_LATER)
     def "recompiles when upstream module-info changes"() {
         given:

subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java

@@ -16,22 +16,25 @@
 
 package org.gradle.api.internal.tasks.compile;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
 import org.gradle.api.internal.tasks.compile.processing.AnnotationProcessorDeclaration;
 import org.gradle.api.tasks.compile.CompileOptions;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
 public class DefaultJavaCompileSpec extends DefaultJvmLanguageCompileSpec implements JavaCompileSpec {
     private MinimalJavaCompileOptions compileOptions;
     private List<File> annotationProcessorPath;
     private Set<AnnotationProcessorDeclaration> effectiveAnnotationProcessors;
     private Set<String> classes;
     private List<File> modulePath;
     private List<File> sourceRoots;
+    private boolean isIncrementalCompilationOfJavaModule;
 
     @Override
     public MinimalJavaCompileOptions getCompileOptions() {
@@ -75,17 +78,25 @@ public void setClasses(Set<String> classes) {
     @Override
     public List<File> getModulePath() {
         if (modulePath == null || modulePath.isEmpty()) {
-            int i = compileOptions.getCompilerArgs().indexOf("--module-path");
-            if (i >= 0) {
-                // This is kept for backward compatibility - may be removed in the future
-                String[] modules = compileOptions.getCompilerArgs().get(i + 1).split(File.pathSeparator);
-                modulePath = Lists.newArrayListWithCapacity(modules.length);
-                for (String module : modules) {
-                    modulePath.add(new File(module));
+            // This is kept for backward compatibility - may be removed in the future
+            int i = 0;
+            List<String> modulePaths = new ArrayList<>();
+            // Some arguments can also be a GString, that is why use Object.toString()
+            for (Object argObj : compileOptions.getCompilerArgs()) {
+                String arg = argObj.toString();
+                if ((arg.equals("--module-path") || arg.equals("-p")) && (i + 1) < compileOptions.getCompilerArgs().size()) {
+                    Object argValue = compileOptions.getCompilerArgs().get(++i);
+                    String[] modules = argValue.toString().split(File.pathSeparator);
+                    modulePaths.addAll(Arrays.asList(modules));
+                } else if (arg.startsWith("--module-path=")) {
+                    String[] modules = arg.replace("--module-path=", "").split(File.pathSeparator);
+                    modulePaths.addAll(Arrays.asList(modules));
                 }
-            } else if (modulePath == null) {
-                modulePath = ImmutableList.of();
+                i++;
             }
+            modulePath = modulePaths.stream()
+                .map(File::new)
+                .collect(toImmutableList());
         }
         return modulePath;
     }
@@ -95,6 +106,16 @@ public void setModulePath(List<File> modulePath) {
         this.modulePath = modulePath;
     }
 
+    @Override
+    public boolean isIncrementalCompilationOfJavaModule() {
+        return isIncrementalCompilationOfJavaModule;
+    }
+
+    @Override
+    public void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule) {
+        this.isIncrementalCompilationOfJavaModule = isIncrementalCompilationOfJavaModule;
+    }
+
     @Override
     public List<File> getSourceRoots() {
         return sourceRoots;

subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java

@@ -47,6 +47,10 @@ public interface JavaCompileSpec extends JvmLanguageCompileSpec {
 
     void setModulePath(List<File> modulePath);
 
+    boolean isIncrementalCompilationOfJavaModule();
+
+    void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule);
+
     default boolean annotationProcessingConfigured() {
         return !getAnnotationProcessorPath().isEmpty() && !getCompileOptions().getCompilerArgs().contains("-proc:none");
     }

subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java

@@ -25,11 +25,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.tools.JavaCompiler;
 import javax.tools.JavaFileManager;
 import javax.tools.JavaFileObject;
 import javax.tools.StandardJavaFileManager;
+import java.io.File;
 import java.io.Serializable;
 import java.nio.charset.Charset;
 import java.util.Iterator;
@@ -66,7 +68,8 @@ private JavaCompiler.CompilationTask createCompileTask(JavaCompileSpec spec, Api
         StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, charset);
         Iterable<? extends JavaFileObject> compilationUnits = standardFileManager.getJavaFileObjectsFromFiles(spec.getSourceFiles());
         boolean hasEmptySourcepaths = JavaVersion.current().isJava9Compatible() && emptySourcepathIn(options);
-        JavaFileManager fileManager = GradleStandardJavaFileManager.wrap(standardFileManager, DefaultClassPath.of(spec.getAnnotationProcessorPath()), hasEmptySourcepaths);
+        File previousClassOutput = maybeGetPreviousClassOutput(spec);
+        JavaFileManager fileManager = GradleStandardJavaFileManager.wrap(standardFileManager, DefaultClassPath.of(spec.getAnnotationProcessorPath()), hasEmptySourcepaths, previousClassOutput);
         JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, spec.getClasses(), compilationUnits);
         if (compiler instanceof IncrementalCompilationAwareJavaCompiler) {
             task = ((IncrementalCompilationAwareJavaCompiler) compiler).makeIncremental(task, result.getSourceClassesMapping(), result.getConstantsAnalysisResult(), new CompilationSourceDirs(spec));
@@ -87,4 +90,12 @@ private static boolean emptySourcepathIn(List<String> options) {
         }
         return false;
     }
+
+    @Nullable
+    private static File maybeGetPreviousClassOutput(JavaCompileSpec spec) {
+        if (JavaVersion.current().isJava9Compatible() && spec.isIncrementalCompilationOfJavaModule() && !spec.getDestinationDir().equals(spec.getOriginalDestinationDir())) {
+            return spec.getOriginalDestinationDir();
+        }
+        return null;
+    }
 }