Revert part of modules fix from #23119

Fix for modules in GradleStandardJavaFileManager is not needed anymore, since we don't change output directories
gradle · Jan 9, 2023 · 479c3d1 · 479c3d1
1 parent 12e2655
commit 479c3d1
Show file tree

Hide file tree

Showing 12 changed files with 40 additions and 103 deletions.
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
@@ -232,7 +232,6 @@ private GroovyJavaJointCompileSpec createSpec() {
         spec.setSourcesRoots(sourceRoots);
         spec.setSourceFiles(stableSourcesAsFileTree);
         spec.setDestinationDir(getDestinationDirectory().getAsFile().get());
-        spec.setOriginalDestinationDir(spec.getDestinationDir());
         spec.setJavaClassCompileOrder(JavaClassCompileOrder.fromInternalSystemProperty());
         spec.setWorkingDir(getProjectLayout().getProjectDirectory().getAsFile());
         spec.setTempDir(getTemporaryDir());

diff --git a/...gradle/java/compile/incremental/CrossTaskIncrementalJavaCompilationIntegrationTest.groovy b/...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/...uage-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java b/...uage-java/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJavaCompileSpec.java
@@ -36,7 +36,6 @@ public class DefaultJavaCompileSpec extends DefaultJvmLanguageCompileSpec implem
     private Set<String> classes;
     private List<File> modulePath;
     private List<File> sourceRoots;
-    private boolean isIncrementalCompilationOfJavaModule;
     private Set<String> undeletedClasses = Collections.emptySet();
     private File backupDestinationDir;
     private JavaClassCompileOrder javacCompilerOrder = JavaClassCompileOrder.UNORDERED;
@@ -142,16 +141,6 @@ public void setModulePath(List<File> modulePath) {
         this.modulePath = modulePath;
     }
 
-    @Override
-    public boolean isIncrementalCompilationOfJavaModule() {
-        return isIncrementalCompilationOfJavaModule;
-    }
-
-    @Override
-    public void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule) {
-        this.isIncrementalCompilationOfJavaModule = isIncrementalCompilationOfJavaModule;
-    }
-
     @Override
     public List<File> getSourceRoots() {
         return sourceRoots;

diff --git a/...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java b/...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JavaCompileSpec.java
@@ -71,10 +71,6 @@ public static JavaClassCompileOrder fromInternalSystemProperty() {
 
     void setModulePath(List<File> modulePath);
 
-    boolean isIncrementalCompilationOfJavaModule();
-
-    void setIsIncrementalCompilationOfJavaModule(boolean isIncrementalCompilationOfJavaModule);
-
     default boolean annotationProcessingConfigured() {
         return !getAnnotationProcessorPath().isEmpty() && !getCompileOptions().getCompilerArgs().contains("-proc:none");
     }

diff --git a/...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java b/...ts/language-java/src/main/java/org/gradle/api/internal/tasks/compile/JdkJavaCompiler.java
@@ -25,7 +25,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.tools.JavaCompiler;
 import javax.tools.JavaFileManager;
@@ -45,7 +44,6 @@
 
 public class JdkJavaCompiler implements Compiler<JavaCompileSpec>, Serializable {
     private static final Logger LOGGER = LoggerFactory.getLogger(JdkJavaCompiler.class);
-    private static final String JAVAC_CLASS_COMPILE_ORDER = "org.gradle.internal.javac.class.compile.order";
 
     private final Factory<JavaCompiler> javaHomeBasedJavaCompilerFactory;
 
@@ -78,8 +76,7 @@ private JavaCompiler.CompilationTask createCompileTask(JavaCompileSpec spec, Api
             : spec.getSourceFiles();
         Iterable<? extends JavaFileObject> compilationUnits = standardFileManager.getJavaFileObjectsFromFiles(sourceFiles);
         boolean hasEmptySourcepaths = JavaVersion.current().isJava9Compatible() && emptySourcepathIn(options);
-        File previousClassOutput = maybeGetPreviousClassOutput(spec);
-        JavaFileManager fileManager = GradleStandardJavaFileManager.wrap(standardFileManager, DefaultClassPath.of(spec.getAnnotationProcessorPath()), hasEmptySourcepaths, previousClassOutput);
+        JavaFileManager fileManager = GradleStandardJavaFileManager.wrap(standardFileManager, DefaultClassPath.of(spec.getAnnotationProcessorPath()), hasEmptySourcepaths);
         JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, spec.getClasses(), compilationUnits);
         if (compiler instanceof IncrementalCompilationAwareJavaCompiler) {
             task = ((IncrementalCompilationAwareJavaCompiler) compiler).makeIncremental(
@@ -106,12 +103,4 @@ private static boolean emptySourcepathIn(List<String> options) {
         }
         return false;
     }
-
-    @Nullable
-    private static File maybeGetPreviousClassOutput(JavaCompileSpec spec) {
-        if (JavaVersion.current().isJava9Compatible() && spec.isIncrementalCompilationOfJavaModule() && !spec.getDestinationDir().equals(spec.getOriginalDestinationDir())) {
-            return spec.getOriginalDestinationDir();
-        }
-        return null;
-    }
 }
diff --git a/...adle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java b/...adle/api/internal/tasks/compile/incremental/recomp/AbstractRecompilationSpecProvider.java
@@ -207,10 +207,7 @@ private static void addModuleInfoToCompile(RecompilationSpec spec, SourceFileCla
         Set<String> moduleInfoSources = sourceFileClassNameConverter.getRelativeSourcePaths(MODULE_INFO_CLASS_NAME);
         if (!moduleInfoSources.isEmpty()) {
             // Always recompile module-info.java if present.
-            // This solves case for incremental compilation for manual --module-path when not combined with --module-source-path or --source-path,
-            // since compiled module-info is not in the output after we change compile outputs in the CompileTransaction.
-            // Alternative would be, that we would move/copy the module-info class to transaction outputs and add transaction outputs to classpath.
-            // First part of fix for: https://github.com/gradle/gradle/issues/23067
+            // 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);
         }
@@ -233,7 +230,6 @@ public CompileTransaction initCompilationSpecAndTransaction(JavaCompileSpec spec
         addClassesToProcess(spec, recompilationSpec);
         addAllUndeletedClasses(spec, recompilationSpec);
         Map<GeneratedResource.Location, PatternSet> resourcesToDelete = prepareResourcePatterns(recompilationSpec.getResourcesToGenerate(), fileOperations);
-        spec.setIsIncrementalCompilationOfJavaModule(recompilationSpec.hasClassToCompile(MODULE_INFO_CLASS_NAME));
         return new CompileTransaction(spec, classesToDelete, resourcesToDelete, fileOperations, deleter);
     }
 

diff --git a/...main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java b/...main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/RecompilationSpec.java
@@ -56,10 +56,6 @@ public Set<String> 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/...ain/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java b/...ain/java/org/gradle/api/internal/tasks/compile/reflect/GradleStandardJavaFileManager.java
@@ -16,53 +16,35 @@
 
 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;
 
 public class GradleStandardJavaFileManager extends ForwardingJavaFileManager<StandardJavaFileManager> {
     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<JavaFileObject> 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<JavaFileObject> previousClassOutput = super.list(GradleLocation.PREVIOUS_CLASS_OUTPUT, packageName, kinds, recurse);
-            Iterable<JavaFileObject> 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
@@ -235,7 +235,6 @@ DefaultJavaCompileSpec createSpec() {
         DefaultJavaCompileSpec spec = new DefaultJavaCompileSpecFactory(compileOptions, getToolchain()).create();
 
         spec.setDestinationDir(getDestinationDirectory().getAsFile().get());
-        spec.setOriginalDestinationDir(spec.getDestinationDir());
         spec.setWorkingDir(getProjectLayout().getProjectDirectory().getAsFile());
         spec.setJavaClassCompileOrder(JavaClassCompileOrder.fromInternalSystemProperty());
         spec.setTempDir(getTemporaryDir());

diff --git a/...vm/src/main/java/org/gradle/api/internal/tasks/compile/DefaultJvmLanguageCompileSpec.java b/...vm/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<File> classpath;
     private File destinationDir;
-    private File originalDestinationDir;
     private Iterable<File> 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/...guage-jvm/src/main/java/org/gradle/api/internal/tasks/compile/JvmLanguageCompileSpec.java b/...guage-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<File> getSourceFiles();
 
     void setSourceFiles(Iterable<File> 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
@@ -159,7 +159,6 @@ protected ScalaJavaJointCompileSpec createSpec() {
         DefaultScalaJavaJointCompileSpec spec = new DefaultScalaJavaJointCompileSpec(javaExecutable);
         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()));