From 5793eeeab0415a22449b4efdd965d32d054a93a9 Mon Sep 17 00:00:00 2001 From: Tobias Roeser Date: Tue, 23 Jan 2024 11:05:43 +0100 Subject: [PATCH] Detect colliding cross module values (#2984) Also add an implicit `ToSegements[os.SubPath]` to accept `os.SubPath` as values in cross modules. Fix https://github.com/com-lihaoyi/mill/issues/2835 Pull request: https://github.com/com-lihaoyi/mill/pull/2984 --- .../failure/cross-collisions/repo/build.sc | 7 +++ .../test/src/CrossCollisionsTests.scala | 19 +++++++ main/api/src/mill/api/MillException.scala | 8 ++- main/define/src/mill/define/Cross.scala | 57 ++++++++++++------- 4 files changed, 68 insertions(+), 23 deletions(-) create mode 100644 integration/failure/cross-collisions/repo/build.sc create mode 100644 integration/failure/cross-collisions/test/src/CrossCollisionsTests.scala diff --git a/integration/failure/cross-collisions/repo/build.sc b/integration/failure/cross-collisions/repo/build.sc new file mode 100644 index 00000000000..6c3365e52ba --- /dev/null +++ b/integration/failure/cross-collisions/repo/build.sc @@ -0,0 +1,7 @@ +import mill._ + +object foo extends Cross[Foo]( + (Seq("a", "b"), Seq("c")), + (Seq("a"), Seq("b", "c")) + ) +trait Foo extends Cross.Module2[Seq[String], Seq[String]] diff --git a/integration/failure/cross-collisions/test/src/CrossCollisionsTests.scala b/integration/failure/cross-collisions/test/src/CrossCollisionsTests.scala new file mode 100644 index 00000000000..b722d4902b3 --- /dev/null +++ b/integration/failure/cross-collisions/test/src/CrossCollisionsTests.scala @@ -0,0 +1,19 @@ +package mill.integration + +import utest._ + +object CrossCollisionsTests extends IntegrationTestSuite { + val tests: Tests = Tests { + initWorkspace() + + test("detect-collision") { + val res = evalStdout("resolve", "foo._") + assert(!res.isSuccess) + assert( + res.err.contains( + "Cross module millbuild.build#foo contains colliding cross values: List(List(a, b), List(c)) and List(List(a), List(b, c))" + ) + ) + } + } +} diff --git a/main/api/src/mill/api/MillException.scala b/main/api/src/mill/api/MillException.scala index 94b143ee307..aa3054a911d 100644 --- a/main/api/src/mill/api/MillException.scala +++ b/main/api/src/mill/api/MillException.scala @@ -6,5 +6,9 @@ package mill.api */ class MillException(msg: String) extends Exception(msg) -class BuildScriptException(msg: String) - extends MillException("Build script contains errors:\n" + msg) +class BuildScriptException(msg: String, script: Option[String]) + extends MillException( + script.map(_ + ": ").getOrElse("") + "Build script contains errors:\n" + msg + ) { + def this(msg: String) = this(msg, None) +} diff --git a/main/define/src/mill/define/Cross.scala b/main/define/src/mill/define/Cross.scala index a125420eaba..101a5d7a3c7 100644 --- a/main/define/src/mill/define/Cross.scala +++ b/main/define/src/mill/define/Cross.scala @@ -1,8 +1,9 @@ package mill.define -import mill.api.Lazy +import mill.api.{BuildScriptException, Lazy} import language.experimental.macros +import scala.collection.mutable import scala.reflect.ClassTag import scala.reflect.macros.blackbox @@ -111,6 +112,7 @@ object Cross { implicit object ShortToPathSegment extends ToSegments[Short](v => List(v.toString)) implicit object ByteToPathSegment extends ToSegments[Byte](v => List(v.toString)) implicit object BooleanToPathSegment extends ToSegments[Boolean](v => List(v.toString)) + implicit object SubPathToPathSegment extends ToSegments[os.SubPath](v => v.segments.toList) implicit def SeqToPathSegment[T: ToSegments]: ToSegments[Seq[T]] = new ToSegments[Seq[T]]( _.flatMap(implicitly[ToSegments[T]].convert).toList ) @@ -286,28 +288,41 @@ class Cross[M <: Cross.Module[_]](factories: Cross.Factory[M]*)(implicit def cls: Class[_] } - val items: List[Item] = for { - factory <- factories.toList - (crossSegments0, (crossValues0, (cls0, make))) <- - factory.crossSegmentsList.zip(factory.crossValuesListLists.zip(factory.makeList)) - } yield { - val relPath = ctx.segment.pathSegments - val module0 = new Lazy(() => - make( - ctx - .withSegments(ctx.segments ++ Seq(ctx.segment)) - .withMillSourcePath(ctx.millSourcePath / relPath) - .withSegment(Segment.Cross(crossSegments0)) - .withCrossValues(factories.flatMap(_.crossValuesRaw)) - .withEnclosingModule(this) + val items: List[Item] = { + val seen = mutable.Map[Seq[String], Seq[Any]]() + for { + factory <- factories.toList + (crossSegments0, (crossValues0, (cls0, make))) <- + factory.crossSegmentsList.zip(factory.crossValuesListLists.zip(factory.makeList)) + } yield { + seen.get(crossSegments0) match { + case None => // no collision + case Some(other) => // collision + throw new BuildScriptException( + s"Cross module ${ctx.enclosing} contains colliding cross values: ${other} and ${crossValues0}", + Option(ctx.fileName).filter(_.nonEmpty) + ) + } + val relPath = ctx.segment.pathSegments + val module0 = new Lazy(() => + make( + ctx + .withSegments(ctx.segments ++ Seq(ctx.segment)) + .withMillSourcePath(ctx.millSourcePath / relPath) + .withSegment(Segment.Cross(crossSegments0)) + .withCrossValues(factories.flatMap(_.crossValuesRaw)) + .withEnclosingModule(this) + ) ) - ) - new Item { - def crossValues = crossValues0.toList - def crossSegments = crossSegments0.toList - def module = module0 - def cls = cls0 + val item = new Item { + def crossValues = crossValues0.toList + def crossSegments = crossSegments0.toList + def module = module0 + def cls = cls0 + } + seen.update(crossSegments0, crossValues0) + item } }