From 368304f8194892e412b45692d2094b69b014bf44 Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Thu, 12 Jan 2017 13:23:45 +0100 Subject: [PATCH] move class handling changed: classes that directly depend on removed sources are now compiled in the first compilation round (IDEA-165813) --- ...DependenciesOnMovedClassesInFirstRound.log | 12 ++++++ .../src/ppp/A.java | 14 ++++++ .../src/ppp/A.java.new | 15 +++++++ .../src/ppp/B.java | 8 ++++ .../src/ppp/C.java.new | 5 +++ .../src/qqq/C.java | 5 +++ .../src/qqq/C.java.remove | 0 .../src/qqq/Placeholder.java | 4 ++ .../common/moveClassToAnotherRoot.log | 6 +-- .../markDirty/recompileTwinDependencies.log | 6 ++- .../src/package2/C.java.new | 5 ++- .../jps/builders/java/JavaBuilderUtil.java | 43 ++++++++++++++----- .../jps/incremental/java/JavaBuilder.java | 21 ++++++++- .../multipleSources/deleteFile.log | 1 - .../org/jetbrains/ether/CommonTest.java | 4 ++ .../org/jetbrains/ether/MarkDirtyTest.java | 2 +- 16 files changed, 129 insertions(+), 22 deletions(-) create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound.log create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/B.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/C.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java.remove create mode 100644 java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/Placeholder.java diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound.log b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound.log new file mode 100644 index 000000000000..8ad18bab091b --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound.log @@ -0,0 +1,12 @@ +Cleaning output files: +out/production/CompileDependenciesOnMovedClassesInFirstRound/qqq/C.class +End of files +Cleaning output files: +out/production/CompileDependenciesOnMovedClassesInFirstRound/ppp/A.class +out/production/CompileDependenciesOnMovedClassesInFirstRound/ppp/B.class +End of files +Compiling files: +src/ppp/A.java +src/ppp/B.java +src/ppp/C.java +End of files \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java new file mode 100644 index 000000000000..b5eef2dda093 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java @@ -0,0 +1,14 @@ +package ppp; +import qqq.*; + +public class A { + private C delegate; + + public A(C delegate) { + this.delegate = delegate; + } + + void foo() { + B.util(delegate); + } +} diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java.new b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java.new new file mode 100644 index 000000000000..f2922e3a90b9 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/A.java.new @@ -0,0 +1,15 @@ +package ppp; +import qqq.*; + +public class A { + + private C delegate; + + public A(C delegate) { + this.delegate = delegate; + } + + void foo() { + B.util(delegate); + } +} diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/B.java b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/B.java new file mode 100644 index 000000000000..3e80262937d8 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/B.java @@ -0,0 +1,8 @@ +package ppp; +import qqq.*; + +public class B { + public static void util(C iface) { + iface.execute(); + } +} diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/C.java.new b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/C.java.new new file mode 100644 index 000000000000..133baf358081 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/ppp/C.java.new @@ -0,0 +1,5 @@ +package ppp; + +public interface C { + void execute(); +} diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java new file mode 100644 index 000000000000..8c20b39907ed --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java @@ -0,0 +1,5 @@ +package qqq; + +public interface C { + void execute(); +} diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java.remove b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/C.java.remove new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/Placeholder.java b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/Placeholder.java new file mode 100644 index 000000000000..2570d12d2c19 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/compileDependenciesOnMovedClassesInFirstRound/src/qqq/Placeholder.java @@ -0,0 +1,4 @@ +package qqq; + +public class Placeholder{ +} \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToAnotherRoot.log b/java/java-tests/testData/compileServer/incremental/common/moveClassToAnotherRoot.log index f16383a14224..d6b53f204f38 100644 --- a/java/java-tests/testData/compileServer/incremental/common/moveClassToAnotherRoot.log +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassToAnotherRoot.log @@ -1,12 +1,10 @@ Cleaning output files: out/production/moveClassToAnotherRoot/pkg/A.class End of files -Compiling files: -src2/pkg/A.java -End of files Cleaning output files: out/production/moveClassToAnotherRoot/pkg/Client.class End of files Compiling files: src/pkg/Client.java -End of files +src2/pkg/A.java +End of files \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log index 4badc88cf13a..c2f6c8fe2507 100644 --- a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log @@ -4,7 +4,11 @@ End of files Cleaning output files: out/production/RecompileTwinDependencies/package1/C.class End of files +Cleaning output files: +out/production/RecompileTwinDependencies/com/B.class +End of files Compiling files: +src/com/B.java src/package2/A.java src/package2/C.java -End of files +End of files \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new index 6badaaa8c109..01c486b5f314 100644 --- a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new @@ -5,7 +5,8 @@ import com.B; public class C { { new B().get(); // should be resolved to "com/B.get:()Lpackage2/A;" - // but package2/C.java is first compiled when B.class still contains "public package1.A get();" - // and compiler returns an error + // If B.class were not compiled in the very first compilation round, it would still contain "public package1.A get();" and compiler would return an error + // Because of special logic, all classes that depend on moved or deleted classes, are marked dirty before compilation starts, + // so they are compiled in the first round and no errors should occur. } } diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/JavaBuilderUtil.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/JavaBuilderUtil.java index 6382ce3c253d..cee7e9b10e68 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/JavaBuilderUtil.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/JavaBuilderUtil.java @@ -127,7 +127,7 @@ public class JavaBuilderUtil { SUCCESSFULLY_COMPILED_FILES_KEY.set(context, null); FileFilter filter = createOrFilter(SKIP_MARKING_DIRTY_FILTERS_KEY.get(context)); SKIP_MARKING_DIRTY_FILTERS_KEY.set(context, null); - return updateMappings(context, delta, dirtyFilesHolder, chunk, compiledFiles, successfullyCompiled, filter); + return updateMappings(context, delta, dirtyFilesHolder, chunk, compiledFiles, successfullyCompiled, CompilationRound.NEXT, filter); } /** @@ -142,21 +142,31 @@ public class JavaBuilderUtil { ModuleChunk chunk, Collection filesToCompile, Collection successfullyCompiled) throws IOException { - return updateMappings(context, delta, dirtyFilesHolder, chunk, filesToCompile, successfullyCompiled, null); + return updateMappings(context, delta, dirtyFilesHolder, chunk, filesToCompile, successfullyCompiled, CompilationRound.NEXT, null); + } + + public static void markDirtyDependenciesForInitialRound(CompileContext context, DirtyFilesHolder dfh, ModuleChunk chunk) throws IOException { + if (hasRemovedPaths(chunk, dfh)) { + final Mappings delta = context.getProjectDescriptor().dataManager.getMappings().createDelta(); + final Set empty = Collections.emptySet(); + updateMappings(context, delta, dfh, chunk, empty, empty, CompilationRound.CURRENT, null); + } } /** - * @param filesToCompile files compiled in this round + * @param filesToCompile files compiled in this round + * @param markDirtyRound compilation round at which dirty files should be visible to builders * @return true if additional compilation pass is required, false otherwise * @throws Exception */ private static boolean updateMappings(CompileContext context, - final Mappings delta, - DirtyFilesHolder dirtyFilesHolder, - ModuleChunk chunk, - Collection filesToCompile, - Collection successfullyCompiled, - @Nullable FileFilter skipMarkingDirtyFilter) throws IOException { + final Mappings delta, + DirtyFilesHolder dirtyFilesHolder, + ModuleChunk chunk, + Collection filesToCompile, + Collection successfullyCompiled, + final CompilationRound markDirtyRound, + @Nullable FileFilter skipMarkingDirtyFilter) throws IOException { try { boolean performIntegrate = true; boolean additionalPassRequired = false; @@ -232,7 +242,7 @@ public class JavaBuilderUtil { } for (File file : newlyAffectedFiles) { - FSOperations.markDirtyIfNotDeleted(context, CompilationRound.NEXT, file); + FSOperations.markDirtyIfNotDeleted(context, markDirtyRound, file); } additionalPassRequired = isCompileJavaIncrementally(context) && chunkContainsAffectedFiles(context, chunk, newlyAffectedFiles); } @@ -255,7 +265,7 @@ public class JavaBuilderUtil { } FileFilter toBeMarkedFilter = skipMarkingDirtyFilter == null ? null : new NegationFileFilter(skipMarkingDirtyFilter); - FSOperations.markDirtyRecursively(context, CompilationRound.NEXT, chunk, toBeMarkedFilter); + FSOperations.markDirtyRecursively(context, markDirtyRound, chunk, toBeMarkedFilter); } } else { @@ -384,6 +394,17 @@ public class JavaBuilderUtil { return removed; } + private static boolean hasRemovedPaths(ModuleChunk chunk, DirtyFilesHolder dirtyFilesHolder) { + if (dirtyFilesHolder.hasRemovedFiles()) { + for (ModuleBuildTarget target : chunk.getTargets()) { + if (!dirtyFilesHolder.getRemovedFiles(target).isEmpty()) { + return true; + } + } + } + return false; + } + public static void cleanupChunkResources(CompileContext context) { ALL_AFFECTED_FILES_KEY.set(context, null); ALL_COMPILED_FILES_KEY.set(context, null); diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/java/JavaBuilder.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/java/JavaBuilder.java index 5c1eb83170ee..bba9482d2fb1 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/java/JavaBuilder.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/java/JavaBuilder.java @@ -37,6 +37,7 @@ import org.jetbrains.jps.api.GlobalOptions; import org.jetbrains.jps.builders.BuildRootIndex; import org.jetbrains.jps.builders.DirtyFilesHolder; import org.jetbrains.jps.builders.FileProcessor; +import org.jetbrains.jps.builders.impl.DirtyFilesHolderBase; import org.jetbrains.jps.builders.java.JavaBuilderExtension; import org.jetbrains.jps.builders.java.JavaBuilderUtil; import org.jetbrains.jps.builders.java.JavaCompilingTool; @@ -64,8 +65,7 @@ import org.jetbrains.jps.model.serialization.PathMacroUtil; import org.jetbrains.jps.service.JpsServiceManager; import org.jetbrains.jps.service.SharedThreadPool; -import javax.tools.Diagnostic; -import javax.tools.JavaFileObject; +import javax.tools.*; import java.io.*; import java.net.ServerSocket; import java.util.*; @@ -157,6 +157,23 @@ public class JavaBuilder extends ModuleLevelBuilder { COMPILER_USAGE_STATISTICS.set(context, new ConcurrentHashMap>()); } + @Override + public void chunkBuildStarted(final CompileContext context, final ModuleChunk chunk) { + // before the first compilation round starts: find and mark dirty all classes that depend on removed or moved classes so + // that all such files are compiled in the first round. + try { + JavaBuilderUtil.markDirtyDependenciesForInitialRound(context, new DirtyFilesHolderBase(context) { + @Override + public void processDirtyFiles(@NotNull FileProcessor processor) throws IOException { + FSOperations.processFilesToRecompile(context, chunk, processor); + } + }, chunk); + } + catch (IOException e) { + throw new RuntimeException(e); + } + } + public void buildFinished(CompileContext context) { final ConcurrentMap> stats = COMPILER_USAGE_STATISTICS.get(context); if (stats.size() == 1) { diff --git a/jps/jps-builders/testData/incremental/multipleSources/deleteFile.log b/jps/jps-builders/testData/incremental/multipleSources/deleteFile.log index 893d2fc1d06f..cfe5b50ba312 100644 --- a/jps/jps-builders/testData/incremental/multipleSources/deleteFile.log +++ b/jps/jps-builders/testData/incremental/multipleSources/deleteFile.log @@ -3,7 +3,6 @@ out/production/m/PackageFacade.class out/production/m/b.class End of files Cleaning output files: -out/production/m/PackageFacade.class out/production/m/a.class End of files Compiling files: diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java index 4c1d1fffed57..cdfd88131fe0 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java @@ -156,6 +156,10 @@ public class CommonTest extends IncrementalTestCase { doTestBuild(1).assertSuccessful(); } + public void testCompileDependenciesOnMovedClassesInFirstRound() throws Exception { + doTest().assertSuccessful(); + } + public void testIntegrateOnSuperclassRemovedAndRestored() throws Exception { setupInitialProject(); diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java index 9687a4456955..a880e2e8baff 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java @@ -54,7 +54,7 @@ public class MarkDirtyTest extends IncrementalTestCase { } public void testRecompileTwinDependencies() { - doTest().assertFailed(); + doTest().assertSuccessful(); } public void testDoNotMarkDirtyCompiledChunks() {