lightbend-labs/mima

Support `mimaReportBinaryIssues / skip`

armanbilge opened this issue ยท 18 comments

As in publish / skip := true and friends, as a way to disable this task.

We've currently implemented this in typelevel/sbt-typelevel#117 (comment) to help accommodate build matrices with holes in them, where certain modules are not available for certain Scala versions.

Is this something you're open to? I can try and help with a PR. Thanks!

I think currently the typical ways to disable mima for some subprojects are to set mimaPreviousArtifacts := Nil or to disablePlugins(MimaPlugin).

It seems a bit confusing to also look at skip. Do I understand correctly that the advantage would be that you can enable skip on a subproject "wholesale", and it would apply to publishing etc and mima "in one go"? That seems like it might be useful and weigh up against the added complexity.

Thanks for the response!

It seems a bit confusing to also look at skip. Do I understand correctly that the advantage would be that you can enable skip on a subproject "wholesale", and it would apply to publishing etc and mima "in one go"?

Yes, that's exactly what we are doing. This is to deal with "holes" in the cross matrix. For example if you have a build set up like:

lazy val core = project.settings(crossScalaVersions := Seq("2.13.8", "3.0.2"))
lazy val laggard = project.dependsOn(core).settings(crossScalaVersions := Seq("2.13.8"))

If in sbt you run +mimaReportBinaryIssues, the ++2.13.8 step executes fine. But in the ++3.0.2 step the build gets into an "inconsistent" state, where for core scalaVersion := "3.0.2" but for laggard scalaVersion := "2.13.8". Then, the laggard/Compile/compile task gets into trouble, because it cannot resolve the dependency to core_2.13. And in fact since that module doesn't support Scala 3 it really shouldn't be doing anything at all ... which leads us to defining laggard/skip that is conditional on Scala version. :)

This is to deal with "holes" in the cross matrix. For example if you have a build set up like:

lazy val core = project.settings(crossScalaVersions := Seq("2.13.8", "3.0.2"))
lazy val laggard = project.dependsOn(core).settings(crossScalaVersions := Seq("2.13.8"))

If in sbt you run +mimaReportBinaryIssues, the ++2.13.8 step executes fine. But in the ++3.0.2 step the build gets into an "inconsistent" state, where for core scalaVersion := "3.0.2" but for laggard scalaVersion := "2.13.8". Then, the laggard/Compile/compile task gets into trouble, because it cannot resolve the dependency to core_2.13. And in fact since that module doesn't support Scala 3 it really shouldn't be doing anything at all ... which leads us to defining laggard/skip that is conditional on Scala version. :)

Hmm, I remember we had this problem in Akka, but we managed to get around it since akka/akka#30981 - for example, akka-osgi doesn't build for Scala 3, but depends on akka-actor which does. I don't quite remember the details, but looking at the comment I think it used to have something to do with the fact that we did scalaVersion :=.

Anyway, perhaps that's a tangent, having mima look at skip might still make sense.

Yes, I thought you may have. It's a common problem!

This PR introduces a custom +~ command that fixes both these limitations

Sounds cool, but I stopped reading there :) ideally sbt would support this out-of-the-box, but for now skip seems like it can get us close for 99% of use-cases.

This PR introduces a custom +~ command that fixes both these limitations

Sounds cool, but I stopped reading there :) ideally sbt would support this out-of-the-box

Hehe that's fair ;). I think the +~ wasn't really needed to make the build work though, only to re-introduce the functionality we previously implemented using scalaVersion :=.

but for now skip seems like it can get us close for 99% of use-cases.

๐Ÿ‘ ;)

Wait, what's the benefit of mimaReportBinaryIssues / skip over mimaPreviousArtifacts := Nil?

In theory, I can set skip := true for an entire project without needing to know about MiMa at all.

That isn't how scoping in sbt works, is it?

Er, it might not be how it's supposed to work, but that's definitely how it works ๐Ÿ˜…

How what works? You set skip := true and then what behaviour changes? Publishing is skipped? Are test runs skipped?

You set skip := true and any command that supports skip does whatever it does when skip := true (generally, some form of nothing/no-op).

sbt:bobcats> inspect skip
[info] Task: Boolean
[info] Description:
[info]  For tasks that support it (currently only 'compile', 'update', and 'publish'), setting skip to true will force the task to not to do its work.  This exact semantics may vary by task.

That sounds incidental and not something anyone should rely on. Imagine test started supporting it and you blindly update the sbt upgrade. Or maybe you have a project in the build without source code, just some mdoc generation, and then an mdoc upgrade adds "mdoc / skip" support.

I think it's best not to have multiple ways of doing something, and I also don't think it's a good idea to setup "skip := true" in something like sbt-typelevel and start using it in various repos: better to specify what you want to skip (just publishing, etc)

Thanks for the explanation, that seems fair.

I also don't think it's a good idea to setup "skip := true" in something like sbt-typelevel

So, out of curiosity, how would you suggest solving the original problem? Which is to enable "skipping" over holes in the matrices without having to enumerate the tasks to be skipped (since there is no way to know this ahead-of-time).

I would expressly enumerate the tasks to be skipped, because I don't want those to vary under my nose.

Right, but the goal here is to skip all tasks on a particular sub-project, so in a sense it's not varying. Of course in that project's build.sbt you could enumerate all the tasks, but the hope is that the plugin would prevent that boilerplate :)

No, I meant enumerate in the plugin.

That requires the plugin to know all the tasks that are available in the project using the plugin, which isn't possible in general.

Anyway, thanks for your input, much appreciated. There might be no good solution here.

You can do it for all the default tasks, obviously. And for other keys like mimaPreviousArtifacts you can still access them by name (SettingKey[Set[ModuleID]]("mimaPreviousArtifacts")).

Things get harder when their keys use their own types, but it's possible:

Def.settings {
  try {
    val clazz = Class.forName("sbt.nio.Keys$WatchBuildSourceOption")
    val value = Class.forName("sbt.nio.Keys$IgnoreSourceChanges$").getDeclaredField("MODULE$").get(null)
    val manifest = new scala.reflect.Manifest[AnyRef]{ def runtimeClass = clazz }
    Seq(
      SettingKey[AnyRef]("onChangedBuildSource")(manifest, sbt.util.NoJsonWriter()) in Global := value
    )
  } catch {
    case e: Throwable =>
      Nil
  }
}