Support circular dependencies

We generate a module script with information on all modules in the chunk, then build the whole chunk as "one big module"

Original commit: 60425b15e6

jps/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinBuilder.java

@@ -39,9 +39,13 @@ import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
-import java.util.*;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
-import static org.jetbrains.jet.cli.common.messages.CompilerMessageSeverity.*;
+import static org.jetbrains.jet.cli.common.messages.CompilerMessageSeverity.EXCEPTION;
+import static org.jetbrains.jet.cli.common.messages.CompilerMessageSeverity.INFO;
+import static org.jetbrains.jet.cli.common.messages.CompilerMessageSeverity.WARNING;
 
 public class KotlinBuilder extends ModuleLevelBuilder {
 
@@ -81,29 +85,12 @@ public class KotlinBuilder extends ModuleLevelBuilder {
 
         messageCollector.report(INFO, "Kotlin JPS plugin version " + KotlinVersion.VERSION, CompilerMessageLocation.NO_LOCATION);
 
-        if (chunk.getModules().size() > 1) {
-            // We do not support circular dependencies, but if they are present, we should not break the build,
-            // so we simply yield a warning and report NOTHING_DONE
-            messageCollector.report(
-                    WARNING, "Circular dependencies are not supported. " +
-                             "The following modules depend on each other: " + StringUtil.join(chunk.getModules(), MODULE_NAME, ", ") + ". " +
-                             "Kotlin is not compiled for these modules",
-                    CompilerMessageLocation.NO_LOCATION);
-            return ExitCode.NOTHING_DONE;
-        }
-
         ModuleBuildTarget representativeTarget = chunk.representativeTarget();
 
         // For non-incremental build: take all sources
         if (!dirtyFilesHolder.hasDirtyFiles()) {
             return ExitCode.NOTHING_DONE;
         }
-        List<File> sourceFiles = KotlinSourceFileCollector.getAllKotlinSourceFiles(representativeTarget);
-        //List<File> sourceFiles = KotlinSourceFileCollector.getDirtySourceFiles(dirtyFilesHolder);
-
-        if (sourceFiles.isEmpty()) {
-            return ExitCode.NOTHING_DONE;
-        }
 
         File outputDir = representativeTarget.getOutputDir();
 
@@ -118,6 +105,24 @@ public class KotlinBuilder extends ModuleLevelBuilder {
         OutputItemsCollectorImpl outputItemCollector = new OutputItemsCollectorImpl(outputDir);
 
         if (JpsUtils.isJsKotlinModule(representativeTarget)) {
+            if (chunk.getModules().size() > 1) {
+                // We do not support circular dependencies, but if they are present, we do our best should not break the build,
+                // so we simply yield a warning and report NOTHING_DONE
+                messageCollector.report(
+                        WARNING, "Circular dependencies are not supported. " +
+                                 "The following JS modules depend on each other: " + StringUtil.join(chunk.getModules(), MODULE_NAME, ", ") + ". " +
+                                 "Kotlin is not compiled for these modules",
+                        CompilerMessageLocation.NO_LOCATION);
+                return ExitCode.NOTHING_DONE;
+            }
+
+            List<File> sourceFiles = KotlinSourceFileCollector.getAllKotlinSourceFiles(representativeTarget);
+            //List<File> sourceFiles = KotlinSourceFileCollector.getDirtySourceFiles(dirtyFilesHolder);
+
+            if (sourceFiles.isEmpty()) {
+                return ExitCode.NOTHING_DONE;
+            }
+
             File outputFile = new File(outputDir, representativeTarget.getModule().getName() + ".js");
 
             KotlinCompilerRunner.runK2JsCompiler(
@@ -129,7 +134,19 @@ public class KotlinBuilder extends ModuleLevelBuilder {
                     outputFile);
         }
         else {
-            File moduleFile = KotlinBuilderModuleScriptGenerator.generateModuleDescription(context, representativeTarget, sourceFiles);
+            if (chunk.getModules().size() > 1) {
+                messageCollector.report(
+                        WARNING, "Circular dependencies are only partially supported. " +
+                                 "The following modules depend on each other: " + StringUtil.join(chunk.getModules(), MODULE_NAME, ", ") + ". " +
+                                 "Kotlin will compile them, but some strange effect may happen",
+                        CompilerMessageLocation.NO_LOCATION);
+            }
+
+            File moduleFile = KotlinBuilderModuleScriptGenerator.generateModuleDescription(context, chunk);
+            if (moduleFile == null) {
+                // No Kotlin sources found
+                return ExitCode.NOTHING_DONE;
+            }
 
             KotlinCompilerRunner.runK2JvmCompiler(
                     messageCollector,

jps/jps-plugin/src/org/jetbrains/jet/jps/build/KotlinBuilderModuleScriptGenerator.java

@@ -20,14 +20,17 @@ import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionGenerator;
+import org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionBuilder;
-import org.jetbrains.jet.compiler.runner.KotlinModuleXmlGenerator;
+import org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionBuilderFactory;
+import org.jetbrains.jet.compiler.runner.KotlinModuleXmlBuilderFactory;
+import org.jetbrains.jps.ModuleChunk;
 import org.jetbrains.jps.builders.java.JavaSourceRootDescriptor;
 import org.jetbrains.jps.incremental.CompileContext;
 import org.jetbrains.jps.incremental.ModuleBuildTarget;
 import org.jetbrains.jps.incremental.messages.BuildMessage;
 import org.jetbrains.jps.incremental.messages.CompilerMessage;
-import org.jetbrains.jps.model.java.*;
+import org.jetbrains.jps.model.java.JpsAnnotationRootType;
+import org.jetbrains.jps.model.java.JpsJavaSdkType;
 import org.jetbrains.jps.model.library.JpsLibrary;
 import org.jetbrains.jps.model.library.sdk.JpsSdk;
 import org.jetbrains.jps.model.library.sdk.JpsSdkType;
@@ -42,42 +45,61 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
-import static org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionGenerator.DependencyProvider;
+import static org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionBuilder.DependencyProcessor;
+import static org.jetbrains.jet.compiler.runner.KotlinModuleDescriptionBuilder.DependencyProvider;
 import static org.jetbrains.jet.jps.build.JpsUtils.getAllDependencies;
 
 public class KotlinBuilderModuleScriptGenerator {
 
-    public static final KotlinModuleDescriptionGenerator GENERATOR = KotlinModuleXmlGenerator.INSTANCE;
+    public static final KotlinModuleDescriptionBuilderFactory FACTORY = KotlinModuleXmlBuilderFactory.INSTANCE;
 
-    public static File generateModuleDescription(CompileContext context, ModuleBuildTarget target, List<File> sourceFiles)
+    @Nullable
+    public static File generateModuleDescription(CompileContext context, ModuleChunk chunk)
             throws IOException
     {
+        KotlinModuleDescriptionBuilder builder = FACTORY.create();
+
+        boolean noSources = true;
+
+        for (ModuleBuildTarget target : chunk.getTargets()) {
+            File outputDir = getOutputDir(target);
+
+            List<File> sourceFiles = KotlinSourceFileCollector.getAllKotlinSourceFiles(target);
+            noSources &= sourceFiles.isEmpty();
+
+            builder.addModule(
+                    target.getId(),
+                    outputDir.getAbsolutePath(),
+                    getKotlinModuleDependencies(context, target),
+                    sourceFiles,
+                    target.isTests(),
+                    // this excludes the output directory from the class path, to be removed for true incremental compilation
+                    Collections.singleton(outputDir)
+            );
+        }
+
+        if (noSources) return null;
+
+        File scriptFile = new File(getOutputDir(chunk.representativeTarget()), "script." + FACTORY.getFileExtension());
+
+        writeScriptToFile(context, builder.asText(), scriptFile);
+
+        return scriptFile;
+    }
+
+    @NotNull
+    private static File getOutputDir(@NotNull ModuleBuildTarget target) {
         File outputDir = target.getOutputDir();
         if (outputDir == null) {
             throw new IllegalStateException("No output directory found for " + target);
-
         }
-        CharSequence moduleScriptText = GENERATOR.generateModuleScript(
+        return outputDir;
-                target.getId(),
-                outputDir.getAbsolutePath(),
-                getKotlinModuleDependencies(context, target),
-                sourceFiles,
-                target.isTests(),
-                // this excludes the output directory from the class path, to be removed for true incremental compilation
-                Collections.singleton(outputDir)
-        );
-
-        File scriptFile = new File(outputDir, "script." + GENERATOR.getFileExtension());
-
-        writeScriptToFile(context, moduleScriptText, scriptFile);
-
-        return scriptFile;
     }
 
     private static DependencyProvider getKotlinModuleDependencies(final CompileContext context, final ModuleBuildTarget target) {
         return new DependencyProvider() {
             @Override
-            public void processClassPath(@NotNull KotlinModuleDescriptionGenerator.DependencyProcessor processor) {
+            public void processClassPath(@NotNull DependencyProcessor processor) {
                 processor.processClassPathSection("Classpath", findClassPathRoots(target));
                 processor.processClassPathSection("Java Source Roots", findSourceRoots(context, target));
                 processor.processAnnotationRoots(findAnnotationRoots(target));

jps/jps-plugin/test/org/jetbrains/jet/jps/build/KotlinJpsBuildTestCase.java

@@ -19,8 +19,11 @@ package org.jetbrains.jet.jps.build;
 import com.intellij.openapi.util.Condition;
 import com.intellij.openapi.util.io.FileUtil;
 import com.intellij.util.containers.ContainerUtil;
-import org.jetbrains.jps.model.java.*;
+import org.jetbrains.jps.builders.BuildResult;
+import org.jetbrains.jps.model.java.JpsJavaDependencyScope;
+import org.jetbrains.jps.model.java.JpsJavaExtensionService;
 import org.jetbrains.jps.model.module.JpsModule;
+import org.jetbrains.jps.util.JpsPathUtil;
 
 import java.io.File;
 
@@ -89,6 +92,22 @@ public class KotlinJpsBuildTestCase extends AbstractKotlinJpsBuildTestCase {
         doTest();
     }
 
+    public void testCircularDependenciesWithKotlinFilesDifferentPackages() {
+        initProject();
+        BuildResult result = makeAll();
+
+        // Check that outputs are located properly
+        for (JpsModule module : myProject.getModules()) {
+            if (module.getName().equals("module2")) {
+                assertFileExistsInOutput(module, "kt1/Kt1Package.class");
+            }
+            if (module.getName().equals("kotlinProject")) {
+                assertFileExistsInOutput(module, "kt2/Kt2Package.class");
+            }
+        }
+        result.assertSuccessful();
+    }
+
     public void testTestDependencyLibrary() throws Throwable {
         initProject();
         addKotlinRuntimeDependency(JpsJavaDependencyScope.TEST, myProject.getModules(), false);
@@ -109,4 +128,24 @@ public class KotlinJpsBuildTestCase extends AbstractKotlinJpsBuildTestCase {
         makeAll().assertSuccessful();
     }
 
+    private static void assertFileExistsInOutput(JpsModule module, String relativePath) {
+        String outputUrl = JpsJavaExtensionService.getInstance().getOutputUrl(module, false);
+        assertNotNull(outputUrl);
+        File outputDir = new File(JpsPathUtil.urlToPath(outputUrl));
+        File outputFile = new File(outputDir, relativePath);
+        assertTrue("Output not written: " + outputFile.getAbsolutePath() + "\n Directory contents: \n" + dirContents(outputFile.getParentFile()),
+                   outputFile.exists());
+    }
+
+    private static String dirContents(File dir) {
+        File[] files = dir.listFiles();
+        if (files == null) {
+            return "<not found>";
+        }
+        StringBuilder builder = new StringBuilder();
+        for (File file : files) {
+            builder.append(" * ").append(file.getName()).append("\n");
+        }
+        return builder.toString();
+    }
 }

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/kotlinProject.iml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+    <orderEntry type="module" module-name="module2" />
+  </component>
+</module>
+

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/kotlinProject.ipr

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CompilerConfiguration">
+    <option name="DEFAULT_COMPILER" value="Javac" />
+    <resourceExtensions />
+    <annotationProcessing>
+      <profile default="true" name="Default" enabled="false">
+        <processorPath useClasspath="true" />
+      </profile>
+    </annotationProcessing>
+  </component>
+  <component name="CopyrightManager" default="">
+    <module2copyright />
+  </component>
+  <component name="DependencyValidationManager">
+    <option name="SKIP_IMPORT_STATEMENTS" value="false" />
+  </component>
+  <component name="Encoding" useUTFGuessing="true" native2AsciiForPropertiesFiles="false" />
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/kotlinProject.iml" filepath="$PROJECT_DIR$/kotlinProject.iml" />
+      <module fileurl="file://$PROJECT_DIR$/module2/module2.iml" filepath="$PROJECT_DIR$/module2/module2.iml" />
+    </modules>
+  </component>
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="IDEA_JDK" project-jdk-type="JavaSDK">
+    <output url="file://$PROJECT_DIR$/out" />
+  </component>
+  <component name="VcsDirectoryMappings">
+    <mapping directory="" vcs="" />
+  </component>
+</project>
+

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/module2/module2.iml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="kotlinProject" />
+    <orderEntry type="library" name="KotlinRuntime" level="project" />
+  </component>
+</module>
+

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/module2/src/JSecond.java

@@ -0,0 +1,5 @@
+public class JSecond {
+    public static void main(String[] args) {
+        new JFirst().foo();
+    }
+}

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/module2/src/kt1.kt

@@ -0,0 +1,7 @@
+package kt1
+
+fun foo() {}
+
+fun bar() {
+    kt2.bar()
+}

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/src/JFirst.java

@@ -0,0 +1,11 @@
+public class JFirst {
+    JSecond s = null;
+
+    public void foo() {
+
+    }
+
+    public static void main(String[] args) {
+        System.out.println(1);
+    }
+}

jps/jps-plugin/testData/CircularDependenciesWithKotlinFilesDifferentPackages/src/kt2.kt

@@ -0,0 +1,7 @@
+package kt2
+
+fun foo() {
+    kt1.foo()
+}
+
+fun bar() {}