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

Improve MiMa workflows #12778

Closed
odersky opened this issue Jun 9, 2021 · 9 comments
Closed

Improve MiMa workflows #12778

odersky opened this issue Jun 9, 2021 · 9 comments

Comments

@odersky
Copy link
Contributor

odersky commented Jun 9, 2021

Currently MiMa does not allow any new additions to the standard library, even for PRs slated for the next minor version and even for things marked as @experimental. What you need to do instead is to add exceptions to an exclude list. I append the current exclude list in a comment below. It should be clear that this is a huge source of boilerplate and very opaque to learn for everyone. I think that makes working with MiMa impossible.

We need to

This is a blocker, as least for my workflow.

@odersky
Copy link
Contributor Author

odersky commented Jun 9, 2021

Here's the current exclude list, which covers only one issue for one tiny release. Can you image the work that will be required to keep this updated?

object MiMaFilters {
  val Library: Seq[ProblemFilter] = Seq(
    // New APIs marked @experimental in 3.0.1
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes.valueOrAbort"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#reportModule.errorAndAbort"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.fieldMember"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.fieldMembers"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.methodMember"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.methodMembers"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.typeMember"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.typeMembers"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TermParamClauseMethods.isErased"),
    exclude[DirectMissingMethodProblem]("scala.quoted.Type.valueOfTuple"),
    exclude[MissingClassProblem]("scala.annotation.experimental"),
    exclude[MissingClassProblem]("scala.annotation.internal.ErasedParam"),
    exclude[MissingClassProblem]("scala.annotation.internal.ErasedParam"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes.valueOrAbort"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#reportModule.errorAndAbort"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.fieldMember"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.fieldMembers"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.methodMember"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.methodMembers"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.typeMember"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#SymbolMethods.typeMembers"),
    exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TermParamClauseMethods.isErased"),
  )

@sjrd
Copy link
Member

sjrd commented Jun 9, 2021

  • turn off MiMa checks for PR's labelled "next minor release", or that have an equivalent incantation somewhere.

We could isolate the MiMa checks in a separate GitHub Actions job, that is skipped if the PR title contains a tag like [next-minor-version], similarly to how some things are skipped by [skip ci] or [skip community_build]. /cc @anatoliykmetyuk

  • have MiMa make an exception for things labelled experimental. Similarly to lightbend/mima#160.

That is more difficult. It would have to be done in MiMa itself, I believe.

@julienrf
Copy link
Contributor

julienrf commented Jun 9, 2021

  • turn off MiMa checks for PR's labelled "next minor release", or that have an equivalent incantation somewhere.

It is not clear to me why such PRs are open against the main branch instead of a 3.1.x branch where MiMa would be configured to allow source incompatibilities?

@smarter
Copy link
Member

smarter commented Jun 9, 2021

@julienrf There's no branch for the next minor release, the current master at any point can become the next minor or next patch release depending on whether there are compatibility breaking changes, if we created a branch for the next minor release we would have to constantly sync it with the master branch instead.

@julienrf
Copy link
Contributor

julienrf commented Jun 9, 2021

if we created a branch for the next minor release we would have to constantly sync it with the master branch instead.

I think this is what is done in scala/scala: the 2.12.x branch is regularly merged into the 2.13.x branch. We could have a 3.1.x branch with more relaxed MiMa constraints, where we would regularly merge the content of the 3.0.x branch.

If we plan to do other patch releases after 3.0.1, one solution to be able to still have the CI work for us on draft PRs targeting the next minor release (without creating a 3.1.x branch) would be to relax the MiMa constraints in the PR itself. It should be as simple as changing the baseVersion to 3.1.0.

https://github.com/lampepfl/dotty/blob/2ea25b5e6d9833c1615b556d0c1b7dd55e6a4cef/project/Build.scala#L68

If we don’t plan to do another patch release after 3.0.1, the main branch could already relax the baseVersion.

@smarter
Copy link
Member

smarter commented Jun 9, 2021

We could have a 3.1.x branch with more relaxed MiMa constraints, where we would regularly merge the content of the 3.0.x branch.

Yes, but we explicitly chose to not go with this model and instead to go with a model closer to the rust one: https://doc.rust-lang.org/book/appendix-07-nightly-rust.html

@dwijnand
Copy link
Member

dwijnand commented Jun 9, 2021

That is more difficult. It would have to be done in MiMa itself, I believe.

Happy to look to make this better. It's been next on the mima todo list for a while, just waiting for a strongly motivating case like this is turning into.

@dwijnand
Copy link
Member

dwijnand commented Nov 11, 2021

So this is basically fixed in MiMa 1.0.0 and just needs a version bump (latest is 1.0.1, we're using 0.9.0 atm). I've not been bitten by this yet myself, so I haven't PR the update. But I've seen others been hit so they might be motivated to PR the version bump.

@dwijnand dwijnand removed their assignment Nov 11, 2021
@ckipp01
Copy link
Member

ckipp01 commented May 14, 2023

Seeing that we now use mimaExcludeAnnotations += "scala.annotation.experimental", I believe this can be closed. If this isn't the case and there are still improvements that need to be done, we can further clarify and re-open.

Closed by #14976.

@ckipp01 ckipp01 closed this as completed May 14, 2023
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

6 participants