GenIdea: unroll module dependencies (backport of #3355) (#3358)

Since IDEA 2024.2, module dependencies in the `.iml` files are no longer interpreted transitively. Therefore, we need to unroll all transitive module dependencies when generating the project files. Fix #3354 Original Pull request: #3355 Pull request: #3358
com-lihaoyi · Aug 9, 2024 · 5c8c67f · 5c8c67f
1 parent 2a07a78
commit 5c8c67f
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 13 deletions.
diff --git a/idea/src/mill/idea/GenIdeaImpl.scala b/idea/src/mill/idea/GenIdeaImpl.scala
@@ -541,14 +541,20 @@ case class GenIdeaImpl(
 
         val libNames = Strict.Agg.from(sanizedDeps).iterator.toSeq
 
-        val depNames = Strict.Agg
-          .from(mod.moduleDeps.map((_, None)) ++
-            mod.compileModuleDeps.map((_, Some("PROVIDED"))))
-          .filter(!_._1.skipIdea)
-          .map { case (v, s) => ScopedOrd(moduleName(moduleLabels(v)), s) }
-          .iterator
-          .toSeq
-          .distinct
+        val depNames = {
+          val allTransitive = mod.transitiveModuleCompileModuleDeps
+          val recursive = mod.recursiveModuleDeps
+          val provided = allTransitive.filterNot(recursive.contains)
+
+          Strict.Agg
+            .from(recursive.map((_, None)) ++
+              provided.map((_, Some("PROVIDED"))))
+            .filter(!_._1.skipIdea)
+            .map { case (v, s) => ScopedOrd(moduleName(moduleLabels(v)), s) }
+            .iterator
+            .toSeq
+            .distinct
+        }
 
         val isTest = mod.isInstanceOf[TestModule]
 

diff --git a/integration/ide/gen-idea/repo/hello-idea/build.sc b/integration/ide/gen-idea/repo/hello-idea/build.sc
@@ -2,7 +2,7 @@ import mill.api.Loose.Agg
 import mill.define.Target
 import mill._
 import mill.scalajslib.ScalaJSModule
-import mill.scalalib.{Dep, DepSyntax, TestModule}
+import mill.scalalib.{Dep, DepSyntax, JavaModule, TestModule}
 
 trait HelloIdeaModule extends scalalib.ScalaModule {
   def scalaVersion = "2.12.5"
@@ -39,4 +39,21 @@ object HelloIdeaJs extends ScalaJSModule {
       ivy"com.lihaoyi::utest::0.8.4"
     )
   }
+}
+
+object moduleA extends JavaModule
+object moduleB extends JavaModule {
+  override def moduleDeps = Seq(moduleA)
+}
+object moduleC extends JavaModule {
+  override def moduleDeps = Seq(moduleB)
+  override def compileModuleDeps = Seq(moduleE)
+}
+object moduleD extends JavaModule {
+  override def compileModuleDeps = Seq(moduleC)
+}
+object moduleE extends JavaModule
+object moduleF extends JavaModule {
+  override def compileModuleDeps = Seq(moduleC)
+  override def moduleDeps = Seq(moduleB, moduleA)
 }
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulea.iml b/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulea.iml
@@ -0,0 +1,12 @@
+<module type="JAVA_MODULE" version="4">
+    <component name="NewModuleRootManager">
+        <output url="file://$MODULE_DIR$/../../out/moduleA/ideaCompileOutput.dest/classes"/>
+        <exclude-output/>
+        <content url="file://$MODULE_DIR$/../../moduleA">
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleA/src" isTestSource="false"/>
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleA/resources" type="java-resource"/>
+        </content>
+        <orderEntry type="inheritedJdk"/>
+        <orderEntry type="sourceFolder" forTests="false"/>
+    </component>
+</module>
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/moduleb.iml b/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/moduleb.iml
@@ -0,0 +1,13 @@
+<module type="JAVA_MODULE" version="4">
+    <component name="NewModuleRootManager">
+        <output url="file://$MODULE_DIR$/../../out/moduleB/ideaCompileOutput.dest/classes"/>
+        <exclude-output/>
+        <content url="file://$MODULE_DIR$/../../moduleB">
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleB/src" isTestSource="false"/>
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleB/resources" type="java-resource"/>
+        </content>
+        <orderEntry type="inheritedJdk"/>
+        <orderEntry type="sourceFolder" forTests="false"/>
+        <orderEntry type="module" module-name="modulea" exported=""/>
+    </component>
+</module>
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulec.iml b/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulec.iml
@@ -0,0 +1,15 @@
+<module type="JAVA_MODULE" version="4">
+    <component name="NewModuleRootManager">
+        <output url="file://$MODULE_DIR$/../../out/moduleC/ideaCompileOutput.dest/classes"/>
+        <exclude-output/>
+        <content url="file://$MODULE_DIR$/../../moduleC">
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleC/src" isTestSource="false"/>
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleC/resources" type="java-resource"/>
+        </content>
+        <orderEntry type="inheritedJdk"/>
+        <orderEntry type="sourceFolder" forTests="false"/>
+        <orderEntry type="module" module-name="modulea" exported=""/>
+        <orderEntry type="module" module-name="moduleb" exported=""/>
+        <orderEntry type="module" module-name="modulee" exported="" scope="PROVIDED"/>
+    </component>
+</module>
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/moduled.iml b/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/moduled.iml
@@ -0,0 +1,15 @@
+<module type="JAVA_MODULE" version="4">
+    <component name="NewModuleRootManager">
+        <output url="file://$MODULE_DIR$/../../out/moduleD/ideaCompileOutput.dest/classes"/>
+        <exclude-output/>
+        <content url="file://$MODULE_DIR$/../../moduleD">
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleD/src" isTestSource="false"/>
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleD/resources" type="java-resource"/>
+        </content>
+        <orderEntry type="inheritedJdk"/>
+        <orderEntry type="sourceFolder" forTests="false"/>
+        <orderEntry type="module" module-name="modulea" exported="" scope="PROVIDED"/>
+        <orderEntry type="module" module-name="moduleb" exported="" scope="PROVIDED"/>
+        <orderEntry type="module" module-name="modulec" exported="" scope="PROVIDED"/>
+    </component>
+</module>
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulee.iml b/integration/ide/gen-idea/repo/hello-idea/idea/mill_modules/modulee.iml
@@ -0,0 +1,12 @@
+<module type="JAVA_MODULE" version="4">
+    <component name="NewModuleRootManager">
+        <output url="file://$MODULE_DIR$/../../out/moduleE/ideaCompileOutput.dest/classes"/>
+        <exclude-output/>
+        <content url="file://$MODULE_DIR$/../../moduleE">
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleE/src" isTestSource="false"/>
+            <sourceFolder url="file://$MODULE_DIR$/../../moduleE/resources" type="java-resource"/>
+        </content>
+        <orderEntry type="inheritedJdk"/>
+        <orderEntry type="sourceFolder" forTests="false"/>
+    </component>
+</module>
diff --git a/integration/ide/gen-idea/repo/hello-idea/idea/modules.xml b/integration/ide/gen-idea/repo/hello-idea/idea/modules.xml
@@ -8,6 +8,12 @@
             <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/helloideajs.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/helloideajs.iml"/>
             <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/helloideajs.test.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/helloideajs.test.iml"/>
             <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/mill-build.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/mill-build.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/modulea.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/modulea.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/moduleb.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/moduleb.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/modulec.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/modulec.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/moduled.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/moduled.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/modulee.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/modulee.iml"/>
+            <module fileurl="file://$PROJECT_DIR$/.idea/mill_modules/modulef.iml" filepath="$PROJECT_DIR$/.idea/mill_modules/modulef.iml"/>
         </modules>
     </component>
 </project>
diff --git a/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala b/scalalib/api/src/mill/scalalib/api/ZincWorkerUtil.scala
@@ -2,7 +2,6 @@ package mill.scalalib.api
 
 import mill.api.Loose.Agg
 import mill.api.PathRef
-import mill.scalalib.api.Versions
 import scala.util.matching.Regex
 
 trait ZincWorkerUtil {

diff --git a/scalalib/src/mill/scalalib/internal/ModuleUtils.scala b/scalalib/src/mill/scalalib/internal/ModuleUtils.scala
@@ -15,7 +15,18 @@ object ModuleUtils {
     (module.millModuleShared.value.getOrElse(Segments()) ++ module.millModuleSegments).render
   }
 
-  def recursive[T <: Module](name: String, start: T, deps: T => Seq[T]): Seq[T] = {
+  /**
+   * Find all dependencies.
+   * The result contains `start` and all its transitive dependencies provided by `deps`,
+   * but does not contain duplicates.
+   * If it detects a cycle, it throws an exception with a meaningful message containing the cycle trace.
+   * @param name The nane is used in the exception message only
+   * @param start the start element
+   * @param deps A function provided the direct dependencies
+   * @throws BuildScriptException if there were cycles in the dependencies
+   */
+  // FIMXE: Remove or consolidate with copy in ZincModuleImpl
+  def recursive[T](name: String, start: T, deps: T => Seq[T]): Seq[T] = {
 
     @tailrec def rec(
         seenModules: List[T],
@@ -36,7 +47,8 @@ object ModuleUtils {
                 throw new BuildScriptException(msg)
               }
               rec(
-                seenModules ++ Seq(cand),
+                if (seenModules.contains(cand)) seenModules
+                else { seenModules ++ Seq(cand) },
                 toAnalyze = ((cand :: trace, deps(cand).toList)) :: (trace, remaining) :: rest
               )
           }

diff --git a/scalalib/worker/src/mill/scalalib/worker/ZincWorkerImpl.scala b/scalalib/worker/src/mill/scalalib/worker/ZincWorkerImpl.scala
@@ -669,7 +669,8 @@ object ZincWorkerImpl {
                 throw sys.error(msg)
               }
               rec(
-                seenModules ++ Seq(cand),
+                if (seenModules.contains(cand)) seenModules
+                else { seenModules ++ Seq(cand) },
                 toAnalyze = ((cand :: trace, deps(cand).toList)) :: (trace, remaining) :: rest
               )
           }