Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when using 0.11 plugin with Mill 0.12 #4390

Open
carlosedp opened this issue Jan 22, 2025 · 12 comments
Open

Error when using 0.11 plugin with Mill 0.12 #4390

carlosedp opened this issue Jan 22, 2025 · 12 comments

Comments

@carlosedp
Copy link
Contributor

carlosedp commented Jan 22, 2025

We have been discussing this issue at ckipp01/mill-ci-release#134 and ckipp01/mill-ci-release#143 regarding using a 0.11 plugin (which also has a 0.12 version published) but the project gets the 0.11 as default since it's supposedly binary compatible.

In this case, when using the mill-ci-release plugin like import $ivy.io.chris-kipp::mill-ci-release::0.3.0`` on my mill-aliases plugin (which is in 0.12.5 in this branch: https://github.com/carlosedp/mill-aliases/tree/mill012), I get:

❯ ./mill io.kipp.mill.ci.release.ReleaseModule/publishAll
[build.sc-64/68] compile
[build.sc-64] [info] compiling 1 Scala source to /Users/cdepaula/repos/scala/mill-aliases/out/mill-build/compile.dest/classes ...
[build.sc-64] [info] done compiling
[68/68] ============================== io.kipp.mill.ci.release.ReleaseModule/publishAll ============================== 4s
java.lang.ClassCastException: class scala.Tuple2 cannot be cast to class scala.Tuple3 (scala.Tuple2 and scala.Tuple3 are in unnamed module of loader 'app')
    mill.resolve.Resolve$.$anonfun$invokeCommand0$1(Resolve.scala:163)
    mill.resolve.Resolve$.$anonfun$invokeCommand0$1$adapted(Resolve.scala:163)
    scala.collection.MapOps$WithFilter.$anonfun$withFilter$1(Map.scala:402)
    scala.collection.MapOps$WithFilter.$anonfun$withFilter$1$adapted(Map.scala:402)
    scala.collection.Iterator$$anon$6.hasNext(Iterator.scala:479)
    scala.collection.Iterator$$anon$10.hasNext(Iterator.scala:601)
    scala.collection.immutable.List.prependedAll(List.scala:152)
    scala.collection.immutable.List$.from(List.scala:685)
    scala.collection.immutable.List$.from(List.scala:682)
    scala.collection.IterableFactory$Delegate.from(Factory.scala:288)
    scala.collection.immutable.Iterable$.from(Iterable.scala:35)
    scala.collection.immutable.Iterable$.from(Iterable.scala:32)
    scala.collection.IterableOps$WithFilter.flatMap(Iterable.scala:903)
    mill.resolve.Resolve$.invokeCommand0(Resolve.scala:163)
    mill.resolve.Resolve$.$anonfun$instantiateCommand$1(Resolve.scala:148)

The way to overcome this is to hard-code the mill 0.12 version of the plugin which makes it work:

import $ivy.`io.chris-kipp::mill-ci-release_mill0.12:0.3.0`

I understand it should either work with the 0.11 version or fetch the 0.12 version in case it's available.

Maybe a workaround would be having a variable that holds the Mill binary version (which should be "0.11", "0.12", etc) that we could import like:

import $ivy.`io.chris-kipp::mill-ci-release_mill$MILL_BIN_VERSION:0.3.0`

Or something like that.

@lefou
Copy link
Member

lefou commented Jan 22, 2025

Your description of the issue is a bit confusing. It's not clear to me how to reproduce. Can you provide steps to reproduce?

Maybe a workaround would be having a variable that holds the Mill binary version (which should be "0.11", "0.12", etc) that we could import like:

This is exactly how it works currently. Except that Mill 0.12 is still using MILL_BIN_VERSION = 0.11, so there is no 0.12. Read more here: https://mill-build.org/mill/extending/import-ivy-plugins.html#_importing_plugins

@lefou
Copy link
Member

lefou commented Jan 22, 2025

No plugin should use a _mill0.12 artifact suffix to date, since this will confuse a lot of users. The _millX suffix is informally reserved to address Mill binary platforms, and there is currently no 0.12 binary platform. There is also none planned.

If your plugin only works for Mill 0.12 than of course you can depend on any newer versions. This is fine. Other plugins do that too, e.g. mill-scalafix typically bumps its minimal supported Mill version shortly after a Mill release. This should only be done when necessary, as it limits the applicability for some users. Hence, you may want to communicate this to your users via other means. But you should keep the _mill0.11 suffix even when you only target Mill 0.12, otherwise users won't be able to consume it as they are supposed and used to.

The next planned bump of the the Mill binary version will be Mill 0.13.

@carlosedp
Copy link
Contributor Author

carlosedp commented Jan 22, 2025

Based on the simplest project from Mill (https://mill-build.org/mill/scalalib/intro.html#_simple_scala_module), a build.mill file like:

package build
import mill._, scalalib._, publish._

import $ivy.`io.chris-kipp::mill-ci-release::0.3.0`
import io.kipp.mill.ci.release.{CiReleaseModule, SonatypeHost}

trait Publish extends CiReleaseModule {
  def pomSettings = PomSettings(
    description = "Reproducer",
    organization = "com.carlosedp",
    url = "https://github.com/carlosedp/repro",
    licenses = Seq(License.MIT),
    versionControl = VersionControl.github("carlosedp", "repro"),
    developers = Seq(
      Developer("carlosedp", "Carlos Eduardo de Paula", "https://github.com/carlosedp")
    ),
  )
  def artifactName = "repro"
  override def sonatypeHost = Some(SonatypeHost.s01)
}

object foo extends ScalaModule with Publish {
  def scalaVersion = "2.13.16"
  def ivyDeps = Agg(
    ivy"com.lihaoyi::scalatags:0.12.0",
    ivy"com.lihaoyi::mainargs:0.6.2"
  )
}

when I run the mill-ci-release command to publish ./mill io.kipp.mill.ci.release.ReleaseModule/publishAll, I get that error.

Hard-coding the version to the _mill0.12 version, it works. Also if I revert to mill 0.11.13 (and rename to build.sc), it also works.

@lefou
Copy link
Member

lefou commented Jan 22, 2025

It looks like it has something to do with a change in the private constructor of the Discover case class. Beside being private, it's used in a pattern match. It looks like the generic type of used map has changed from Tuple2 to Tuple3 and our MiMa tooling might have not catched it due to being in a generic signature which is erased at runtime. I think this could be a bug and there is likely a way to fix or detect and workaround it. But I wonder, why we didn't discover it earlier.

@lihaoyi, could you please have a look?

@ckipp01
Copy link
Contributor

ckipp01 commented Jan 23, 2025

I haven't had time to minimize or reproduce yet, but yesterday I also got the same report here for someone using my mill-dependency-submission plugin now that I allow the 0.12 version to be used in the action. So it looks like from Mill 0.12.x you can't use the published 0.11.x version of that plugin either. It looks to be the same error.

@lihaoyi
Copy link
Member

lihaoyi commented Jan 23, 2025

I remember people reporting this before in one of the previous updates. Might be a similar issue

@lihaoyi
Copy link
Member

lihaoyi commented Jan 23, 2025

Probably caused by #3532 , which increased the tuple size from 2 to 3. It's unfortunate that MIMA didn't catch it, I guess because it's a generic and so there's erasure. I'm also surprised that nobody has reported this sooner.

Given we've released 0.12.0 -> 0.12.5 on this new code, we probably can't reasonably revert the behavior without breaking people again. So we'll probably just have to wait until the Discover data structure is overhauled in 0.13.0

@lefou
Copy link
Member

lefou commented Jan 23, 2025

Can't we cat the runtime type being incompatible and convert it, if there is any reasonable conversion possible?

@nightscape
Copy link
Contributor

Would publishing a separate version for Mill 0.12 and using that instead (as we did for mill-ci-release) be a good intermediate solution then?

@lihaoyi
Copy link
Member

lihaoyi commented Jan 23, 2025

@lefou we might be able to? I think if we change the type from Map[Class[_], (Seq[String], Seq[mainargs.MainData[_, _]], Seq[String])] to Map[Class[_], Any] we could then do runtime type checks and pattern matching at runtime

@lihaoyi
Copy link
Member

lihaoyi commented Jan 23, 2025

Seems like it may be due to the macro codegen generating Tuple2s, which it then puts into a Map[Class[_], Tuple3], which doesn't raise an error because of JVM erasure. It only affects plugins which define ExternalModules, which is why mill-mima/mill-scalafix/etc. that we use internally in Mill's own build appear to be unaffected

@lefou
Copy link
Member

lefou commented Jan 23, 2025

Would publishing a separate version for Mill 0.12 and using that instead (as we did for mill-ci-release) be a good intermediate solution then?

It would. But Mill will not pick up the 0.12 binary platform suffix, and using if for publishing plugins will confuse more people than helping them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants