Skip to content

Commit

Permalink
Fix incremental compilation with modules
Browse files Browse the repository at this point in the history 
Backport from #23119 to 7.6.1

Fixes #23224
  • Loading branch information
@asodja
asodja committed Feb 16, 2023
1 parent ed6d092 commit a650e73
Show file tree
Hide file tree
Showing 16 changed files with 371 additions and 36 deletions.
1 change: 1 addition & 0 deletions subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java
View file
Expand Up @@ -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()));
Expand Down
52 changes: 52 additions & 0 deletions ...dle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
98 changes: 98 additions & 0 deletions ...gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
43 changes: 32 additions & 11 deletions ...uage-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java
View file
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions ...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java
View file
Expand Up @@ -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");
}
Expand Down
13 changes: 12 additions & 1 deletion ...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
Expand All @@ -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;
}
}
35 changes: 27 additions & 8 deletions ...adle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java
View file
Expand Up @@ -42,6 +42,10 @@
import java.util.Set;

abstract class AbstractRecompilationSpecProvider implements RecompilationSpecProvider {

private static final String MODULE_INFO_CLASS_NAME = "module-info";
private static final String PACKAGE_INFO_CLASS_NAME = "package-info";

private final Deleter deleter;
private final FileOperations fileOperations;
private final FileTree sourceTree;
Expand All @@ -64,19 +68,20 @@ public AbstractRecompilationSpecProvider(

@Override
public RecompilationSpec provideRecompilationSpec(CurrentCompilation current, PreviousCompilation previous) {
RecompilationSpec spec = new RecompilationSpec();
RecompilationSpec recompilationSpec = new RecompilationSpec();
SourceFileClassNameConverter sourceFileClassNameConverter = new FileNameDerivingClassNameConverter(previous.getSourceToClassConverter(), getFileExtensions());

processClasspathChanges(current, previous, spec);
processClasspathChanges(current, previous, recompilationSpec);

SourceFileChangeProcessor sourceFileChangeProcessor = new SourceFileChangeProcessor(previous);
processSourceChanges(current, sourceFileChangeProcessor, spec, sourceFileClassNameConverter);
collectAllSourcePathsAndIndependentClasses(sourceFileChangeProcessor, spec, sourceFileClassNameConverter);
processSourceChanges(current, sourceFileChangeProcessor, recompilationSpec, sourceFileClassNameConverter);
collectAllSourcePathsAndIndependentClasses(sourceFileChangeProcessor, recompilationSpec, sourceFileClassNameConverter);

Set<String> typesToReprocess = previous.getTypesToReprocess(spec.getClassesToCompile());
processTypesToReprocess(typesToReprocess, spec, sourceFileClassNameConverter);
Set<String> typesToReprocess = previous.getTypesToReprocess(recompilationSpec.getClassesToCompile());
processTypesToReprocess(typesToReprocess, recompilationSpec, sourceFileClassNameConverter);
addModuleInfoToCompile(recompilationSpec, sourceFileClassNameConverter);

return spec;
return recompilationSpec;
}

protected abstract Set<String> getFileExtensions();
Expand Down Expand Up @@ -177,7 +182,7 @@ private static Set<String> collectIndependentClassesForSourcePath(String sourceP

private static void processTypesToReprocess(Set<String> typesToReprocess, RecompilationSpec spec, SourceFileClassNameConverter sourceFileClassNameConverter) {
for (String typeToReprocess : typesToReprocess) {
if (typeToReprocess.endsWith("package-info") || typeToReprocess.equals("module-info")) {
if (typeToReprocess.endsWith(PACKAGE_INFO_CLASS_NAME) || typeToReprocess.equals(MODULE_INFO_CLASS_NAME)) {
// Fixes: https://github.com/gradle/gradle/issues/17572
// package-info classes cannot be passed as classes to reprocess to the Java compiler.
// Therefore, we need to recompile them every time anything changes if they are processed by an aggregating annotation processor.
Expand All @@ -189,6 +194,19 @@ private static void processTypesToReprocess(Set<String> typesToReprocess, Recomp
}
}

private static void addModuleInfoToCompile(RecompilationSpec spec, SourceFileClassNameConverter sourceFileClassNameConverter) {
Set<String> moduleInfoSources = sourceFileClassNameConverter.getRelativeSourcePaths(MODULE_INFO_CLASS_NAME);
if (!moduleInfoSources.isEmpty()) {
// Always recompile module-info.java if present.
// This solves case for incremental compilation for manual --module-path when not combined with --module-source-path or --source-path,
// since compiled module-info is not in the output after we change compile outputs in the CompileTransaction.
// Alternative would be, that we would move/copy the module-info class to transaction outputs and add transaction outputs to classpath.
// First part of fix for: https://github.com/gradle/gradle/issues/23067
spec.addClassToCompile(MODULE_INFO_CLASS_NAME);
spec.addSourcePaths(moduleInfoSources);
}
}

@Override
public CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec, RecompilationSpec recompilationSpec) {
if (!recompilationSpec.isBuildNeeded()) {
Expand All @@ -205,6 +223,7 @@ public CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec
includePreviousCompilationOutputOnClasspath(spec);
addClassesToProcess(spec, recompilationSpec);
Map<GeneratedResource.Location, PatternSet> resourcesToDelete = prepareResourcePatterns(recompilationSpec.getResourcesToGenerate(), fileOperations);
spec.setIsIncrementalCompilationOfJavaModule(recompilationSpec.hasClassToCompile(MODULE_INFO_CLASS_NAME));
return new CompileTransaction(spec, classesToDelete, resourcesToDelete, fileOperations, deleter);
}

Expand Down

0 comments on commit a650e73

Please sign in to comment.