From e2a879391a14e2b6efd61e580134b89e3e686149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C5=BEe=20Sodja?= Date: Wed, 26 Oct 2022 20:42:51 +0200 Subject: [PATCH 1/4] [Backport] Fix for Groovy and Java joint compilation failure on Java class private dependency change Backport of #22608 --- .../GroovyRecompilationSpecProvider.java | 40 +++- .../api/tasks/compile/GroovyCompile.java | 8 + ...GroovyIncrementalCompilationSupport.groovy | 18 +- ...pilationAfterFailureIntegrationTest.groovy | 32 ++++ ...crementalCompilationIntegrationTest.groovy | 176 ++++++++++++++++++ ...crementalCompilationIntegrationTest.groovy | 10 +- .../incremental/SelectiveCompiler.java | 2 +- .../AbstractRecompilationSpecProvider.java | 10 +- .../recomp/JavaRecompilationSpecProvider.java | 12 +- .../recomp/RecompilationSpecProvider.java | 4 +- .../recomp/SourceFileChangeProcessor.java | 10 + 11 files changed, 306 insertions(+), 16 deletions(-) create mode 100644 subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskGroovyJavaJointIncrementalCompilationIntegrationTest.groovy diff --git a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/GroovyRecompilationSpecProvider.java b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/GroovyRecompilationSpecProvider.java index e9e4dcd7f6a2..98ea620cbff8 100644 --- a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/GroovyRecompilationSpecProvider.java +++ b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/GroovyRecompilationSpecProvider.java @@ -19,13 +19,18 @@ import com.google.common.collect.ImmutableSet; import org.gradle.api.file.FileTree; import org.gradle.api.internal.file.FileOperations; +import org.gradle.api.internal.tasks.compile.GroovyJavaJointCompileSpec; +import org.gradle.api.internal.tasks.compile.JavaCompileSpec; import org.gradle.internal.file.Deleter; import org.gradle.work.FileChange; import java.util.Set; +import java.util.stream.Collectors; public class GroovyRecompilationSpecProvider extends AbstractRecompilationSpecProvider { + private static final Set SUPPORTED_FILE_EXTENSIONS = ImmutableSet.of(".java", ".groovy"); + public GroovyRecompilationSpecProvider( Deleter deleter, FileOperations fileOperations, @@ -36,9 +41,42 @@ public GroovyRecompilationSpecProvider( super(deleter, fileOperations, sources, sourceChanges, incremental); } + /** + * For all classes with Java source that we will be recompiled due to some change, we need to recompile all subclasses. + * This is because Groovy might try to load some subclass when analysing Groovy classes before Java compilation, but if parent class was stale, + * it has been deleted, so class loading of a subclass will fail. + * + * Fix for issue #22531. + */ + @Override + protected void processCompilerSpecificDependencies( + JavaCompileSpec spec, + RecompilationSpec recompilationSpec, + SourceFileChangeProcessor sourceFileChangeProcessor, + SourceFileClassNameConverter sourceFileClassNameConverter + ) { + if (!supportsGroovyJavaJointCompilation(spec)) { + return; + } + Set classesWithJavaSource = recompilationSpec.getClassesToCompile().stream() + .flatMap(classToCompile -> sourceFileClassNameConverter.getRelativeSourcePaths(classToCompile).stream()) + .filter(sourcePath -> sourcePath.endsWith(".java")) + .flatMap(sourcePath -> sourceFileClassNameConverter.getClassNames(sourcePath).stream()) + .collect(Collectors.toSet()); + if (!classesWithJavaSource.isEmpty()) { + // We need to collect just accessible dependents, since it seems + // private references to classes are not problematic when Groovy compiler loads a class + sourceFileChangeProcessor.processOnlyAccessibleChangeOfClasses(classesWithJavaSource, recompilationSpec); + } + } + + private boolean supportsGroovyJavaJointCompilation(JavaCompileSpec spec) { + return spec instanceof GroovyJavaJointCompileSpec && ((GroovyJavaJointCompileSpec) spec).getGroovyCompileOptions().getFileExtensions().contains("java"); + } + @Override protected Set getFileExtensions() { - return ImmutableSet.of(".java", ".groovy"); + return SUPPORTED_FILE_EXTENSIONS; } @Override diff --git a/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java b/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java index 171d200d73d9..40be005d7005 100644 --- a/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java +++ b/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java @@ -29,6 +29,7 @@ import org.gradle.api.internal.file.FileTreeInternal; import org.gradle.api.internal.file.temp.TemporaryFileProvider; import org.gradle.api.internal.tasks.compile.CleaningJavaCompiler; +import org.gradle.api.internal.tasks.compile.CommandLineJavaCompileSpec; import org.gradle.api.internal.tasks.compile.CompilationSourceDirs; import org.gradle.api.internal.tasks.compile.CompilerForkUtils; import org.gradle.api.internal.tasks.compile.DefaultGroovyJavaJointCompileSpec; @@ -134,10 +135,17 @@ protected void compile(InputChanges inputChanges) { checkGroovyClasspathIsNonEmpty(); warnIfCompileAvoidanceEnabled(); GroovyJavaJointCompileSpec spec = createSpec(); + maybeDisableIncrementalCompilationAfterFailure(spec); WorkResult result = getCompiler(spec, inputChanges).execute(spec); setDidWork(result.getDidWork()); } + private void maybeDisableIncrementalCompilationAfterFailure(GroovyJavaJointCompileSpec spec) { + if (CommandLineJavaCompileSpec.class.isAssignableFrom(spec.getClass())) { + spec.getCompileOptions().setSupportsIncrementalCompilationAfterFailure(false); + } + } + /** * The previous compilation analysis. Internal use only. * diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy index 897692f584b2..b781269ecd84 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy @@ -31,8 +31,20 @@ abstract class AbstractJavaGroovyIncrementalCompilationSupport extends AbstractI } File sourceForProject(String project, String... classBodies) { + return sourceForLanguageWithSuffixForProject(language, language.name, project, classBodies) + } + + File sourceWithFileSuffixForProject(String suffix, String project, String... classBodies) { + return sourceForLanguageWithSuffixForProject(language, suffix, project, classBodies) + } + + File sourceForLanguageForProject(CompiledLanguage language, String project, String... classBodies) { + return sourceForLanguageWithSuffixForProject(language, language.name, project, classBodies) + } + + private File sourceForLanguageWithSuffixForProject(CompiledLanguage language, String suffix, String project, String... classBodies) { File out = null - def basePath = project.isEmpty() ? "src/main/${language.name}" : "$project/src/main/${language.name}" + def basePath = project.isEmpty() ? "src/main/$language.name" : "$project/src/main/$language.name" for (String body : classBodies) { def packageGroup = (body =~ "\\s*package ([\\w.]+).*") String packageName = packageGroup.size() > 0 ? packageGroup[0][1] : "" @@ -41,9 +53,9 @@ abstract class AbstractJavaGroovyIncrementalCompilationSupport extends AbstractI assert className: "unable to find class name" def f if (packageFolder.isEmpty()) { - f = file("$basePath/${className}.${language.name}") + f = file("$basePath/${className}.$suffix") } else { - f = file("$basePath/${packageFolder}/${className}.${language.name}") + f = file("$basePath/${packageFolder}/${className}.$suffix") } f.createFile() f.text = body diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy index 887f0e9471d2..7509015c36ef 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy @@ -16,11 +16,14 @@ package org.gradle.java.compile.incremental +import org.gradle.api.JavaVersion import org.gradle.api.internal.cache.StringInterner import org.gradle.api.internal.tasks.compile.incremental.recomp.PreviousCompilationAccess +import org.gradle.integtests.fixtures.AvailableJavaHomes import org.gradle.integtests.fixtures.CompiledLanguage import org.gradle.util.Requires import org.gradle.util.TestPrecondition +import org.gradle.util.internal.TextUtil import spock.lang.Issue import static org.junit.Assume.assumeFalse @@ -220,6 +223,35 @@ abstract class BaseIncrementalCompilationAfterFailureIntegrationTest extends Abs outputs.recompiledClasses("A", "B", "C") outputContains("Full recompilation is required") } + + def "is not incremental compilation after failure when cli compiler is used"() { + given: + def compileTask = language == CompiledLanguage.GROOVY ? "GroovyCompile" : "JavaCompile" + buildFile << """ + tasks.withType($compileTask) { + options.incremental = true + options.fork = true + options.forkOptions.executable = '${TextUtil.escapeString(AvailableJavaHomes.getJdk(JavaVersion.current()).javacExecutable)}' + } + """ + source "class A {}", "class B extends A {}", "class C {}" + + when: "First compilation is always full compilation" + run language.compileTaskName + + then: + outputs.recompiledClasses("A", "B", "C") + + when: "Compilation after failure is full recompilation when optimization is disabled" + outputs.snapshot { source("class A { garbage }") } + runAndFail language.compileTaskName + outputs.snapshot { source("class A {}") } + run language.compileTaskName, "--info" + + then: + outputs.recompiledClasses("A", "B", "C") + outputContains("Full recompilation is required") + } } class JavaIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrementalCompilationAfterFailureIntegrationTest { diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskGroovyJavaJointIncrementalCompilationIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskGroovyJavaJointIncrementalCompilationIntegrationTest.groovy new file mode 100644 index 000000000000..656ce7fa1ef4 --- /dev/null +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskGroovyJavaJointIncrementalCompilationIntegrationTest.groovy @@ -0,0 +1,176 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle.java.compile.incremental + +import org.gradle.integtests.fixtures.CompiledLanguage +import spock.lang.Issue + +class CrossTaskGroovyJavaJointIncrementalCompilationIntegrationTest extends AbstractCrossTaskIncrementalCompilationSupport { + CompiledLanguage language = CompiledLanguage.GROOVY + boolean useJar = false + + def setup() { + configureGroovyIncrementalCompilation() + } + + @Issue("https://github.com/gradle/gradle/issues/22531") + def 'incremental compilation does not fail on api change referenced via static property when affected class is #bCompileStatic#bSuffix'() { + given: + // A is a private dependency of B1 and B1 is referenced in E1.isCacheEnabled through inheritance. + // B1 is also a private dependency of B2 that is referenced in E2.isCacheEnabled through inheritance. + File aClass = sourceForLanguageForProject(CompiledLanguage.JAVA, "api", "class A { void m1() {}; }") + sourceWithFileSuffixForProject(bSuffix, "impl", "$bCompileStatic class B1 { void m1() { A a = new A(); a.m1(); }; }") + sourceWithFileSuffixForProject("java", "impl", "class C1 extends B1 {}") + sourceWithFileSuffixForProject("java", "impl", "class D1 extends C1 { static boolean getCache() { return true; } }") + File e1Class = sourceWithFileSuffixForProject("groovy", "impl", "class E1 { boolean isCacheEnabled = D1.cache }") + + sourceWithFileSuffixForProject("java", "impl", "class B2 { void m1() { B1 b = new B1(); b.m1(); }; }") + sourceWithFileSuffixForProject("java", "impl", "class C2 extends B2 { }") + sourceWithFileSuffixForProject("java", "impl", "class D2 extends C2 { static boolean getCache() { return true; } }") + File e2Class = sourceWithFileSuffixForProject("groovy", "impl", "class E2 { boolean isCacheEnabled = D2.cache }") + + run ":impl:compileGroovy" + + when: + impl.snapshot { + aClass.text = "class A { void m1() {}; void m2() {}; }" + e1Class.text = "class E1 { boolean isCacheEnabled = D1.cache; int a = 0; }" + e2Class.text = "class E2 { boolean isCacheEnabled = D2.cache; int a = 0; }" + } + run ":impl:compileGroovy" + + then: + impl.recompiledClasses(*expectedRecompiledClass) + + where: + bSuffix | bCompileStatic | expectedRecompiledClass + "java" | "" | ["B1", "C1", "D1", "E1", "E2"] + "groovy" | "" | ["B1", "E1", "E2"] + "groovy" | "@groovy.transform.CompileStatic " | ["B1", "E1", "E2"] + } + + def 'incremental compilation does not fail when some deleted class with Java source is private referenced in class that is loaded by Groovy'() { + given: + // A is a private dependency of B1 and B1 is referenced in E1.isCacheEnabled through inheritance. + // B1 is also a private dependency of B2 that is referenced in E2.isCacheEnabled through inheritance. + File aClass = sourceForLanguageForProject(CompiledLanguage.JAVA, "api", "class A { void m1() {}; }") + sourceWithFileSuffixForProject("java", "impl", "class B1 { static B1 m2() { return null; }; void m1() { A a = new A(); a.m1(); }; }") + sourceWithFileSuffixForProject("java", "impl", "class C1 extends B1 {}") + sourceWithFileSuffixForProject("java", "impl", "class D1 extends C1 { static boolean getCache() { return true; } }") + File e1Class = sourceWithFileSuffixForProject("groovy", "impl", "class E1 { boolean isCacheEnabled = D1.cache }") + + sourceWithFileSuffixForProject("java", "impl", """ + class B2 { + private static final Class bClass = B1.class; + private static final B1 b1 = new B1(); + private static final B1 b2; + static { + b2 = new B1(); + } + private static B1 b3 = new B1(); + private B1 b4 = new B1(); + private B1 m1(B1 b) { return new B1(); }; + } + """) + sourceWithFileSuffixForProject("java", "impl", "class C2 extends B2 { }") + sourceWithFileSuffixForProject("java", "impl", """ + class D2 extends C2 { + + private static final Class bClass = B1.class; + private static final B1 b1 = new B1(); + private static final B1 b2; + static { + b2 = new B1(); + } + private static B1 b3 = new B1(); + private B1 b4 = new B1(); + private B1 m1(B1 b) { return new B1(); }; + static boolean getCache() { return true; } + } + """) + File e2Class = sourceWithFileSuffixForProject("groovy", "impl", "class E2 { boolean isCacheEnabled = D2.cache }") + + run ":impl:compileGroovy" + + when: + impl.snapshot { + aClass.text = "class A { void m1() {}; void m2() {}; }" + e1Class.text = "class E1 { boolean isCacheEnabled = D1.cache; int a = 0; }" + e2Class.text = "class E2 { boolean isCacheEnabled = D2.cache; int a = 0; }" + } + run ":impl:compileGroovy" + + then: + impl.recompiledClasses("B1", "C1", "D1", "E1", "E2") + } + + def 'incremental compilation does not fail on api change when we compile only groovy in the dependent project and affected class is #bCompileStatic#bSuffix'() { + given: + // A is a private dependency of B and B is referenced in E.isCacheEnabled through inheritance. + File aClass = sourceForLanguageForProject(CompiledLanguage.JAVA, "api", "class A { void m1() {}; }") + sourceWithFileSuffixForProject("groovy", "impl", "$bCompileStatic class B { void m1() { A a = new A(); a.m1(); }; }") + sourceWithFileSuffixForProject("groovy", "impl", "class C extends B {}") + sourceWithFileSuffixForProject("groovy", "impl", "class D extends C { static boolean getCache() { return true; } }") + File eClass = sourceWithFileSuffixForProject("groovy", "impl", "class E { boolean isCacheEnabled = D.cache }") + + run ":impl:compileGroovy" + + when: + impl.snapshot { + aClass.text = "class A { void m1() {}; void m2() {}; }" + eClass.text = "class E { boolean isCacheEnabled = D.cache; int a = 0; }" + } + run ":impl:compileGroovy" + + then: + impl.recompiledClasses(*expectedRecompiledClass) + + where: + bSuffix | bCompileStatic | expectedRecompiledClass + "groovy" | "" | ["B", "E"] + "groovy" | "@groovy.transform.CompileStatic " | ["B", "E"] + } + + def 'incremental compilation after a failure works on api dependency change'() { + given: + File aClass = sourceForLanguageForProject(CompiledLanguage.JAVA, "api", "class A { void m1() {}; }") + sourceWithFileSuffixForProject("java", "impl", "class B { void m1() { new A().m1(); }; }") + sourceWithFileSuffixForProject("java", "impl", "class C extends B {}") + sourceWithFileSuffixForProject("java", "impl", "class D extends C { static boolean getCache() { return true; } }") + File eClass = sourceWithFileSuffixForProject("groovy", "impl", "class E { boolean isCacheEnabled = D.cache }") + sourceWithFileSuffixForProject("java", "impl", "class F {}") + sourceWithFileSuffixForProject("groovy", "impl", "class G {}") + run ":impl:compileGroovy" + + when: + aClass.text = "class A { void m1() {}; void m2() {}; }" + eClass.text = "class E { boolean isCacheEnabled = D.cache; garbage }" + + then: + runAndFail ":impl:compileGroovy" + + when: + impl.snapshot { + aClass.text = "class A { void m1() {}; void m2() {}; }" + eClass.text = "class E { boolean isCacheEnabled = D.cache; int i = 0; }" + } + run ":impl:compileGroovy" + + then: + impl.recompiledClasses("B", "C", "D", "E") + } +} diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/GroovyJavaJointIncrementalCompilationIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/GroovyJavaJointIncrementalCompilationIntegrationTest.groovy index 61e495935707..4daa6abbd079 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/GroovyJavaJointIncrementalCompilationIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/GroovyJavaJointIncrementalCompilationIntegrationTest.groovy @@ -120,7 +120,7 @@ class GroovyJavaJointIncrementalCompilationIntegrationTest extends AbstractJavaG outputs.noneRecompiled() when: 'second build' - applyGroovyFileSet(secondChange) + outputs.snapshot { applyGroovyFileSet(secondChange) } run "compileGroovy", "--info" then: @@ -128,8 +128,7 @@ class GroovyJavaJointIncrementalCompilationIntegrationTest extends AbstractJavaG outputs.recompiledClasses(secondChangeRecompiledClasses as String[]) when: 'third build' - outputs.snapshot() - applyGroovyFileSet(thirdChange) + outputs.snapshot { applyGroovyFileSet(thirdChange) } run "compileGroovy", "--info" then: @@ -157,7 +156,7 @@ class GroovyJavaJointIncrementalCompilationIntegrationTest extends AbstractJavaG outputs.noneRecompiled() when: 'second build' - applyMixFileSet(secondChange) + outputs.snapshot { applyMixFileSet(secondChange) } run "compileGroovy", "--info" then: @@ -166,8 +165,7 @@ class GroovyJavaJointIncrementalCompilationIntegrationTest extends AbstractJavaG outputs.recompiledClasses(secondChangeRecompiledClasses as String[]) when: 'third build' - outputs.snapshot() - applyMixFileSet(thirdChange) + outputs.snapshot { applyMixFileSet(thirdChange) } run "compileGroovy", "--info" then: diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java index 3c51a94c98d7..4c1ad41c1711 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java @@ -84,7 +84,7 @@ public WorkResult execute(T spec) { PreviousCompilationData previousCompilationData = previousCompilationAccess.readPreviousCompilationData(previousCompilationDataFile); PreviousCompilation previousCompilation = new PreviousCompilation(previousCompilationData); - RecompilationSpec recompilationSpec = recompilationSpecProvider.provideRecompilationSpec(currentCompilation, previousCompilation); + RecompilationSpec recompilationSpec = recompilationSpecProvider.provideRecompilationSpec(spec, currentCompilation, previousCompilation); if (recompilationSpec.isFullRebuildNeeded()) { LOG.info("Full recompilation is required because {}. Analysis took {}.", recompilationSpec.getFullRebuildCause(), clock.getElapsed()); diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java index c062422c325e..0b0fbd8d40a5 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java @@ -67,7 +67,7 @@ public AbstractRecompilationSpecProvider( } @Override - public RecompilationSpec provideRecompilationSpec(CurrentCompilation current, PreviousCompilation previous) { + public RecompilationSpec provideRecompilationSpec(JavaCompileSpec spec, CurrentCompilation current, PreviousCompilation previous) { RecompilationSpec recompilationSpec = new RecompilationSpec(); SourceFileClassNameConverter sourceFileClassNameConverter = new FileNameDerivingClassNameConverter(previous.getSourceToClassConverter(), getFileExtensions()); @@ -75,6 +75,7 @@ public RecompilationSpec provideRecompilationSpec(CurrentCompilation current, Pr SourceFileChangeProcessor sourceFileChangeProcessor = new SourceFileChangeProcessor(previous); processSourceChanges(current, sourceFileChangeProcessor, recompilationSpec, sourceFileClassNameConverter); + processCompilerSpecificDependencies(spec, recompilationSpec, sourceFileChangeProcessor, sourceFileClassNameConverter); collectAllSourcePathsAndIndependentClasses(sourceFileChangeProcessor, recompilationSpec, sourceFileClassNameConverter); Set typesToReprocess = previous.getTypesToReprocess(recompilationSpec.getClassesToCompile()); @@ -122,6 +123,13 @@ private String rebuildClauseForChangedNonSourceFile(FileChange fileChange) { return String.format("%s '%s' has been %s", "resource", fileChange.getFile().getName(), fileChange.getChangeType().name().toLowerCase(Locale.US)); } + protected abstract void processCompilerSpecificDependencies( + JavaCompileSpec spec, + RecompilationSpec recompilationSpec, + SourceFileChangeProcessor sourceFileChangeProcessor, + SourceFileClassNameConverter sourceFileClassNameConverter + ); + protected abstract boolean isIncrementalOnResourceChanges(CurrentCompilation currentCompilation); /** diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/JavaRecompilationSpecProvider.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/JavaRecompilationSpecProvider.java index 3f1d47c4d677..9531f3f6ce4e 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/JavaRecompilationSpecProvider.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/JavaRecompilationSpecProvider.java @@ -16,16 +16,19 @@ package org.gradle.api.internal.tasks.compile.incremental.recomp; +import com.google.common.collect.ImmutableSet; import org.gradle.api.file.FileTree; import org.gradle.api.internal.file.FileOperations; +import org.gradle.api.internal.tasks.compile.JavaCompileSpec; import org.gradle.internal.file.Deleter; import org.gradle.work.FileChange; -import java.util.Collections; import java.util.Set; public class JavaRecompilationSpecProvider extends AbstractRecompilationSpecProvider { + private static final Set SUPPORTED_FILE_EXTENSIONS = ImmutableSet.of(".java"); + public JavaRecompilationSpecProvider( Deleter deleter, FileOperations fileOperations, @@ -38,7 +41,12 @@ public JavaRecompilationSpecProvider( @Override protected Set getFileExtensions() { - return Collections.singleton(".java"); + return SUPPORTED_FILE_EXTENSIONS; + } + + @Override + protected void processCompilerSpecificDependencies(JavaCompileSpec spec, RecompilationSpec recompilationSpec, SourceFileChangeProcessor sourceFileChangeProcessor, SourceFileClassNameConverter sourceFileClassNameConverter) { + // Nothing to do for Java compiler here } @Override diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpecProvider.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpecProvider.java index e47826d1c26c..0db39c83c292 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpecProvider.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpecProvider.java @@ -22,7 +22,7 @@ /** * In a typical incremental recompilation, there're three steps: - * First, examine the incremental change files to get the classes to be recompiled: {@link #provideRecompilationSpec(CurrentCompilation, PreviousCompilation)} + * First, examine the incremental change files to get the classes to be recompiled: {@link #provideRecompilationSpec(JavaCompileSpec, CurrentCompilation, PreviousCompilation)} * Second, initialize the recompilation (e.g. delete stale class files and narrow down the source files to be recompiled): {@link #initCompilationSpecAndTransaction(JavaCompileSpec, RecompilationSpec)} * Third, decorate the compilation result if necessary: {@link #decorateResult(RecompilationSpec, PreviousCompilationData, WorkResult)}, for example, notify whether current recompilation is full recompilation. */ @@ -30,7 +30,7 @@ public interface RecompilationSpecProvider { boolean isIncremental(); - RecompilationSpec provideRecompilationSpec(CurrentCompilation current, PreviousCompilation previous); + RecompilationSpec provideRecompilationSpec(JavaCompileSpec spec, CurrentCompilation current, PreviousCompilation previous); CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec, RecompilationSpec recompilationSpec); diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/SourceFileChangeProcessor.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/SourceFileChangeProcessor.java index 52ebe425fe91..ee36f12be004 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/SourceFileChangeProcessor.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/SourceFileChangeProcessor.java @@ -39,6 +39,16 @@ public void processChange(Set classNames, RecompilationSpec spec) { spec.addResourcesToGenerate(actualDependents.getDependentResources()); } + public void processOnlyAccessibleChangeOfClasses(Set classNames, RecompilationSpec spec) { + DependentsSet actualDependents = previousCompilation.findDependentsOfSourceChanges(classNames); + if (actualDependents.isDependencyToAll()) { + spec.setFullRebuildCause(actualDependents.getDescription()); + return; + } + spec.addClassesToCompile(actualDependents.getAccessibleDependentClasses()); + spec.addResourcesToGenerate(actualDependents.getDependentResources()); + } + public Set processAnnotationDependenciesOfIndependentClasses(Set classNames, RecompilationSpec spec) { Set newAdded = new LinkedHashSet<>(); for (String className : classNames) { From 000da92f08df0292a977fe234b91ecd7abac62b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C5=BEe=20Sodja?= Date: Mon, 19 Dec 2022 10:36:53 +0100 Subject: [PATCH 2/4] [Backport] Use compiler API data for incremental compilation after failure Backport of #23226 --- .../compiler/java/IncrementalCompileTask.java | 5 +- .../classnames/ClassNameCollector.java | 15 +- .../listeners/ConstantsCollectorTest.groovy | 1 + .../internal/compiler/java/TestCompiler.java | 5 +- .../tasks/compile/ApiGroovyCompiler.java | 22 +- .../api/tasks/compile/GroovyCompile.java | 1 - ...AnnotationProcessingIntegrationTest.groovy | 62 ++++ ...AnnotationProcessingIntegrationTest.groovy | 20 +- ...GroovyIncrementalCompilationSupport.groovy | 4 + ...pilationAfterFailureIntegrationTest.groovy | 173 ++++++++++- ...entalJavaCompilationIntegrationTest.groovy | 35 +++ .../tasks/compile/ApiCompilerResult.java | 5 + .../CompilationClassBackupService.java | 91 ++++++ .../compile/CompilationFailedException.java | 19 +- .../tasks/compile/DefaultJavaCompileSpec.java | 46 +-- ...crementalCompilationAwareJavaCompiler.java | 8 +- .../tasks/compile/JavaCompileSpec.java | 28 +- .../tasks/compile/JdkJavaCompiler.java | 26 +- .../api/internal/tasks/compile/JdkTools.java | 6 +- .../incremental/SelectiveCompiler.java | 2 +- .../AbstractRecompilationSpecProvider.java | 11 +- .../incremental/recomp/RecompilationSpec.java | 4 - .../transaction/CompileTransaction.java | 271 ++++++++---------- .../GradleStandardJavaFileManager.java | 52 +--- .../gradle/api/tasks/compile/JavaCompile.java | 1 - .../transaction/CompileTransactionTest.groovy | 184 ++++++------ .../DefaultJvmLanguageCompileSpec.java | 11 - .../tasks/compile/JvmLanguageCompileSpec.java | 4 - .../scala/tasks/AbstractScalaCompile.java | 1 - 29 files changed, 729 insertions(+), 384 deletions(-) create mode 100644 subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationClassBackupService.java rename subprojects/{language-jvm => language-java}/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java (64%) diff --git a/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/IncrementalCompileTask.java b/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/IncrementalCompileTask.java index f77cd733a8dc..9ccaf0ce6b03 100644 --- a/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/IncrementalCompileTask.java +++ b/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/IncrementalCompileTask.java @@ -46,15 +46,18 @@ public class IncrementalCompileTask implements JavaCompiler.CompilationTask { private final Function> relativize; private final Consumer>> classNameConsumer; + private final Consumer classBackupService; private final ConstantDependentsConsumer constantDependentsConsumer; private final JavacTask delegate; public IncrementalCompileTask(JavaCompiler.CompilationTask delegate, Function> relativize, + Consumer classBackupService, Consumer>> classNamesConsumer, BiConsumer publicDependentDelegate, BiConsumer privateDependentDelegate) { this.relativize = relativize; + this.classBackupService = classBackupService; this.classNameConsumer = classNamesConsumer; this.constantDependentsConsumer = new ConstantDependentsConsumer(publicDependentDelegate, privateDependentDelegate); if (delegate instanceof JavacTask) { @@ -81,7 +84,7 @@ public void setLocale(Locale locale) { @Override public Boolean call() { - ClassNameCollector classNameCollector = new ClassNameCollector(relativize, delegate.getElements()); + ClassNameCollector classNameCollector = new ClassNameCollector(relativize, classBackupService, delegate.getElements()); ConstantsCollector constantsCollector = new ConstantsCollector(delegate, constantDependentsConsumer); delegate.addTaskListener(classNameCollector); delegate.addTaskListener(constantsCollector); diff --git a/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/classnames/ClassNameCollector.java b/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/classnames/ClassNameCollector.java index 285956ccabd8..c6e7b088a3cd 100644 --- a/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/classnames/ClassNameCollector.java +++ b/subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/classnames/ClassNameCollector.java @@ -28,16 +28,19 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; +import java.util.function.Consumer; import java.util.function.Function; public class ClassNameCollector implements TaskListener { private final Map> relativePaths = new HashMap<>(); + private final Consumer classBackupService; private final Map> mapping = new HashMap<>(); private final Function> relativize; private final Elements elements; - public ClassNameCollector(Function> relativize, Elements elements) { + public ClassNameCollector(Function> relativize, Consumer classBackupService, Elements elements) { this.relativize = relativize; + this.classBackupService = classBackupService; this.elements = elements; } @@ -47,11 +50,6 @@ public Map> getMapping() { @Override public void started(TaskEvent e) { - - } - - @Override - public void finished(TaskEvent e) { JavaFileObject sourceFile = e.getSourceFile(); if (isSourceFile(sourceFile)) { File asSourceFile = new File(sourceFile.getName()); @@ -63,6 +61,10 @@ public void finished(TaskEvent e) { } } + @Override + public void finished(TaskEvent e) { + } + private static boolean isSourceFile(JavaFileObject sourceFile) { return sourceFile != null && sourceFile.getKind() == JavaFileObject.Kind.SOURCE; } @@ -73,6 +75,7 @@ private void processSourceFile(TaskEvent e, File sourceFile) { String key = relativePath.get(); String symbol = normalizeName(e.getTypeElement()); registerMapping(key, symbol); + classBackupService.accept(symbol); } } diff --git a/subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/listeners/ConstantsCollectorTest.groovy b/subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/listeners/ConstantsCollectorTest.groovy index a4aa1cd765f4..485f5a80f50a 100644 --- a/subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/listeners/ConstantsCollectorTest.groovy +++ b/subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/listeners/ConstantsCollectorTest.groovy @@ -40,6 +40,7 @@ class ConstantsCollectorTest extends AbstractCompilerPluginTest { Files.createTempDirectory(temporaryFolder.toPath(), null).toFile(), { f -> Optional.empty() }, {}, + {}, consumer ) } diff --git a/subprojects/java-compiler-plugin/src/testFixtures/java/org/gradle/internal/compiler/java/TestCompiler.java b/subprojects/java-compiler-plugin/src/testFixtures/java/org/gradle/internal/compiler/java/TestCompiler.java index ad85039ecd67..18fbd796a5cb 100644 --- a/subprojects/java-compiler-plugin/src/testFixtures/java/org/gradle/internal/compiler/java/TestCompiler.java +++ b/subprojects/java-compiler-plugin/src/testFixtures/java/org/gradle/internal/compiler/java/TestCompiler.java @@ -38,15 +38,18 @@ public class TestCompiler { private final File outputFolder; private final Function> relativize; + private final Consumer classBackupService; private final Consumer>> classNameConsumer; private final ConstantDependentsConsumer constantDependentsConsumer; public TestCompiler(File outputFolder, Function> relativize, + Consumer classBackupService, Consumer>> classNamesConsumer, ConstantDependentsConsumer constantDependentsConsumer) { this.outputFolder = outputFolder; this.relativize = relativize; + this.classBackupService = classBackupService; this.classNameConsumer = classNamesConsumer; this.constantDependentsConsumer = constantDependentsConsumer; } @@ -58,7 +61,7 @@ public void compile(List sourceFiles) { Iterable compilationUnits = fileManager.getJavaFileObjectsFromFiles(sourceFiles); List arguments = Arrays.asList("-d", outputFolder.getAbsolutePath()); JavaCompiler.CompilationTask delegate = compiler.getTask(output, fileManager, null, arguments, null, compilationUnits); - IncrementalCompileTask task = new IncrementalCompileTask(delegate, relativize, classNameConsumer, constantDependentsConsumer::consumeAccessibleDependent, constantDependentsConsumer::consumePrivateDependent); + IncrementalCompileTask task = new IncrementalCompileTask(delegate, relativize, classBackupService, classNameConsumer, constantDependentsConsumer::consumeAccessibleDependent, constantDependentsConsumer::consumePrivateDependent); if (!task.call()) { throw new RuntimeException(output.toString()); } diff --git a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java index d563aace2ffc..52c837d7ea05 100644 --- a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java +++ b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java @@ -58,6 +58,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import static org.gradle.internal.FileUtils.hasExtension; @@ -73,7 +74,7 @@ public ApiGroovyCompiler(Compiler javaCompiler, ProjectLayout p private static abstract class IncrementalCompilationCustomizer extends CompilationCustomizer { static IncrementalCompilationCustomizer fromSpec(GroovyJavaJointCompileSpec spec, ApiCompilerResult result) { if (spec.incrementalCompilationEnabled()) { - return new TrackingClassGenerationCompilationCustomizer(new CompilationSourceDirs(spec), result); + return new TrackingClassGenerationCompilationCustomizer(new CompilationSourceDirs(spec), result, new CompilationClassBackupService(spec, result)); } else { return new NoOpCompilationCustomizer(); } @@ -101,10 +102,12 @@ public void call(SourceUnit source, GeneratorContext context, ClassNode classNod private static class TrackingClassGenerationCompilationCustomizer extends IncrementalCompilationCustomizer { private final CompilationSourceDirs compilationSourceDirs; private final ApiCompilerResult result; + private final CompilationClassBackupService compilationClassBackupService; - private TrackingClassGenerationCompilationCustomizer(CompilationSourceDirs compilationSourceDirs, ApiCompilerResult result) { + private TrackingClassGenerationCompilationCustomizer(CompilationSourceDirs compilationSourceDirs, ApiCompilerResult result, CompilationClassBackupService compilationClassBackupService) { this.compilationSourceDirs = compilationSourceDirs; this.result = result; + this.compilationClassBackupService = compilationClassBackupService; } @Override @@ -113,8 +116,10 @@ public void call(SourceUnit source, GeneratorContext context, ClassNode classNod } private void inspectClassNode(SourceUnit sourceUnit, ClassNode classNode) { + String classFqName = classNode.getName(); String relativePath = compilationSourceDirs.relativize(new File(sourceUnit.getSource().getURI().getPath())).orElseThrow(IllegalStateException::new); - result.getSourceClassesMapping().computeIfAbsent(relativePath, key -> new HashSet<>()).add(classNode.getName()); + result.getSourceClassesMapping().computeIfAbsent(relativePath, key -> new HashSet<>()).add(classFqName); + compilationClassBackupService.maybeBackupClassFile(classFqName); Iterator iterator = classNode.getInnerClasses(); while (iterator.hasNext()) { inspectClassNode(sourceUnit, iterator.next()); @@ -257,14 +262,21 @@ public boolean apply(File file) { try { WorkResult javaCompilerResult = javaCompiler.execute(spec); if (javaCompilerResult instanceof ApiCompilerResult) { - result.getSourceClassesMapping().putAll(((ApiCompilerResult) javaCompilerResult).getSourceClassesMapping()); + copyJavaCompilerResult((ApiCompilerResult) javaCompilerResult); } } catch (CompilationFailedException e) { + Optional partialResult = e.getCompilerPartialResult(); + partialResult.ifPresent(result -> copyJavaCompilerResult(result)); cu.getErrorCollector().addFatalError(new SimpleMessage(e.getMessage(), cu)); } } }; } + + private void copyJavaCompilerResult(ApiCompilerResult javaCompilerResult) { + result.getSourceClassesMapping().putAll(javaCompilerResult.getSourceClassesMapping()); + result.getBackupClassFiles().putAll(javaCompilerResult.getBackupClassFiles()); + } }); try { @@ -274,7 +286,7 @@ public boolean apply(File file) { System.err.println(e.getMessage()); // Explicit flush, System.err is an auto-flushing PrintWriter unless it is replaced. System.err.flush(); - throw new CompilationFailedException(); + throw new CompilationFailedException(result); } finally { // Remove compile and AST types from the Groovy loader compilerGroovyLoader.discardTypesFrom(classPathLoader); diff --git a/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java b/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java index 40be005d7005..7588d5ae2dcd 100644 --- a/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java +++ b/subprojects/language-groovy/src/main/java/org/gradle/api/tasks/compile/GroovyCompile.java @@ -270,7 +270,6 @@ 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())); diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/AggregatingIncrementalAnnotationProcessingIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/AggregatingIncrementalAnnotationProcessingIntegrationTest.groovy index d78b30e035b7..d66ebbd64346 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/AggregatingIncrementalAnnotationProcessingIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/AggregatingIncrementalAnnotationProcessingIntegrationTest.groovy @@ -57,6 +57,68 @@ class AggregatingIncrementalAnnotationProcessingIntegrationTest extends Abstract serviceRegistryReferences("A", "B") } + def "incremental processing works on subsequent incremental compilations after failure"() { + def a = java "@Service class A {}" + java "@Service class B {}" + java "class Unrelated {}" + + outputs.snapshot { run "compileJava" } + + when: + a.text = "@Service class A { public String foo() { return 0; } }" + runAndFail "compileJava", "-d" + + then: + outputs.noneRecompiled() + outputContains("Deleting generated files: [${file("build/classes/java/main/ServiceRegistryResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/ServiceRegistry.java")}]") + outputContains("Restoring stashed files: [" + + "${file("build/classes/java/main/A.class")}, " + + "${file("build/classes/java/main/ServiceRegistry.class")}, " + + "${file("build/classes/java/main/ServiceRegistryResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/ServiceRegistry.java")}]" + ) + + when: + a.text = "@Service class A { public int foo() { return 0; } }" + run "compileJava" + + then: + outputs.recompiledFiles("A", "ServiceRegistry", "ServiceRegistryResource.txt") + serviceRegistryReferences("A", "B") + } + + + def "incremental processing works when new aggregating type is added on subsequent incremental compilations after failure"() { + def a = java "@Service class A {}" + java "@Service class B {}" + java "class Unrelated {}" + + outputs.snapshot { run "compileJava" } + + when: + def c = java "@Service class C { public String foo() { return 0; } }" + runAndFail "compileJava", "-d" + + then: + outputs.noneRecompiled() + outputContains("Deleting generated files: [${file("build/classes/java/main/ServiceRegistryResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/ServiceRegistry.java")}]") + outputContains("Restoring stashed files: [" + + "${file("build/classes/java/main/ServiceRegistry.class")}, " + + "${file("build/classes/java/main/ServiceRegistryResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/ServiceRegistry.java")}]" + ) + + when: + c.text = "@Service class C { public int foo() { return 0; } }" + run "compileJava" + + then: + outputs.recompiledFiles("C", "ServiceRegistry", "ServiceRegistryResource.txt") + serviceRegistryReferences("A", "B", "C") + } + def "unrelated files are not recompiled when annotated file changes"() { def a = java "@Service class A {}" java "class Unrelated {}" diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/IsolatingIncrementalAnnotationProcessingIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/IsolatingIncrementalAnnotationProcessingIntegrationTest.groovy index 2fbaf7e0295b..6275b4c3435a 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/IsolatingIncrementalAnnotationProcessingIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/api/tasks/compile/IsolatingIncrementalAnnotationProcessingIntegrationTest.groovy @@ -189,17 +189,27 @@ class IsolatingIncrementalAnnotationProcessingIntegrationTest extends AbstractIn def "incremental processing works on subsequent incremental compilations after failure"() { given: def a = java "@Helper class A {}" + java "@Helper class B {}" java "class Unrelated {}" run "compileJava" a.text = "@Helper class A { public void foo() {} }" outputs.snapshot { run "compileJava" } when: - a.text = "@Helper class A { public void bar() { garbage } }" - runAndFail "compileJava" + a.text = "@Helper class A { public String bar() { return 0; } }" + runAndFail "compileJava", "-d" then: outputs.noneRecompiled() + outputContains("Deleting generated files: [${file("build/classes/java/main/AHelperResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/AHelper.java")}]" + ) + outputContains("Restoring stashed files: [" + + "${file("build/classes/java/main/A.class")}, " + + "${file("build/classes/java/main/AHelper.class")}, " + + "${file("build/classes/java/main/AHelperResource.txt")}, " + + "${file("build/generated/sources/annotationProcessor/java/main/AHelper.java")}]" + ) when: a.text = "@Helper class A { public void bar() {} }" @@ -232,11 +242,13 @@ class IsolatingIncrementalAnnotationProcessingIntegrationTest extends AbstractIn outputs.snapshot { run "compileJava" } when: - unrelated.text = "class Unrelated { public void foo() { garbage } }" - runAndFail "compileJava" + unrelated.text = "class Unrelated { public Unrelated foo() { return 0; } }" + runAndFail "compileJava", "-d" then: outputs.noneRecompiled() + outputContains("Deleting generated files: []") + outputContains("Restoring stashed files: [${file("build/classes/java/main/Unrelated.class")}]") when: unrelated.text = "class Unrelated { public void foo() {} }" diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy index b781269ecd84..fd3cad79cf67 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy @@ -34,6 +34,10 @@ abstract class AbstractJavaGroovyIncrementalCompilationSupport extends AbstractI return sourceForLanguageWithSuffixForProject(language, language.name, project, classBodies) } + File sourceWithFileSuffix(String suffix, String... classBodies) { + return sourceForLanguageWithSuffixForProject(language, suffix, "", classBodies) + } + File sourceWithFileSuffixForProject(String suffix, String project, String... classBodies) { return sourceForLanguageWithSuffixForProject(language, suffix, project, classBodies) } diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy index 7509015c36ef..453cb805b4fa 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy @@ -26,10 +26,10 @@ import org.gradle.util.TestPrecondition import org.gradle.util.internal.TextUtil import spock.lang.Issue -import static org.junit.Assume.assumeFalse - abstract class BaseIncrementalCompilationAfterFailureIntegrationTest extends AbstractJavaGroovyIncrementalCompilationSupport { + String compileStaticAnnotation = "" + def "detects deletion of a source base class that leads to compilation failure but keeps old files"() { def a = source "class A {}" source "class B extends A {}" @@ -45,17 +45,108 @@ abstract class BaseIncrementalCompilationAfterFailureIntegrationTest extends Abs outputs.deletedClasses() } - def "old classes are restored after the compile failure"() { + def "old classes are restored after the compile failure and incremental compilation works after a failure"() { source("class A {}", "class B {}") outputs.snapshot { run language.compileTaskName } when: - source("class A { garbage }") + def a = source("class A { garbage }") runAndFail language.compileTaskName then: outputs.noneRecompiled() outputs.hasFiles(file("A.class"), file("B.class")) + + when: + a.text = "class A {}" + run language.compileTaskName + + then: + outputs.noneRecompiled() + } + + def "new generated classes are removed on the compile failure and incremental compilation works after a failure"() { + def b = source("package b; class B {}") + source("class Unrelated {}") + outputs.snapshot { run language.compileTaskName } + + when: + // Compile more classes, so there is possibility some is generated before a compile failure. + // Since order of passed classes to javac is unstable we can compile classes in different order on different platforms. + // Compile .java also for Groovy, so the file is written before failure. + sourceWithFileSuffix("java", "package a; class A {}") + b.text = "package b; $compileStaticAnnotation class B { B m1() { return 0; } }" + sourceWithFileSuffix("java", "package c; class C {}") + runAndFail language.compileTaskName + + then: + outputs.noneRecompiled() + outputs.hasFiles(file("b/B.class"), file("Unrelated.class")) + !file("build/classes/$languageName/main/a/").exists() + !file("build/classes/$languageName/main/c/").exists() + + when: + b.text = "package b; class B {}" + run language.compileTaskName + + then: + outputs.recompiledClasses("A", "C") + } + + def "classes in renamed files are restored on the compile failure and incremental compilation works after a failure"() { + def a = source"package a; class A {}" + def b = source "package b; class B {}" + def c = source "package c; class C {}" + source "class Unrelated {}" + outputs.snapshot { run language.compileTaskName } + + when: + // Compile more classes, so there is possibility some is generated before a compile failure. + // Since order of passed classes to javac is unstable we can compile classes in different order on different platforms. + // Compile .java also for Groovy, so the file is written before failure. + a.delete() + c.delete() + file("src/main/$languageName/a/A1.java").text = "package a; class A { void m1() {} }" + b.text = "package b; $compileStaticAnnotation class B { B getB() { return 0; } }" + file("src/main/$languageName/c/C1.java").text = "package c; class C { void m1() {} }" + runAndFail language.compileTaskName + + then: + outputs.noneRecompiled() + outputs.hasFiles(file("a/A.class"), file("b/B.class"), file("c/C.class"), file("Unrelated.class")) + + when: + b.text = "package b; class B {}" + run language.compileTaskName + + then: + outputs.recompiledClasses("A", "C") + } + + def "overwritten classes are restored on the compile failure and incremental compilation works after a failure"() { + source("package a; class A {}", "package c; class C {}", "class Unrelated {}") + def b = source("package b; class B {}") + outputs.snapshot { run language.compileTaskName } + + when: + // Compile more classes, so there is possibility some is generated before a compile failure. + // Since order of passed classes to javac is unstable we can compile classes in different order on different platforms. + // Compile .java also for Groovy, so the file is written before failure. + file("src/main/$languageName/a/A1.java").text = "package a; class A { void m1() {} }" + b.text = "package b; $compileStaticAnnotation class B { B getB() { return 0; } }" + file("src/main/$languageName/c/C1.java").text = "package c; class C { void m1() {} }" + runAndFail language.compileTaskName + + then: + outputs.noneRecompiled() + outputs.hasFiles(file("a/A.class"), file("b/B.class"), file("c/C.class"), file("Unrelated.class")) + + when: + b.text = "package b; class B {}" + run language.compileTaskName + + then: + outputs.recompiledClasses("A", "C") } def "incremental compilation works after a compile failure"() { @@ -110,7 +201,6 @@ abstract class BaseIncrementalCompilationAfterFailureIntegrationTest extends Abs def "writes relative paths to source-to-class mapping after incremental compilation"() { given: - assumeFalse("Source-to-class mapping is not written for CLI compiler", this.class == JavaSourceCliIncrementalCompilationIntegrationTest.class) source("package test.gradle; class A {}", "package test2.gradle; class B {}") outputs.snapshot { run language.compileTaskName } def previousCompilationDataFile = file("build/tmp/${language.compileTaskName}/previous-compilation-data.bin") @@ -306,12 +396,41 @@ class JavaIncrementalCompilationAfterFailureIntegrationTest extends BaseIncremen "with inferred module-path" | "true" | "[]" | "" "with manual module-path" | "false" | "[\"--module-path=\${classpath.join(File.pathSeparator)}\"]" | "classpath = layout.files()" } + + def "incremental compilation after failure works with header output"() { + given: + source("class Unrelated {}") + def b = source("package b; class B { public native void foo(); }") + succeeds "compileJava" + + when: + outputs.snapshot { + source("package a; class A { public native void foo(); }") + b.text = "package b; class B { public native void foo(); String m1() { return 0; } }" + source("package c; class C { public native void foo(); }") + } + + then: + runAndFail "compileJava" + outputs.noneRecompiled() + + when: + b.text = 'package b; class B { public native void foo(); String m1() { return ""; } }' + run "compileJava" + + then: + outputs.recompiledClasses("A", "B", "C") + } } class GroovyIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrementalCompilationAfterFailureIntegrationTest { + + private static final COMPILE_STATIC_ANNOTATION = "@groovy.transform.CompileStatic" + CompiledLanguage language = CompiledLanguage.GROOVY def setup() { + compileStaticAnnotation = COMPILE_STATIC_ANNOTATION configureGroovyIncrementalCompilation() } @@ -380,4 +499,48 @@ class GroovyIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrem compileTransactionDir.exists() stashDirClasses() == ["AppTest.class", "AppTest\$_some_test_closure1.class", "AppTest\$_some_test_closure1\$_closure2.class", "BaseTest.class"] as Set } + + /** + * While it's possible to reproduce a compilation failure after some .class is written to a disk for Java, it's much harder for Groovy, + * since Groovy compiler will first analyze all classes and only then write all on disk. While Java compiler can also generate classes before other are analysed. + * + * That is why we test restoring overwritten and deleting new files just with Java files. + */ + def 'incremental compilation after a failure works with mix java/groovy sources when #description compilation fails and new classes are deleted and overwritten classes are restored'() { + given: + sourceWithFileSuffix("groovy", "package a; class A {}") + File b = sourceWithFileSuffix(fileWithErrorSuffix, "package b; class B { }") + sourceWithFileSuffix("groovy", "package c; class C {}") + run "compileGroovy" + + when: + outputs.snapshot { + file("src/main/groovy/a/A1.java").text = "package a; class A { void m1() {} }" + b.text = "package b; $compileErrorClassAnnotation class B { B m1() { return 0; }; }" + file("src/main/groovy/c/C1.java").text = "package c; class C { void m1() {} }" + sourceWithFileSuffix("java", "package d; class D {}") + } + + then: + runAndFail "compileGroovy" + outputs.noneRecompiled() + + when: + outputs.snapshot { + b.text = "package b; class B { B m1() { return new B(); }; }" + } + file("src/main/groovy/a/A1.java").delete() + file("src/main/groovy/c/C1.java").delete() + run "compileGroovy" + + then: + outputs.recompiledClasses("B", "D") + + where: + // We must trigger failure in a "Semantic Analysis" stage and not in parse stage, otherwise compiler won't output anything on a disk. + // So for example "class A { garbage }" will cause compiler to fail very early and nothing will be written on a disk. + description | fileWithErrorSuffix | compileErrorClassAnnotation + "Java" | "java" | "" + "Groovy" | "groovy" | COMPILE_STATIC_ANNOTATION + } } diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy index 26340b051a90..91bda57b036e 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy @@ -120,6 +120,41 @@ abstract class CrossTaskIncrementalJavaCompilationIntegrationTest extends Abstra "with manual module-path" | "false" | "[\"--module-path=\${classpath.join(File.pathSeparator)}\"]" | "classpath = layout.files()" } + @Requires(TestPrecondition.JDK9_OR_LATER) + def "incremental compilation detects if some exported package for compiled module was deleted #description"() { + file("impl/build.gradle") << """ + def layout = project.layout + tasks.compileJava { + modularity.inferModulePath = $inferModulePath + options.compilerArgs.addAll($compileArgs) + doFirst { + $doFirst + } + } + """ + source impl: ["package a; public class A {}", "package b; public class B {}"] + file("impl/src/main/$languageName/module-info.$languageName").text = """ + module impl { + exports a; + exports b; + } + """ + succeeds "impl:${language.compileTaskName}" + + when: + impl.snapshot { file("impl/src/main/$languageName/b/B.$languageName").delete() } + + then: + runAndFail "impl:${language.compileTaskName}", "--info" + impl.noneRecompiled() + result.hasErrorOutput("module-info.java:4: error: package is empty or does not exist: b") + + 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") << """ diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/ApiCompilerResult.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/ApiCompilerResult.java index e69f36a7bac4..7c8c8b3a3c3c 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/ApiCompilerResult.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/ApiCompilerResult.java @@ -29,6 +29,7 @@ public class ApiCompilerResult extends DefaultWorkResult { private final AnnotationProcessingResult annotationProcessingResult = new AnnotationProcessingResult(); private final ConstantsAnalysisResult constantsAnalysisResult = new ConstantsAnalysisResult(); private final Map> sourceToClassMapping = new HashMap<>(); + private final Map backupClassFiles = new HashMap<>(); public ApiCompilerResult() { super(true, null); @@ -45,4 +46,8 @@ public ConstantsAnalysisResult getConstantsAnalysisResult() { public Map> getSourceClassesMapping() { return sourceToClassMapping; } + + public Map getBackupClassFiles() { + return backupClassFiles; + } } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationClassBackupService.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationClassBackupService.java new file mode 100644 index 000000000000..1583ceeea9d7 --- /dev/null +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationClassBackupService.java @@ -0,0 +1,91 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle.api.internal.tasks.compile; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Set; +import java.util.concurrent.atomic.AtomicLong; + +/** + * A service that backups a class just before new version of a class is generated. + * This is used for restoring compiler outputs on compilation failure. + * + * Service only backups classes that: + * - exists before compilation + * - were not deleted before compilation + * - javac tries to overwrite during compilation. + * + * This service is called by compiler through Compiler API. + * + * We don't backup classes/resources generated by annotation processors, since any overwriting of existing generated types + * that were not deleted by Gradle before annotation processing, would anyway result in full-recompilation, due to annotation processors limitations. + * And in case of a Groovy/Java joint compilation, incremental compilation is anyway disabled, when annotation processing is present. + */ +public class CompilationClassBackupService { + + private final Set classesToCompile; + private final File destinationDir; + private final File headerOutputDir; + private final File classBackupDir; + private final ApiCompilerResult result; + private final AtomicLong uniqueIndex; + private final boolean shouldBackupFiles; + + public CompilationClassBackupService(JavaCompileSpec spec, ApiCompilerResult result) { + this.classesToCompile = spec.getClassesToCompile(); + this.destinationDir = spec.getDestinationDir(); + this.headerOutputDir = spec.getCompileOptions().getHeaderOutputDirectory(); + this.classBackupDir = spec.getClassBackupDir(); + this.shouldBackupFiles = classBackupDir != null; + this.result = result; + this.uniqueIndex = new AtomicLong(); + } + + public void maybeBackupClassFile(String classFqName) { + // Classes to compile are stashed before the compilation, so there is nothing to backup + if (shouldBackupFiles && !classesToCompile.contains(classFqName)) { + String classFilePath = classFqName.replace(".", "/").concat(".class"); + maybeBackupFile(destinationDir, classFilePath); + if (headerOutputDir != null) { + String headerFilePath = classFqName.replaceAll("[.$]", "_").concat(".h"); + maybeBackupFile(headerOutputDir, headerFilePath); + } + } + } + + private void maybeBackupFile(File destinationDir, String relativePath) { + File classFile = new File(destinationDir, relativePath); + if (!result.getBackupClassFiles().containsKey(classFile.getAbsolutePath()) && classFile.exists()) { + File backupFile = new File(classBackupDir, classFile.getName() + uniqueIndex.incrementAndGet()); + copy(classFile.toPath(), backupFile.toPath()); + result.getBackupClassFiles().put(classFile.getAbsolutePath(), backupFile.getAbsolutePath()); + } + } + + private static void copy(Path from, Path to) { + try { + Files.copy(from, to, StandardCopyOption.COPY_ATTRIBUTES); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} diff --git a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java similarity index 64% rename from subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java rename to subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java index 3fae6a5c8107..33d8c9798217 100644 --- a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFailedException.java @@ -1,5 +1,5 @@ /* - * Copyright 2011 the original author or authors. + * Copyright 2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,16 +15,33 @@ */ package org.gradle.api.internal.tasks.compile; +import javax.annotation.Nullable; +import java.util.Optional; + public class CompilationFailedException extends RuntimeException { + + private final ApiCompilerResult compilerPartialResult; + public CompilationFailedException() { + this((ApiCompilerResult) null); + } + + public CompilationFailedException(@Nullable ApiCompilerResult compilerPartialResult) { super("Compilation failed; see the compiler error output for details."); + this.compilerPartialResult = compilerPartialResult; } public CompilationFailedException(int exitCode) { super(String.format("Compilation failed with exit code %d; see the compiler error output for details.", exitCode)); + this.compilerPartialResult = null; } public CompilationFailedException(Throwable cause) { super(cause); + this.compilerPartialResult = null; + } + + public Optional getCompilerPartialResult() { + return Optional.ofNullable(compilerPartialResult); } } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java index 5bffca0835dd..e21f70bd8ae3 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java @@ -19,9 +19,11 @@ import org.gradle.api.internal.tasks.compile.processing.AnnotationProcessorDeclaration; import org.gradle.api.tasks.compile.CompileOptions; +import javax.annotation.Nullable; import java.io.File; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -31,16 +33,28 @@ public class DefaultJavaCompileSpec extends DefaultJvmLanguageCompileSpec implem private MinimalJavaCompileOptions compileOptions; private List annotationProcessorPath; private Set effectiveAnnotationProcessors; - private Set classes; + private Set classesToProcess; private List modulePath; private List sourceRoots; - private boolean isIncrementalCompilationOfJavaModule; + private Set classesToCompile = Collections.emptySet(); + private File backupDestinationDir; @Override public MinimalJavaCompileOptions getCompileOptions() { return compileOptions; } + @Nullable + @Override + public File getClassBackupDir() { + return backupDestinationDir; + } + + @Override + public void setClassBackupDir(@Nullable File classBackupDir) { + this.backupDestinationDir = classBackupDir; + } + public void setCompileOptions(CompileOptions compileOptions) { this.compileOptions = new MinimalJavaCompileOptions(compileOptions); } @@ -66,13 +80,23 @@ public void setEffectiveAnnotationProcessors(Set } @Override - public Set getClasses() { - return classes; + public Set getClassesToProcess() { + return classesToProcess; + } + + @Override + public void setClassesToProcess(Set classes) { + this.classesToProcess = classes; } @Override - public void setClasses(Set classes) { - this.classes = classes; + public void setClassesToCompile(Set classes) { + this.classesToCompile = classes; + } + + @Override + public Set getClassesToCompile() { + return classesToCompile; } @Override @@ -106,16 +130,6 @@ public void setModulePath(List modulePath) { this.modulePath = modulePath; } - @Override - public boolean isIncrementalCompilationOfJavaModule() { - return isIncrementalCompilationOfJavaModule; - } - - @Override - public void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule) { - this.isIncrementalCompilationOfJavaModule = isIncrementalCompilationOfJavaModule; - } - @Override public List getSourceRoots() { return sourceRoots; diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/IncrementalCompilationAwareJavaCompiler.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/IncrementalCompilationAwareJavaCompiler.java index ba9b858fe713..d3540da0fcb8 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/IncrementalCompilationAwareJavaCompiler.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/IncrementalCompilationAwareJavaCompiler.java @@ -22,5 +22,11 @@ import java.util.Set; public interface IncrementalCompilationAwareJavaCompiler extends JavaCompiler { - JavaCompiler.CompilationTask makeIncremental(JavaCompiler.CompilationTask task, Map> sourceToClassMapping, ConstantsAnalysisResult constantsAnalysisResult, CompilationSourceDirs compilationSourceDirs); + JavaCompiler.CompilationTask makeIncremental( + JavaCompiler.CompilationTask task, + Map> sourceToClassMapping, + ConstantsAnalysisResult constantsAnalysisResult, + CompilationSourceDirs compilationSourceDirs, + CompilationClassBackupService classBackupService + ); } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java index 758dbe33ad34..719bf43a0316 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java @@ -18,16 +18,23 @@ import org.gradle.api.internal.tasks.compile.processing.AnnotationProcessorDeclaration; +import javax.annotation.Nullable; import java.io.File; import java.util.List; import java.util.Set; public interface JavaCompileSpec extends JvmLanguageCompileSpec { + MinimalJavaCompileOptions getCompileOptions(); @Override File getDestinationDir(); + @Nullable + File getClassBackupDir(); + + void setClassBackupDir(@Nullable File classBackupDir); + /** * The annotation processor path to use. When empty, no processing should be done. When not empty, processing should be done. */ @@ -39,17 +46,26 @@ public interface JavaCompileSpec extends JvmLanguageCompileSpec { Set getEffectiveAnnotationProcessors(); - void setClasses(Set classes); + void setClassesToProcess(Set classes); - Set getClasses(); + /** + * Classes to process are already compiled classes that are passed to Java compiler. + * They are passed to Java compiler since they are required by some annotation processor to revisit. + */ + Set getClassesToProcess(); - List getModulePath(); + void setClassesToCompile(Set classes); - void setModulePath(List modulePath); + /** + * Classes to compile are all classes that we know from Java sources that will be compiled. + * These classes are deleted before a compilation and are not passed to Java compiler (their sources are passed to a compiler). + * We only need them in {@link CompilationClassBackupService} so we know what files don't need a backup. + */ + Set getClassesToCompile(); - boolean isIncrementalCompilationOfJavaModule(); + List getModulePath(); - void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule); + void setModulePath(List modulePath); default boolean annotationProcessingConfigured() { return !getAnnotationProcessorPath().isEmpty() && !getCompileOptions().getCompilerArgs().contains("-proc:none"); diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java index c0c7cadb0a67..f7a43b7ee64f 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java @@ -25,13 +25,11 @@ 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; @@ -40,6 +38,7 @@ public class JdkJavaCompiler implements Compiler, Serializable { private static final Logger LOGGER = LoggerFactory.getLogger(JdkJavaCompiler.class); + private final Factory javaHomeBasedJavaCompilerFactory; @Inject @@ -55,7 +54,7 @@ public WorkResult execute(JavaCompileSpec spec) { JavaCompiler.CompilationTask task = createCompileTask(spec, result); boolean success = task.call(); if (!success) { - throw new CompilationFailedException(); + throw new CompilationFailedException(result); } return result; } @@ -68,11 +67,16 @@ private JavaCompiler.CompilationTask createCompileTask(JavaCompileSpec spec, Api StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, charset); Iterable compilationUnits = standardFileManager.getJavaFileObjectsFromFiles(spec.getSourceFiles()); boolean hasEmptySourcepaths = JavaVersion.current().isJava9Compatible() && emptySourcepathIn(options); - 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); + JavaFileManager fileManager = GradleStandardJavaFileManager.wrap(standardFileManager, DefaultClassPath.of(spec.getAnnotationProcessorPath()), hasEmptySourcepaths); + JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, spec.getClassesToProcess(), compilationUnits); if (compiler instanceof IncrementalCompilationAwareJavaCompiler) { - task = ((IncrementalCompilationAwareJavaCompiler) compiler).makeIncremental(task, result.getSourceClassesMapping(), result.getConstantsAnalysisResult(), new CompilationSourceDirs(spec)); + task = ((IncrementalCompilationAwareJavaCompiler) compiler).makeIncremental( + task, + result.getSourceClassesMapping(), + result.getConstantsAnalysisResult(), + new CompilationSourceDirs(spec), + new CompilationClassBackupService(spec, result) + ); } Set annotationProcessors = spec.getEffectiveAnnotationProcessors(); task = new AnnotationProcessingCompileTask(task, annotationProcessors, spec.getAnnotationProcessorPath(), result.getAnnotationProcessingResult()); @@ -90,12 +94,4 @@ private static boolean emptySourcepathIn(List 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; - } } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkTools.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkTools.java index 35728c1d100e..6249192be372 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkTools.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkTools.java @@ -165,10 +165,14 @@ public int isSupportedOption(String option) { } @Override - public JavaCompiler.CompilationTask makeIncremental(JavaCompiler.CompilationTask task, Map> sourceToClassMapping, ConstantsAnalysisResult constantsAnalysisResult, CompilationSourceDirs compilationSourceDirs) { + public JavaCompiler.CompilationTask makeIncremental(JavaCompiler.CompilationTask task, Map> sourceToClassMapping, + ConstantsAnalysisResult constantsAnalysisResult, CompilationSourceDirs compilationSourceDirs, + CompilationClassBackupService classBackupService + ) { ensureCompilerTask(); return DirectInstantiator.instantiate(incrementalCompileTaskClass, task, (Function>) compilationSourceDirs::relativize, + (Consumer) classBackupService::maybeBackupClassFile, (Consumer>>) sourceToClassMapping::putAll, (BiConsumer) constantsAnalysisResult::addPublicDependent, (BiConsumer) constantsAnalysisResult::addPrivateDependent diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java index 4c1ad41c1711..4e99c6705c98 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/SelectiveCompiler.java @@ -93,7 +93,7 @@ public WorkResult execute(T spec) { CompileTransaction transaction = recompilationSpecProvider.initCompilationSpecAndTransaction(spec, recompilationSpec); return transaction.execute(workResult -> { - if (Iterables.isEmpty(spec.getSourceFiles()) && spec.getClasses().isEmpty()) { + if (Iterables.isEmpty(spec.getSourceFiles()) && spec.getClassesToProcess().isEmpty()) { LOG.info("None of the classes needs to be compiled! Analysis took {}. ", clock.getElapsed()); return new RecompilationNotNecessary(previousCompilationData, recompilationSpec); } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java index 0b0fbd8d40a5..d6e30557815d 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java @@ -206,10 +206,7 @@ private static void addModuleInfoToCompile(RecompilationSpec spec, SourceFileCla Set 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 + // This solves case for incremental compilation where some package was deleted and exported in module-info, but compilation doesn't fail. spec.addClassToCompile(MODULE_INFO_CLASS_NAME); spec.addSourcePaths(moduleInfoSources); } @@ -219,7 +216,7 @@ private static void addModuleInfoToCompile(RecompilationSpec spec, SourceFileCla public CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec, RecompilationSpec recompilationSpec) { if (!recompilationSpec.isBuildNeeded()) { spec.setSourceFiles(ImmutableSet.of()); - spec.setClasses(Collections.emptySet()); + spec.setClassesToProcess(Collections.emptySet()); return new CompileTransaction(spec, fileOperations.patternSet(), ImmutableMap.of(), fileOperations, deleter); } @@ -230,8 +227,8 @@ public CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec spec.setSourceFiles(narrowDownSourcesToCompile(sourceTree, sourceToCompile)); includePreviousCompilationOutputOnClasspath(spec); addClassesToProcess(spec, recompilationSpec); + spec.setClassesToCompile(recompilationSpec.getClassesToCompile()); Map resourcesToDelete = prepareResourcePatterns(recompilationSpec.getResourcesToGenerate(), fileOperations); - spec.setIsIncrementalCompilationOfJavaModule(recompilationSpec.hasClassToCompile(MODULE_INFO_CLASS_NAME)); return new CompileTransaction(spec, classesToDelete, resourcesToDelete, fileOperations, deleter); } @@ -264,7 +261,7 @@ private static void prepareFilePatterns(Collection staleClasses, Collect private static void addClassesToProcess(JavaCompileSpec spec, RecompilationSpec recompilationSpec) { Set classesToProcess = new HashSet<>(recompilationSpec.getClassesToProcess()); classesToProcess.removeAll(recompilationSpec.getClassesToCompile()); - spec.setClasses(classesToProcess); + spec.setClassesToProcess(classesToProcess); } private static void includePreviousCompilationOutputOnClasspath(JavaCompileSpec spec) { diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java index 24df64e4436a..6bd8732cb9bb 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java @@ -55,10 +55,6 @@ public Set getClassesToCompile() { return Collections.unmodifiableSet(classesToCompile); } - public boolean hasClassToCompile(String className) { - return classesToCompile.contains(className); - } - public void addClassToReprocess(String classToReprocess) { classesToProcess.add(classToReprocess); } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransaction.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransaction.java index 6cba9fbb3748..24f15891d09f 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransaction.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransaction.java @@ -17,8 +17,10 @@ package org.gradle.api.internal.tasks.compile.incremental.transaction; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableSet; import org.gradle.api.UncheckedIOException; import org.gradle.api.internal.file.FileOperations; +import org.gradle.api.internal.tasks.compile.ApiCompilerResult; import org.gradle.api.internal.tasks.compile.CompilationFailedException; import org.gradle.api.internal.tasks.compile.JavaCompileSpec; import org.gradle.api.internal.tasks.compile.incremental.compilerapi.deps.GeneratedResource; @@ -27,6 +29,8 @@ import org.gradle.api.tasks.util.PatternSet; import org.gradle.internal.file.Deleter; import org.gradle.language.base.internal.tasks.StaleOutputCleaner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.io.File; @@ -34,14 +38,14 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; -import java.util.Comparator; +import java.util.EnumMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -56,6 +60,8 @@ */ public class CompileTransaction { + private static final Logger LOG = LoggerFactory.getLogger(CompileTransaction.class); + private final Deleter deleter; private final FileOperations fileOperations; private final PatternSet classesToDelete; @@ -63,6 +69,7 @@ public class CompileTransaction { private final Map resourcesToDelete; private final File stashDirectory; private final File tempDir; + private final File backupDirectory; public CompileTransaction( JavaCompileSpec spec, @@ -74,6 +81,7 @@ public CompileTransaction( this.spec = spec; this.tempDir = new File(spec.getTempDir(), "compileTransaction"); this.stashDirectory = new File(tempDir, "stash-dir"); + this.backupDirectory = new File(tempDir, "backup-dir"); this.classesToDelete = classesToDelete; this.resourcesToDelete = resourcesToDelete; this.fileOperations = fileOperations; @@ -87,29 +95,32 @@ public CompileTransaction( * Execution steps:
* 1. At start create empty temporary directories or make sure they are empty
* 2. Stash all files that should be deleted from compiler destination directories to a temporary directories
- * 3. Change the compiler destination directories to a temporary directories (different from the stash step)
- * 4. a. In case of a success copy compiler outputs to original destination directories
- * b. In case of a failure restore stashed files back to original destination directories
+ * 3. a. In case of a success do nothing
+ * b. In case of a failure delete generated files and restore stashed files
*/ public T execute(Function function) { - List stagedOutputs = collectOutputsToStage(); - ensureEmptyDirectoriesBeforeExecution(stagedOutputs); - StashResult stashResult = stashFilesThatShouldBeDeleted(); + ensureEmptyDirectoriesBeforeExecution(); + List stashedFiles = stashFilesThatShouldBeDeleted(); try { - setupSpecOutputs(stagedOutputs); - T result = function.apply(stashResult.mapToWorkResult()); - deletePotentiallyEmptyDirectories(stashResult); - moveCompileOutputToOriginalFolders(stagedOutputs); + if (supportsIncrementalCompilationAfterFailure()) { + spec.setClassBackupDir(backupDirectory); + } + T result = function.apply(WorkResults.didWork(!stashedFiles.isEmpty())); + deleteEmptyDirectoriesAfterCompilation(stashedFiles); return result; - } catch (CompilationFailedException t) { - rollbackStash(stashResult.stashedFiles); - throw t; - } finally { - restoreSpecOutputs(stagedOutputs); + } catch (CompilationFailedException e) { + if (supportsIncrementalCompilationAfterFailure()) { + rollback(stashedFiles, e.getCompilerPartialResult().orElse(null)); + } + throw e; } } - private void ensureEmptyDirectoriesBeforeExecution(List stagedOutputs) { + private boolean supportsIncrementalCompilationAfterFailure() { + return spec.getCompileOptions().supportsIncrementalCompilationAfterFailure(); + } + + private void ensureEmptyDirectoriesBeforeExecution() { try { tempDir.mkdirs(); @@ -117,10 +128,8 @@ private void ensureEmptyDirectoriesBeforeExecution(List stagedOutp Set ensureEmptyDirectories = new HashSet<>(); deleter.ensureEmptyDirectory(stashDirectory); ensureEmptyDirectories.add(stashDirectory); - for (StagedOutput output : stagedOutputs) { - ensureEmptyKeepingFolderStructure(output); - ensureEmptyDirectories.add(output.stagingDirectory); - } + deleter.ensureEmptyDirectory(backupDirectory); + ensureEmptyDirectories.add(backupDirectory); // Delete any other file or directory try (Stream dirStream = Files.list(tempDir.toPath())) { @@ -133,29 +142,6 @@ private void ensureEmptyDirectoriesBeforeExecution(List stagedOutp } } - private void ensureEmptyKeepingFolderStructure(StagedOutput output) throws IOException { - Path currentDir = output.stagingDirectory.toPath(); - if (!Files.exists(currentDir)) { - Files.createDirectory(currentDir); - return; - } - try (Stream dirStream = Files.walk(currentDir)) { - // Delete all files and delete all directories - // that don't exist in sourceDirectory - dirStream - // Order files in a direction that we can avoid recursive deletion - .sorted(Comparator.reverseOrder()) - .filter(path -> !Files.isDirectory(path) || !isDirectoryAlsoInOtherRoot(path, currentDir, output.sourceDirectory)) - .forEach(path -> path.toFile().delete()); - } - } - - private boolean isDirectoryAlsoInOtherRoot(Path directory, Path root, File otherRoot) { - Path relativePath = root.relativize(directory); - File fileInOtherRoot = new File(otherRoot, relativePath.toString()); - return fileInOtherRoot.isDirectory(); - } - private void deleteRecursively(File file) { try { deleter.deleteRecursively(file); @@ -164,94 +150,118 @@ private void deleteRecursively(File file) { } } - private List collectOutputsToStage() { - List stagedOutputs = new ArrayList<>(); - stagedOutputs.add(new StagedOutput(spec.getDestinationDir(), new File(tempDir, "compile-output"), spec::setDestinationDir)); + private List stashFilesThatShouldBeDeleted() { + int uniqueId = 0; + List stashedFiles = new ArrayList<>(); + for (File fileToDelete : collectFilesToDelete(classesToDelete, resourcesToDelete)) { + File stashedFile = new File(stashDirectory, fileToDelete.getName() + ".uniqueId" + uniqueId++); + moveFile(fileToDelete, stashedFile); + stashedFiles.add(new StashedFile(fileToDelete, stashedFile)); + } + return stashedFiles; + } + + private void deleteEmptyDirectoriesAfterCompilation(List stashedFiles) { + ImmutableSet outputDirectories = getOutputDirectories(); + Set potentiallyEmptyFolders = stashedFiles.stream() + .map(file -> file.sourceFile.getParentFile()) + .collect(Collectors.toSet()); + StaleOutputCleaner.cleanEmptyOutputDirectories(deleter, potentiallyEmptyFolders, outputDirectories); + } - File annotationOutputDir = spec.getCompileOptions().getAnnotationProcessorGeneratedSourcesDirectory(); - if (annotationOutputDir != null) { - StagedOutput stagedOutput = new StagedOutput(annotationOutputDir, new File(tempDir, "annotation-output"), file -> spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(file)); - stagedOutputs.add(stagedOutput); + private void rollback(List stashResult, @Nullable ApiCompilerResult compilerResult) { + if (compilerResult != null) { + deleteGeneratedFiles(compilerResult); + rollbackOverwrittenFiles(compilerResult); } + rollbackStashedFiles(stashResult); + } - File headerOutputDir = spec.getCompileOptions().getHeaderOutputDirectory(); - if (spec.getCompileOptions().getHeaderOutputDirectory() != null) { - StagedOutput stagedOutput = new StagedOutput(headerOutputDir, new File(tempDir, "header-output"), file -> spec.getCompileOptions().setHeaderOutputDirectory(file)); - stagedOutputs.add(stagedOutput); + private void deleteGeneratedFiles(ApiCompilerResult compilerResult) { + PatternSet classesToDelete = getNewGeneratedClasses(compilerResult); + Map resourcesToDelete = getNewGeneratedResources(compilerResult); + Set filesToDelete = collectFilesToDelete(classesToDelete, resourcesToDelete); + StaleOutputCleaner.cleanOutputs(deleter, filesToDelete, getOutputDirectories()); + if (LOG.isDebugEnabled()) { + LOG.debug("Deleting generated files: {}", filesToDelete.stream().sorted().collect(Collectors.toList())); } - return stagedOutputs; } - private StashResult stashFilesThatShouldBeDeleted() { - int uniqueId = 0; + private PatternSet getNewGeneratedClasses(ApiCompilerResult result) { + PatternSet filesToDelete = fileOperations.patternSet(); + result.getSourceClassesMapping().values().stream() + .flatMap(Collection::stream) + .forEach(className -> { + filesToDelete.include(className.replace(".", "/").concat(".class")); + filesToDelete.include(className.replaceAll("[.$]", "_").concat(".h")); + }); + Set annotationProcessorTypes = new HashSet<>(result.getAnnotationProcessingResult().getGeneratedAggregatingTypes()); + result.getAnnotationProcessingResult().getGeneratedTypesWithIsolatedOrigin().values().stream() + .flatMap(Collection::stream) + .forEach(annotationProcessorTypes::add); + annotationProcessorTypes.forEach(className -> { + filesToDelete.include(className.replace(".", "/").concat(".class")); + filesToDelete.include(className.replaceAll("[.$]", "_").concat(".h")); + filesToDelete.include(className.replace(".", "/").concat(".java")); + }); + return filesToDelete; + } + + private Map getNewGeneratedResources(ApiCompilerResult result) { + Map resourcesByLocation = new EnumMap<>(GeneratedResource.Location.class); + Stream.of(GeneratedResource.Location.values()).forEach(location -> resourcesByLocation.put(location, fileOperations.patternSet())); + result.getAnnotationProcessingResult() + .getGeneratedAggregatingResources() + .forEach(resource -> resourcesByLocation.get(resource.getLocation()).include(resource.getPath())); + result.getAnnotationProcessingResult().getGeneratedResourcesWithIsolatedOrigin().values().stream() + .flatMap(Collection::stream) + .forEach(resource -> resourcesByLocation.get(resource.getLocation()).include(resource.getPath())); + return resourcesByLocation; + } + + private Set collectFilesToDelete(PatternSet classesToDelete, Map resourcesToDelete) { File compileOutput = spec.getDestinationDir(); File annotationProcessorOutput = spec.getCompileOptions().getAnnotationProcessorGeneratedSourcesDirectory(); File headerOutput = spec.getCompileOptions().getHeaderOutputDirectory(); - List stashedFiles = new ArrayList<>(); - for (File sourceFile : collectFilesToStash(compileOutput, annotationProcessorOutput, headerOutput)) { - File stashedFile = new File(stashDirectory, sourceFile.getName() + ".uniqueId" + uniqueId++); - moveFile(sourceFile, stashedFile); - stashedFiles.add(new StashedFile(sourceFile, stashedFile)); - } - List sourceDirectories = Stream.of(compileOutput, annotationProcessorOutput, headerOutput) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - return new StashResult(sourceDirectories, stashedFiles); - } - - private Set collectFilesToStash(File compileOutput, @Nullable File annotationProcessorOutput, @Nullable File headerOutput) { - Set filesToStash = new HashSet<>(); - filesToStash.addAll(collectFilesToStash(classesToDelete, compileOutput)); - filesToStash.addAll(collectFilesToStash(classesToDelete, annotationProcessorOutput)); - filesToStash.addAll(collectFilesToStash(classesToDelete, headerOutput)); - filesToStash.addAll(collectFilesToStash(resourcesToDelete.get(CLASS_OUTPUT), compileOutput)); + Set filesToDelete = new HashSet<>(); + filesToDelete.addAll(collectFilesToDelete(classesToDelete, compileOutput)); + filesToDelete.addAll(collectFilesToDelete(classesToDelete, annotationProcessorOutput)); + filesToDelete.addAll(collectFilesToDelete(classesToDelete, headerOutput)); + filesToDelete.addAll(collectFilesToDelete(resourcesToDelete.get(CLASS_OUTPUT), compileOutput)); // If the client has not set a location for SOURCE_OUTPUT, javac outputs those files to the CLASS_OUTPUT directory, so delete that instead. - filesToStash.addAll(collectFilesToStash(resourcesToDelete.get(SOURCE_OUTPUT), MoreObjects.firstNonNull(annotationProcessorOutput, compileOutput))); + filesToDelete.addAll(collectFilesToDelete(resourcesToDelete.get(SOURCE_OUTPUT), MoreObjects.firstNonNull(annotationProcessorOutput, compileOutput))); // In the same situation with NATIVE_HEADER_OUTPUT, javac just NPEs. Don't bother. - filesToStash.addAll(collectFilesToStash(resourcesToDelete.get(NATIVE_HEADER_OUTPUT), headerOutput)); - return filesToStash; + filesToDelete.addAll(collectFilesToDelete(resourcesToDelete.get(NATIVE_HEADER_OUTPUT), headerOutput)); + return filesToDelete; } - private Set collectFilesToStash(PatternSet patternSet, File sourceDirectory) { + private Set collectFilesToDelete(PatternSet patternSet, File sourceDirectory) { if (patternSet != null && !patternSet.isEmpty() && sourceDirectory != null && sourceDirectory.exists()) { return fileOperations.fileTree(sourceDirectory).matching(patternSet).getFiles(); } return Collections.emptySet(); } - private void deletePotentiallyEmptyDirectories(StashResult stashResult) { - Set potentiallyEmptyFolders = stashResult.stashedFiles.stream() - .map(file -> file.sourceFile.getParentFile()) - .collect(Collectors.toSet()); - StaleOutputCleaner.cleanEmptyOutputDirectories(deleter, potentiallyEmptyFolders, stashResult.sourceDirectories); - } - - private void moveCompileOutputToOriginalFolders(List stagedOutputs) { - stagedOutputs.forEach(StagedOutput::unstage); - } - - private void rollbackStash(List stashedFiles) { - if (supportsIncrementalCompilationAfterFailure()) { - stashedFiles.forEach(StashedFile::unstash); - } + private ImmutableSet getOutputDirectories() { + return Stream.of(spec.getDestinationDir(), spec.getCompileOptions().getAnnotationProcessorGeneratedSourcesDirectory(), spec.getCompileOptions().getHeaderOutputDirectory()) + .filter(Objects::nonNull) + .collect(ImmutableSet.toImmutableSet()); } - private void setupSpecOutputs(List stagedOutputs) { - if (supportsIncrementalCompilationAfterFailure()) { - stagedOutputs.forEach(StagedOutput::setupSpecOutput); + private static void rollbackOverwrittenFiles(ApiCompilerResult result) { + result.getBackupClassFiles().forEach((original, backup) -> moveFile(new File(backup), new File(original))); + if (LOG.isDebugEnabled()) { + LOG.debug("Restoring overwritten files: {}", result.getBackupClassFiles().keySet().stream().sorted().collect(Collectors.toList())); } } - private void restoreSpecOutputs(List stagedOutputs) { - if (supportsIncrementalCompilationAfterFailure()) { - stagedOutputs.forEach(StagedOutput::restoreSpecOutput); + private static void rollbackStashedFiles(List stashedFiles) { + stashedFiles.forEach(StashedFile::unstash); + if (LOG.isDebugEnabled()) { + LOG.debug("Restoring stashed files: {}", stashedFiles.stream().map(f -> f.sourceFile.getAbsolutePath()).sorted().collect(Collectors.toList())); } } - private boolean supportsIncrementalCompilationAfterFailure() { - return spec.getCompileOptions().supportsIncrementalCompilationAfterFailure(); - } - private static void moveFile(File sourceFile, File destinationFile) { try { destinationFile.getParentFile().mkdirs(); @@ -261,55 +271,6 @@ private static void moveFile(File sourceFile, File destinationFile) { } } - private static class StashResult { - - private final List sourceDirectories; - private final List stashedFiles; - - private StashResult(List sourceDirectories, List stashedFiles) { - this.sourceDirectories = sourceDirectories; - this.stashedFiles = stashedFiles; - } - - public WorkResult mapToWorkResult() { - return WorkResults.didWork(!stashedFiles.isEmpty()); - } - } - - private static class StagedOutput { - private final File sourceDirectory; - private final File stagingDirectory; - private final Consumer setSpecOutput; - - private StagedOutput(File sourceDirectory, File stagingDirectory, Consumer setSpecOutput) { - this.sourceDirectory = sourceDirectory; - this.stagingDirectory = stagingDirectory; - this.setSpecOutput = setSpecOutput; - } - - public void setupSpecOutput() { - setSpecOutput.accept(stagingDirectory); - } - - public void restoreSpecOutput() { - setSpecOutput.accept(sourceDirectory); - } - - public void unstage() { - Path stagingPath = stagingDirectory.toPath(); - try (Stream dirStream = Files.walk(stagingPath)) { - dirStream - .filter(Files::isRegularFile) - .forEach(path -> { - File newFile = new File(sourceDirectory, stagingPath.relativize(path).toString()); - moveFile(path.toFile(), newFile); - }); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - } - private static class StashedFile { private final File sourceFile; private final File stashFile; diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java index c221a1e527ee..71caa4d60eeb 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java @@ -16,20 +16,15 @@ package org.gradle.api.internal.tasks.compile.reflect; -import com.google.common.collect.Iterables; import org.gradle.internal.classpath.ClassPath; -import javax.annotation.Nullable; import javax.tools.ForwardingJavaFileManager; import javax.tools.JavaFileManager; import javax.tools.JavaFileObject; import javax.tools.StandardJavaFileManager; import javax.tools.StandardLocation; -import java.io.File; import java.io.IOException; -import java.io.UncheckedIOException; import java.net.URLClassLoader; -import java.util.Collections; import java.util.Set; import static org.gradle.api.internal.tasks.compile.filter.AnnotationProcessorFilter.getFilteredClassLoader; @@ -37,32 +32,19 @@ public class GradleStandardJavaFileManager extends ForwardingJavaFileManager { private final ClassPath annotationProcessorPath; private final boolean hasEmptySourcePaths; - private final boolean hasPreviousClassOutput; - private GradleStandardJavaFileManager(StandardJavaFileManager fileManager, ClassPath annotationProcessorPath, boolean hasEmptySourcePaths, @Nullable File previousClassOutput) { + private GradleStandardJavaFileManager(StandardJavaFileManager fileManager, ClassPath annotationProcessorPath, boolean hasEmptySourcePaths) { super(fileManager); this.annotationProcessorPath = annotationProcessorPath; this.hasEmptySourcePaths = hasEmptySourcePaths; - this.hasPreviousClassOutput = previousClassOutput != null; - registerPreviousClassOutput(previousClassOutput); - } - - private void registerPreviousClassOutput(@Nullable File previousClassOutput) { - if (previousClassOutput != null) { - try { - fileManager.setLocation(GradleLocation.PREVIOUS_CLASS_OUTPUT, Collections.singleton(previousClassOutput)); - } catch (IOException e) { - throw new UncheckedIOException("Problem registering previous class output location", e); - } - } } /** * Overrides particular methods to prevent javac from accessing source files outside of Gradle's understanding or * classloaders outside of Gradle's control. */ - public static JavaFileManager wrap(StandardJavaFileManager delegate, ClassPath annotationProcessorPath, boolean hasEmptySourcePaths, @Nullable File previousClassOutput) { - return new GradleStandardJavaFileManager(delegate, annotationProcessorPath, hasEmptySourcePaths, previousClassOutput); + public static JavaFileManager wrap(StandardJavaFileManager delegate, ClassPath annotationProcessorPath, boolean hasEmptySourcePaths) { + return new GradleStandardJavaFileManager(delegate, annotationProcessorPath, hasEmptySourcePaths); } @Override @@ -96,20 +78,6 @@ public Iterable list(Location location, String packageName, Set< kinds.remove(JavaFileObject.Kind.SOURCE); } } - - if (hasPreviousClassOutput && location.equals(StandardLocation.CLASS_OUTPUT)) { - // For Java module compilation we list also previous class output as class output. - // This is needed for incremental compilation after a failure where we change output folders. - // With that we make sure that all module classes/packages are found by javac. - // In case one of --module-source-path or --source-path is provided, this makes sure, that javac won't automatically recompile - // classes that are not in CLASS_OUTPUT. And in case when --module-source-path or --source-path are not provided, - // this makes sure that javac doesn't fail on missing packages or classes that are not in CLASS_OUTPUT. - // Second and last part of fix for: https://github.com/gradle/gradle/issues/23067 - Iterable previousClassOutput = super.list(GradleLocation.PREVIOUS_CLASS_OUTPUT, packageName, kinds, recurse); - Iterable classOutput = super.list(location, packageName, kinds, recurse); - return Iterables.concat(previousClassOutput, classOutput); - } - return super.list(location, packageName, kinds, recurse); } @@ -124,18 +92,4 @@ public ClassLoader getClassLoader(Location location) { return classLoader; } - - private enum GradleLocation implements Location { - PREVIOUS_CLASS_OUTPUT; - - @Override - public String getName() { - return name(); - } - - @Override - public boolean isOutputLocation() { - return false; - } - } } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java index 5aa22f684848..a9b9ebf1b9ee 100644 --- a/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java +++ b/subprojects/language-java/src/main/java/org/gradle/api/tasks/compile/JavaCompile.java @@ -301,7 +301,6 @@ DefaultJavaCompileSpec createSpec() { final DefaultJavaCompileSpec spec = createBaseSpec(); spec.setDestinationDir(getDestinationDirectory().getAsFile().get()); - spec.setOriginalDestinationDir(spec.getDestinationDir()); spec.setWorkingDir(getProjectLayout().getProjectDirectory().getAsFile()); spec.setTempDir(getTemporaryDir()); spec.setCompileClasspath(ImmutableList.copyOf(javaModuleDetector.inferClasspath(isModule, getClasspath()))); diff --git a/subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransactionTest.groovy b/subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransactionTest.groovy index 5be5d4874dc0..127ef896c34f 100644 --- a/subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransactionTest.groovy +++ b/subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/transaction/CompileTransactionTest.groovy @@ -17,6 +17,7 @@ package org.gradle.api.internal.tasks.compile.incremental.transaction import org.gradle.api.internal.file.TestFiles +import org.gradle.api.internal.tasks.compile.ApiCompilerResult import org.gradle.api.internal.tasks.compile.CompilationFailedException import org.gradle.api.internal.tasks.compile.DefaultJavaCompileSpec import org.gradle.api.internal.tasks.compile.JavaCompileSpec @@ -25,15 +26,19 @@ import org.gradle.api.tasks.WorkResults import org.gradle.api.tasks.compile.CompileOptions import org.gradle.api.tasks.util.PatternSet import org.gradle.util.TestUtil +import spock.lang.Shared import spock.lang.Specification import spock.lang.TempDir import java.nio.file.Files import java.nio.file.Path -import java.util.function.Function +import java.util.function.Supplier import java.util.stream.Stream import static com.google.common.base.Preconditions.checkNotNull +import static org.gradle.api.internal.tasks.compile.incremental.compilerapi.deps.GeneratedResource.Location.CLASS_OUTPUT +import static org.gradle.api.internal.tasks.compile.incremental.compilerapi.deps.GeneratedResource.Location.NATIVE_HEADER_OUTPUT +import static org.gradle.api.internal.tasks.compile.incremental.compilerapi.deps.GeneratedResource.Location.SOURCE_OUTPUT class CompileTransactionTest extends Specification { @@ -43,12 +48,15 @@ class CompileTransactionTest extends Specification { File temporaryFolder File transactionDir File stashDir + @Shared + File classBackupDir JavaCompileSpec spec def setup() { transactionDir = new File(temporaryFolder, "compileTransaction") transactionDir.mkdir() stashDir = new File(transactionDir, "stash-dir") + classBackupDir = new File(transactionDir, "backup-dir") spec = new DefaultJavaCompileSpec() spec.setTempDir(temporaryFolder) spec.setCompileOptions(new CompileOptions(TestUtil.objectFactory())) @@ -99,9 +107,9 @@ class CompileTransactionTest extends Specification { spec.getCompileOptions().setHeaderOutputDirectory(headerOutput) def classesToDelete = new PatternSet().include("**/*.class") Map sourcesToDelete = [:] - sourcesToDelete[GeneratedResource.Location.CLASS_OUTPUT] = new PatternSet().include("**/*.txt") - sourcesToDelete[GeneratedResource.Location.SOURCE_OUTPUT] = new PatternSet().include("**/*.ann") - sourcesToDelete[GeneratedResource.Location.NATIVE_HEADER_OUTPUT] = new PatternSet().include("**/*.h") + sourcesToDelete[CLASS_OUTPUT] = new PatternSet().include("**/*.txt") + sourcesToDelete[SOURCE_OUTPUT] = new PatternSet().include("**/*.ann") + sourcesToDelete[NATIVE_HEADER_OUTPUT] = new PatternSet().include("**/*.h") when: newCompileTransaction(classesToDelete, sourcesToDelete).execute { @@ -244,123 +252,117 @@ class CompileTransactionTest extends Specification { new PatternSet().include("**/*.txt") | null | "empty directory" } - def "on success all files are moved from staging dir to an output directory"() { - def destinationDir = spec.getDestinationDir() - def annotationOutput = createNewDirectory(file("annotationOut")) - spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(annotationOutput) - def headerOutput = createNewDirectory(file("headerOut")) - spec.getCompileOptions().setHeaderOutputDirectory(headerOutput) - def compileStagingDir = fileInTransactionDir("compile-output") - def annotationsStagingDir = fileInTransactionDir("annotation-output") - def headerStagingDir = fileInTransactionDir("header-output") + def "stash and backup directory are generated"() { + given: + spec.setDestinationDir(createNewFile(file("compile"))) + spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(createNewFile(file("annotation"))) + spec.getCompileOptions().setHeaderOutputDirectory(createNewFile(file("header"))) when: - newCompileTransaction().execute { - new File(compileStagingDir, "file.txt").createNewFile() - new File(compileStagingDir, "subDir").mkdir() - new File(compileStagingDir, "subDir/another-file.txt").createNewFile() - new File(annotationsStagingDir, "annotation-file.txt").createNewFile() - new File(headerStagingDir, "header-file.txt").createNewFile() - return DID_WORK + def directories = newCompileTransaction().execute { + return transactionDir.list() } then: - destinationDir.list() as Set ==~ ["file.txt", "subDir"] - new File(destinationDir,"subDir").list() as Set ==~ ["another-file.txt"] - annotationOutput.list() as Set ==~ ["annotation-file.txt"] - headerOutput.list() as Set ==~ ["header-file.txt"] + directories as Set ==~ ["stash-dir", "backup-dir"] } - def "on compile failure files are not moved to an output directory"() { - def destinationDir = createNewDirectory(file("someDir")) - def stagingDir = fileInTransactionDir("compile-output") - spec.setDestinationDir(destinationDir) + def "generated classes and resources are deleted and classes in backup dir are restored on a failure"() { + def destinationDir = spec.getDestinationDir() + def annotationGeneratedSourcesDir = createNewDirectory(file("generated/annotation-resources")) + def overwrittenFile = createNewFile(new File(destinationDir, "Overwritten.class")) + spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(annotationGeneratedSourcesDir) when: newCompileTransaction().execute { - new File(stagingDir, "file.txt").createNewFile() - new File(stagingDir, "subDir").mkdir() - new File(stagingDir, "subDir/another-file.txt").createNewFile() - throw new CompilationFailedException() - } + def compilerResult = simulateGeneratingClassesAndResources(destinationDir, annotationGeneratedSourcesDir, overwrittenFile) + throw new CompilationFailedException(compilerResult) + } then: thrown(CompilationFailedException) - isEmptyDirectory(destinationDir) + overwrittenFile.text == "" + destinationDir.list() as Set == ["Overwritten.class"] as Set + isEmptyDirectory(annotationGeneratedSourcesDir) } - def "unique directory is generated for stash directory and every staging directory that exists"() { - given: - spec.setDestinationDir(createNewFile(file("compile"))) - spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(createNewFile(file("annotation"))) - spec.getCompileOptions().setHeaderOutputDirectory(createNewFile(file("header"))) + def "generated classes and resources are not deleted and classes in backup dir are not restored on a success"() { + def destinationDir = spec.getDestinationDir() + def annotationGeneratedSourcesDir = createNewDirectory(file("generated/annotation-resources")) + def overwrittenFile = createNewFile(new File(destinationDir, "Overwritten.class")) + spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(annotationGeneratedSourcesDir) when: - def directories = newCompileTransaction().execute { - return transactionDir.list() + newCompileTransaction().execute { + return simulateGeneratingClassesAndResources(destinationDir, annotationGeneratedSourcesDir, overwrittenFile) } then: - directories as Set ==~ ["stash-dir", "compile-output", "annotation-output", "header-output"] + overwrittenFile.text == "Overwritten" + listAllFiles(destinationDir) == ["Overwritten.class", "com/example/A.class", "com/example/A.txt", "com/example/B.class", "com/example/C\$D.class", "com/example/C.class"] + listAllFiles(annotationGeneratedSourcesDir) == ["com/example/A.java", "com/example/B.java", "com/example/B.txt"] } - def "#stagingDir directory is cleaned before the execution and folder structure is the same as in #originalDir"() { - createNewDirectory(file("$originalDir/dir1/dir1")) - createNewDirectory(file("$originalDir/dir2")) - // This is a file, so folder ./dir1/dir2 in transactional dir should be deleted - createNewFile(file("$originalDir/dir1/dir2")) - createNewFile(fileInTransactionDir("$stagingDir/dir1/dir1/file1.txt")) // only file should be deleted - createNewDirectory(fileInTransactionDir("$stagingDir/dir1/dir2")) // dir2 directory should be deleted - createNewDirectory(fileInTransactionDir("$stagingDir/dir3")) // dir3 directory should be deleted - createNewFile(fileInTransactionDir("$stagingDir/dir2/file1.txt")) // only file should be deleted - createNewDirectory(fileInTransactionDir("some-other-dir")) // some-other-dir directory should be deleted - spec.setDestinationDir(file("classes")) - spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(file("annotation-generated-sources")) - spec.getCompileOptions().setHeaderOutputDirectory(file("header-sources")) + def "generated classes and resources are not deleted and classes in backup dir are not restored if incremental compilation after failure is not supported"() { + def destinationDir = spec.getDestinationDir() + def annotationGeneratedSourcesDir = createNewDirectory(file("generated/annotation-resources")) + def overwrittenFile = createNewFile(new File(destinationDir, "Overwritten.class")) + spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(annotationGeneratedSourcesDir) + spec.getCompileOptions().setSupportsIncrementalCompilationAfterFailure(false) - expect: + when: newCompileTransaction().execute { - assert transactionDir.list() as Set ==~ ["stash-dir", "compile-output", "annotation-output", "header-output"] - assert fileInTransactionDir(stagingDir).list() as Set ==~ ["dir1", "dir2"] - assert fileInTransactionDir("$stagingDir/dir1").list() as Set ==~ ["dir1"] - assert hasOnlyDirectories(fileInTransactionDir(stagingDir)) - return DID_WORK + def compilerResult = simulateGeneratingClassesAndResources(destinationDir, annotationGeneratedSourcesDir, overwrittenFile) + throw new CompilationFailedException(compilerResult) } - where: - originalDir | stagingDir - "classes" | "compile-output" - "annotation-generated-sources" | "annotation-output" - "header-sources" | "header-output" + then: + thrown(CompilationFailedException) + overwrittenFile.text == "Overwritten" + listAllFiles(destinationDir) == ["Overwritten.class", "com/example/A.class", "com/example/A.txt", "com/example/B.class", "com/example/C\$D.class", "com/example/C.class"] + listAllFiles(annotationGeneratedSourcesDir) == ["com/example/A.java", "com/example/B.java", "com/example/B.txt"] } - def "#description modify spec outputs and restores them when incremental compilation after failure is set to #incrementalCompilationAfterFilure"() { - spec.setDestinationDir(file("classes")) - spec.getCompileOptions().setAnnotationProcessorGeneratedSourcesDirectory(file("annotation-generated-sources")) - spec.getCompileOptions().setHeaderOutputDirectory(file("header-sources")) - spec.getCompileOptions().setSupportsIncrementalCompilationAfterFailure(incrementalCompilationAfterFilure) + ApiCompilerResult simulateGeneratingClassesAndResources(File destinationDir, File annotationGeneratedSourcesDir, File overwrittenFile) { + def compilerResult = new ApiCompilerResult() + compilerResult.annotationProcessingResult.generatedAggregatingTypes.addAll(["com.example.A"]) + compilerResult.annotationProcessingResult.generatedAggregatingResources.addAll(new GeneratedResource(CLASS_OUTPUT, "com.example", "A.txt")) + compilerResult.annotationProcessingResult.addGeneratedType("com.example.B", ["ElementA"] as Set) + compilerResult.annotationProcessingResult.addGeneratedResource(new GeneratedResource(SOURCE_OUTPUT, "com.example", "B.txt"), ["ElementB"] as Set) + compilerResult.sourceClassesMapping.put("com/example/C.java", ["com.example.C", "com.example.C\$D"] as Set) + def fileInBackupDir = new File(classBackupDir, "Overwritten.class") + Files.copy(overwrittenFile.toPath(), fileInBackupDir.toPath()) + overwrittenFile.text = "Overwritten" + compilerResult.backupClassFiles.put(overwrittenFile.absolutePath, fileInBackupDir.absolutePath) + + createNewFile(new File(destinationDir, "com/example/A.class")) + createNewFile(new File(annotationGeneratedSourcesDir, "com/example/A.java")) + createNewFile(new File(destinationDir, "com/example/B.class")) + createNewFile(new File(annotationGeneratedSourcesDir, "com/example/B.java")) + createNewFile(new File(destinationDir, "com/example/A.txt")) + createNewFile(new File(annotationGeneratedSourcesDir, "com/example/B.txt")) + createNewFile(new File(destinationDir, "com/example/C.class")) + createNewFile(new File(destinationDir, "com/example/C\$D.class")) + return compilerResult + } - expect: - // Check if output folders were modified - newCompileTransaction().execute { - def fileGenerator = incrementalCompilationAfterFilure - ? { String fileName -> fileInTransactionDir(fileName) } as Function - : { String fileName -> file(fileName) } as Function - assert spec.getDestinationDir() == fileGenerator.apply(destinationDir) - assert spec.getCompileOptions().getAnnotationProcessorGeneratedSourcesDirectory() == fileGenerator.apply(annotationProcessorDir) - assert spec.getCompileOptions().getHeaderOutputDirectory() == fileGenerator.apply(headerDir) - return DID_WORK + def "class backup directory is #description when compile after failure is set to #compileAfterFailure"() { + given: + spec.setDestinationDir(createNewFile(file("compile"))) + spec.getCompileOptions().setSupportsIncrementalCompilationAfterFailure(compileAfterFailure) + + when: + def classBackupDir = newCompileTransaction().execute { + return spec.getClassBackupDir() } - // And restore output folders after - spec.getDestinationDir() == file("classes") - spec.getCompileOptions().getAnnotationProcessorGeneratedSourcesDirectory() == file("annotation-generated-sources") - spec.getCompileOptions().getHeaderOutputDirectory() == file("header-sources") + then: + classBackupDir == (expectedClassBackupDir as Supplier).get() where: - description | incrementalCompilationAfterFilure | destinationDir | annotationProcessorDir | headerDir - "doesn't" | false | "classes" | "annotation-generated-sources" | "header-sources" - "does" | true | "compile-output" | "annotation-output" | "header-output" + description | compileAfterFailure | expectedClassBackupDir + "set" | true | { classBackupDir } + "not set" | false | { null } } private File fileInTransactionDir(String path) { @@ -387,6 +389,12 @@ class CompileTransactionTest extends Specification { file.listFiles().length == 0 } + private List listAllFiles(File file) { + return Files.find(file.toPath(), 99, { path, attributes -> attributes.isRegularFile() }) + .collect { file.toPath().relativize(it).toString().replace("\\", "/") } + .toSorted() + } + private boolean hasOnlyDirectories(File file) { try (Stream stream = Files.walk(file.toPath())) { return stream.allMatch { Files.isDirectory(it) } diff --git a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJvmLanguageCompileSpec.java b/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJvmLanguageCompileSpec.java index 3828fe5c35c9..17c9565256be 100644 --- a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJvmLanguageCompileSpec.java +++ b/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJvmLanguageCompileSpec.java @@ -28,7 +28,6 @@ public class DefaultJvmLanguageCompileSpec implements JvmLanguageCompileSpec, Se private File tempDir; private List classpath; private File destinationDir; - private File originalDestinationDir; private Iterable sourceFiles; private Integer release; private String sourceCompatibility; @@ -55,16 +54,6 @@ public void setDestinationDir(File destinationDir) { this.destinationDir = destinationDir; } - @Override - public File getOriginalDestinationDir() { - return originalDestinationDir; - } - - @Override - public void setOriginalDestinationDir(File originalDestinationDir) { - this.originalDestinationDir = originalDestinationDir; - } - @Override public File getTempDir() { return tempDir; diff --git a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/JvmLanguageCompileSpec.java b/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/JvmLanguageCompileSpec.java index 05a24bdc9d71..c000e4bbbde2 100644 --- a/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/JvmLanguageCompileSpec.java +++ b/subprojects/language-jvm/src/main/java/org/gradle/api/internal/tasks/compile/JvmLanguageCompileSpec.java @@ -35,10 +35,6 @@ public interface JvmLanguageCompileSpec extends CompileSpec { void setDestinationDir(File destinationDir); - File getOriginalDestinationDir(); - - void setOriginalDestinationDir(File originalDestinationDir); - Iterable getSourceFiles(); void setSourceFiles(Iterable sourceFiles); diff --git a/subprojects/scala/src/main/java/org/gradle/language/scala/tasks/AbstractScalaCompile.java b/subprojects/scala/src/main/java/org/gradle/language/scala/tasks/AbstractScalaCompile.java index d21cdeeacb2a..10ef563978ab 100644 --- a/subprojects/scala/src/main/java/org/gradle/language/scala/tasks/AbstractScalaCompile.java +++ b/subprojects/scala/src/main/java/org/gradle/language/scala/tasks/AbstractScalaCompile.java @@ -175,7 +175,6 @@ protected ScalaJavaJointCompileSpec createSpec() { DefaultScalaJavaJointCompileSpec spec = new DefaultScalaJavaJointCompileSpecFactory(compileOptions, getToolchain()).create(); spec.setSourceFiles(getSource().getFiles()); spec.setDestinationDir(getDestinationDirectory().getAsFile().get()); - spec.setOriginalDestinationDir(spec.getDestinationDir()); spec.setWorkingDir(getProjectLayout().getProjectDirectory().getAsFile()); spec.setTempDir(getTemporaryDir()); spec.setCompileClasspath(ImmutableList.copyOf(getClasspath())); From 569467b3b4e7e4af1d35fffb29b00bb31866a327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C5=BEe=20Sodja?= Date: Thu, 26 Jan 2023 11:17:01 +0100 Subject: [PATCH 3/4] [Backport] Remove mentioning temporary compile outputs Backport of #23661 --- .../src/docs/userguide/jvm/java_plugin.adoc | 20 ------------------- .../migration/upgrading_version_7.adoc | 7 ------- 2 files changed, 27 deletions(-) diff --git a/subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc b/subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc index 760b5854e883..4583af9ddcdd 100644 --- a/subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc +++ b/subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc @@ -509,26 +509,6 @@ 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 <> 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//compileTransaction/compile-output` -| `options.generatedSourceOutputDirectory` | `build/generated/sources/annotationProcessor/java/main` | `build/tmp//compileTransaction/annotation-output` -| `options.headerOutputDirectory` | `build/generated/sources/headers/java/main` | `build/tmp//compileTransaction/header-output` -|=== - [[sec:incremental_annotation_processing]] == Incremental annotation processing diff --git a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc index 575071c6234e..ed63c77c6ebe 100644 --- a/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc +++ b/subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc @@ -173,13 +173,6 @@ Use `implementation.bundle(libs.bundles.testing)` instead. For more information, see the updated <> 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 <> for more details. - === Deprecations [[invalid_toolchain_specification_deprecation]] From 10caff4f21bdd7d55fa7b46ee30d2a68700cf186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C5=BEe=20Sodja?= Date: Fri, 26 May 2023 10:49:48 +0200 Subject: [PATCH 4/4] [Backport] Do full recompilation on fatal failure for Groovy Backport of #24880 --- .../tasks/compile/ApiGroovyCompiler.java | 29 ++++++++++++++ ...GroovyIncrementalCompilationSupport.groovy | 2 +- ...pilationAfterFailureIntegrationTest.groovy | 38 +++++++++++++++++++ .../compile/CompilationFatalException.java | 27 +++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFatalException.java diff --git a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java index 52c837d7ea05..26bba16a9956 100644 --- a/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java +++ b/subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java @@ -29,9 +29,11 @@ import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilePhase; import org.codehaus.groovy.control.CompilerConfiguration; +import org.codehaus.groovy.control.MultipleCompilationErrorsException; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.CompilationCustomizer; import org.codehaus.groovy.control.customizers.ImportCustomizer; +import org.codehaus.groovy.control.messages.ExceptionMessage; import org.codehaus.groovy.control.messages.SimpleMessage; import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit; import org.codehaus.groovy.tools.javac.JavaCompiler; @@ -283,6 +285,12 @@ private void copyJavaCompilerResult(ApiCompilerResult javaCompilerResult) { unit.compile(); return result; } catch (org.codehaus.groovy.control.CompilationFailedException e) { + if (isFatalException(e)) { + // This indicates a compiler bug and not a user error, + // so we cannot recover from such error: we need to force full recompilation. + throw new CompilationFatalException(e); + } + System.err.println(e.getMessage()); // Explicit flush, System.err is an auto-flushing PrintWriter unless it is replaced. System.err.flush(); @@ -297,6 +305,27 @@ private void copyJavaCompilerResult(ApiCompilerResult javaCompilerResult) { } } + /** + * Returns true if the exception is fatal, unrecoverable for the incremental compilation. Example of such error: + * 
+     * error: startup failed:
+     * General error during instruction selection: java.lang.NoClassDefFoundError: Unable to load class ClassName due to missing dependency DependencyName
+     *   java.lang.RuntimeException: java.lang.NoClassDefFoundError: Unable to load class ClassName due to missing dependency DependencyName
+     *      at org.codehaus.groovy.control.CompilationUnit$IPrimaryClassNodeOperation.doPhaseOperation(CompilationUnit.java:977)
+     *      at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:672)
+     *      at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:636)
+     *      at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:611)
+     *
+ */ + private static boolean isFatalException(org.codehaus.groovy.control.CompilationFailedException e) { + if (e instanceof MultipleCompilationErrorsException) { + // Groovy compiler wraps any uncontrolled exception (e.g. IOException, NoClassDefFoundError and similar) in a `ExceptionMessage` + return ((MultipleCompilationErrorsException) e).getErrorCollector().getErrors().stream() + .anyMatch(message -> message instanceof ExceptionMessage); + } + return false; + } + private static boolean shouldProcessAnnotations(GroovyJavaJointCompileSpec spec) { return spec.getGroovyCompileOptions().isJavaAnnotationProcessing() && spec.annotationProcessingConfigured(); } diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy index fd3cad79cf67..0c27ab20577d 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractJavaGroovyIncrementalCompilationSupport.groovy @@ -53,7 +53,7 @@ abstract class AbstractJavaGroovyIncrementalCompilationSupport extends AbstractI def packageGroup = (body =~ "\\s*package ([\\w.]+).*") String packageName = packageGroup.size() > 0 ? packageGroup[0][1] : "" String packageFolder = packageName.replaceAll("[.]", "/") - def className = (body =~ /(?s).*?(?:class|interface|enum) ([\w$]+) .*/)[0][1] + def className = (body =~ /(?s).*?(?:class|interface|enum|trait) ([\w$]+) .*/)[0][1] assert className: "unable to find class name" def f if (packageFolder.isEmpty()) { diff --git a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy index 453cb805b4fa..43c563e322dd 100644 --- a/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy +++ b/subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/BaseIncrementalCompilationAfterFailureIntegrationTest.groovy @@ -543,4 +543,42 @@ class GroovyIncrementalCompilationAfterFailureIntegrationTest extends BaseIncrem "Java" | "java" | "" "Groovy" | "groovy" | COMPILE_STATIC_ANNOTATION } + + @Issue("https://github.com/gradle/gradle/issues/22814") + def 'does full recompilation on fatal failure'() { + given: + def a = source("class A extends ABase implements WithTrait { def m() { println('a') } }") + source "class ABase { def mBase() { println(A) } }" + source """ + import groovy.transform.SelfType + @SelfType(ABase) + trait WithTrait { + final AllPluginsValidation allPlugins = new AllPluginsValidation(this) + + static class AllPluginsValidation { + final ABase base + AllPluginsValidation(ABase base) { + this.base = base + } + } + } + """ + run "compileGroovy" + outputs.recompiledClasses('ABase', 'A', 'WithTrait', 'WithTrait$Trait$FieldHelper', 'WithTrait$AllPluginsValidation', 'WithTrait$Trait$Helper') + + when: + a.text = "class A extends ABase implements WithTrait { def m() { println('b') } }" + + then: + executer.withStackTraceChecksDisabled() + def execution = runAndFail "compileGroovy" + execution.assertHasCause("Unrecoverable compilation error") + + when: + run"compileGroovy", "--info" + + then: + outputs.recompiledClasses('ABase', 'A', 'WithTrait', 'WithTrait$Trait$FieldHelper', 'WithTrait$AllPluginsValidation', 'WithTrait$Trait$Helper') + outputContains("Full recompilation is required") + } } diff --git a/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFatalException.java b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFatalException.java new file mode 100644 index 000000000000..0476eeda4223 --- /dev/null +++ b/subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/CompilationFatalException.java @@ -0,0 +1,27 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle.api.internal.tasks.compile; + +/** + * Indicates a fatal error during compilation. Gradle will not try to recover output files from a previous compilation. + */ +public class CompilationFatalException extends RuntimeException { + + public CompilationFatalException(Throwable cause) { + super("Unrecoverable compilation error: " + cause.getMessage(), cause); + } +}