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

did 2.2.0 break bincompat on 2.11? #367

Closed
SethTisue opened this issue Sep 15, 2020 · 7 comments
Closed

did 2.2.0 break bincompat on 2.11? #367

SethTisue opened this issue Sep 15, 2020 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@SethTisue
Copy link
Member

SethTisue commented Sep 15, 2020

old tickets that explain the problem are: #288, #292, #249

PR with the breakage: #341, and then I wrongly added a MiMa exclusion in #356

most recent discussion: #356 (but let's continue here)

on #356 @NthPortal wrote:

What is the correct thing to do now?

  • move the method to the package objects, draft a new release ASAP, and make a note that the release was bad and shouldn't be used?
  • leave it as is?
  • have the method in both places?
@SethTisue
Copy link
Member Author

move the method to the package objects, draft a new release ASAP, and make a note that the release was bad and shouldn't be used

I think this is what we should do. Besides the handful of people active on the tickets, probably only Scala Steward users even noticed the release, and if we do a prompt followup release, the steward will send followup PRs.

But I will bring it up at our team meeting in a few hours and see what others think.

@SethTisue SethTisue self-assigned this Sep 15, 2020
@SethTisue SethTisue added the bug Something isn't working label Sep 15, 2020
@SethTisue SethTisue mentioned this issue Sep 15, 2020
2 tasks
@SethTisue
Copy link
Member Author

I just found #162, in which @szeiger and @dwijnand both made the exact same argument I made:

the trait is package-private (which MiMa doesn't see) and should not be extended by users

as a result, releases since 2.1.1 have not been binary compatible with 2.0.0, if we are right that this form of change is not safe

@SethTisue
Copy link
Member Author

SethTisue commented Sep 15, 2020

@julienf are you certain that the change is not safe, even though the trait is package-private? I cannot locate any ticket or discussion in which any case is laid out that this is actually a problem for a trait which the end user cannot extend

@lrytz
Copy link
Member

lrytz commented Sep 15, 2020

I also think the new MiMa rule

exclude[ReversedMissingMethodProblem]("scala.collection.compat.PackageShared.*"), // it's package-private

is correct. The backwards bin compat issue is that subclasses of the trait need to be recompiled to gain a mixin forwarder method which implements the new abstract method in the interface. All subclasses are known, part of the library itself, so we're fine.

@julienrf
Copy link
Contributor

Sorry, I overlooked at the problem, it seems that we are safe!

@SethTisue SethTisue changed the title 2.2.0 broke bincompat on 2.11, what to do? did 2.2.0 break bincompat on 2.11? Sep 15, 2020
@NthPortal
Copy link
Contributor

NthPortal commented Sep 15, 2020

does that mean we can deduplicate the other methods back into PackageShared?

@SethTisue
Copy link
Member Author

@julienrf better a surplus of caution than a shortage!

does that mean we can deduplicate the other methods back into PackageShared?

let's try it: #368

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants