From 7cd43429a7cf4d794e6b0ff62bb5710d782b3572 Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Fri, 14 Jun 2013 17:58:17 +0400 Subject: [PATCH] incremental compilation: ensure javac always resolves dependencies within same module against sources and not classes compiled on previous steps (IDEA-108215) --- .../markDirty/recompileTwinDependencies.log | 10 ++++++++ .../recompileTwinDependencies/src/com/B.java | 10 ++++++++ .../src/package1/A.java | 4 +++ .../src/package1/A.java.remove | 0 .../src/package1/C.java | 9 +++++++ .../src/package1/C.java.remove | 0 .../src/package1/Dummy.java | 5 ++++ .../src/package2/A.java.new | 4 +++ .../src/package2/C.java.new | 11 ++++++++ .../src/package2/Dummy.java | 5 ++++ .../jps/incremental/IncProjectBuilder.java | 16 +++++++++++- .../jps/incremental/fs/BuildFSState.java | 25 +++++++++++++------ .../jps/incremental/java/JavaBuilder.java | 16 +++++++++--- .../org/jetbrains/jps/javac/JavacMain.java | 3 ++- .../org/jetbrains/ether/MarkDirtyTest.java | 4 +++ 15 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/com/B.java create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java.remove create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java.remove create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/Dummy.java create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/A.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/Dummy.java diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log new file mode 100644 index 000000000000..4badc88cf13a --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies.log @@ -0,0 +1,10 @@ +Cleaning output files: +out/production/RecompileTwinDependencies/package1/A.class +End of files +Cleaning output files: +out/production/RecompileTwinDependencies/package1/C.class +End of files +Compiling files: +src/package2/A.java +src/package2/C.java +End of files diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/com/B.java b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/com/B.java new file mode 100644 index 000000000000..f3ead618ddb7 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/com/B.java @@ -0,0 +1,10 @@ +package com; + +import package1.*; +import package2.*; + +public class B { + public A get() { // resolves to "public package1.A get();" or "public package2.A get();" depending on where A is + return null; + } +} diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java new file mode 100644 index 000000000000..2e50d5432960 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java @@ -0,0 +1,4 @@ +package package1; + +public class A { // resolves to "class package1.A" +} diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java.remove b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/A.java.remove new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java new file mode 100644 index 000000000000..416bad3728ec --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java @@ -0,0 +1,9 @@ +package package1; + +import com.B; + +public class C { + { + new B().get(); // resolves to invoking of "com/B.get:()Lpackage1/A;" + } +} diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java.remove b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/C.java.remove new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/Dummy.java b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/Dummy.java new file mode 100644 index 000000000000..d54006392b56 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package1/Dummy.java @@ -0,0 +1,5 @@ +package package1; + +// Dummy class for non-empty package +public class Dummy { +} diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/A.java.new b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/A.java.new new file mode 100644 index 000000000000..a44c4a191fc5 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/A.java.new @@ -0,0 +1,4 @@ +package package2; + +public class A { // resolves to "class package2.A" +} 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 new file mode 100644 index 000000000000..6badaaa8c109 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/C.java.new @@ -0,0 +1,11 @@ +package package2; + +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 + } +} diff --git a/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/Dummy.java b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/Dummy.java new file mode 100644 index 000000000000..0d7e7fa2eac4 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/markDirty/recompileTwinDependencies/src/package2/Dummy.java @@ -0,0 +1,5 @@ +package package2; + +// Dummy class for non-empty package +public class Dummy { +} diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/IncProjectBuilder.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/IncProjectBuilder.java index cb8ca72699fc..b9bf4e5564ec 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/IncProjectBuilder.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/IncProjectBuilder.java @@ -935,7 +935,21 @@ public class IncProjectBuilder { final SourceToOutputMapping sourceToOutputStorage = context.getProjectDescriptor().dataManager.getSourceToOutputMap(target); final ProjectBuilderLogger logger = context.getLoggingManager().getProjectBuilderLogger(); // actually delete outputs associated with removed paths - for (String deletedSource : deletedPaths) { + final Collection pathsForIteration; + if (Utils.IS_TEST_MODE) { + // ensure predictable order in test logs + pathsForIteration = new ArrayList(deletedPaths); + Collections.sort((List)pathsForIteration, new Comparator() { + @Override + public int compare(String o1, String o2) { + return o1.compareTo(o2); + } + }); + } + else { + pathsForIteration = deletedPaths; + } + for (String deletedSource : pathsForIteration) { // deleting outputs corresponding to non-existing source final Collection outputs = sourceToOutputStorage.getOutputs(deletedSource); diff --git a/jps/jps-builders/src/org/jetbrains/jps/incremental/fs/BuildFSState.java b/jps/jps-builders/src/org/jetbrains/jps/incremental/fs/BuildFSState.java index e3d974cf5f0e..e92c0d59a3b0 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/incremental/fs/BuildFSState.java +++ b/jps/jps-builders/src/org/jetbrains/jps/incremental/fs/BuildFSState.java @@ -72,15 +72,24 @@ public class BuildFSState extends FSState { return super.getSourcesToRecompile(context, target); } - //public boolean isMarkedForRecompilation(BuildRootDescriptor rd, File file) { - // final Map> recompile = getDelta(rd.getTarget()).getSourcesToRecompile(); - // //noinspection SynchronizationOnLocalVariableOrMethodParameter - // synchronized (recompile) { - // final Set files = recompile.get(rd); - // return files != null && files.contains(file); - // } - //} + public boolean isMarkedForRecompilation(@Nullable CompileContext context, BuildRootDescriptor rd, File file) { + FilesDelta delta = getRoundDelta(LAST_ROUND_DELTA_KEY, context); + if (delta == null) { + delta = getDelta(rd.getTarget()); + } + + final Map> recompile = delta.getSourcesToRecompile(); + //noinspection SynchronizationOnLocalVariableOrMethodParameter + synchronized (recompile) { + final Set files = recompile.get(rd); + return files != null && files.contains(file); + } + } + /** + * Note: marked file will well be visible as "dirty" only on the next compilation round! + * @throws IOException + */ @Override public boolean markDirty(@Nullable CompileContext context, File file, final BuildRootDescriptor rd, @Nullable Timestamps tsStorage, boolean saveEventStamp) throws IOException { final FilesDelta roundDelta = getRoundDelta(CURRENT_ROUND_DELTA_KEY, context); 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 7f32f11b732e..d3a654a47327 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 @@ -50,14 +50,15 @@ import org.jetbrains.jps.incremental.messages.ProgressMessage; import org.jetbrains.jps.javac.*; import org.jetbrains.jps.model.JpsDummyElement; import org.jetbrains.jps.model.JpsProject; -import org.jetbrains.jps.model.java.JpsJavaExtensionService; -import org.jetbrains.jps.model.java.JpsJavaSdkType; -import org.jetbrains.jps.model.java.LanguageLevel; +import org.jetbrains.jps.model.JpsSimpleElement; +import org.jetbrains.jps.model.java.*; import org.jetbrains.jps.model.java.compiler.*; import org.jetbrains.jps.model.library.sdk.JpsSdk; import org.jetbrains.jps.model.module.JpsModule; import org.jetbrains.jps.model.module.JpsModuleType; +import org.jetbrains.jps.model.module.JpsTypedModuleSourceRoot; import org.jetbrains.jps.service.JpsServiceManager; +import org.jetbrains.jps.util.JpsPathUtil; import javax.tools.*; import java.io.*; @@ -247,6 +248,7 @@ public class JavaBuilder extends ModuleLevelBuilder { exitCode = ExitCode.OK; final Set srcPath = new HashSet(); + collectSourceRoots(chunk, srcPath, chunk.containsTests()? JavaSourceRootType.TEST_SOURCE : JavaSourceRootType.SOURCE); final BuildRootIndex index = pd.getBuildRootIndex(); for (ModuleBuildTarget target : chunk.getTargets()) { for (JavaSourceRootDescriptor rd : index.getTempTargetRoots(target, context)) { @@ -301,6 +303,14 @@ public class JavaBuilder extends ModuleLevelBuilder { return exitCode; } + private static void collectSourceRoots(ModuleChunk chunk, Set srcPath, final JavaSourceRootType rootType) { + for (JpsModule module : chunk.getModules()) { + for (JpsTypedModuleSourceRoot> root : module.getSourceRoots(rootType)) { + srcPath.add(JpsPathUtil.urlToFile(root.getUrl())); + } + } + } + private boolean compileJava( final CompileContext context, ModuleChunk chunk, diff --git a/jps/jps-builders/src/org/jetbrains/jps/javac/JavacMain.java b/jps/jps-builders/src/org/jetbrains/jps/javac/JavacMain.java index c53183c7af27..33e04756db22 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/javac/JavacMain.java +++ b/jps/jps-builders/src/org/jetbrains/jps/javac/JavacMain.java @@ -40,7 +40,7 @@ public class JavacMain { "-d", "-classpath", "-cp", "-bootclasspath" )); private static final Set FILTERED_SINGLE_OPTIONS = new HashSet(Arrays.asList( - /*javac options*/ "-verbose", "-proc:only", "-implicit:class", "-implicit:none", + /*javac options*/ "-verbose", "-proc:only", "-implicit:class", "-implicit:none", "-Xprefer:newer", "-Xprefer:source", /*eclipse options*/"-noExit" )); @@ -211,6 +211,7 @@ public class JavacMain { private static Collection prepareOptions(final Collection options, boolean usingJavac) { final List result = new ArrayList(); if (usingJavac) { + result.add("-Xprefer:source"); result.add("-implicit:class"); // the option supported by javac only } else { // is Eclipse diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java index c7582b815573..6e605db97203 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/MarkDirtyTest.java @@ -52,4 +52,8 @@ public class MarkDirtyTest extends IncrementalTestCase { JpsModuleRootModificationUtil.addDependency(util, lib); doTestBuild(1).assertSuccessful(); } + + public void testRecompileTwinDependencies() { + doTest().assertSuccessful(); + } }