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

X has a different signature in current version #361

Closed
eed3si9n opened this issue Jul 26, 2019 · 6 comments
Closed

X has a different signature in current version #361

eed3si9n opened this issue Jul 26, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@eed3si9n
Copy link

eed3si9n commented Jul 26, 2019

steps

@xuwei-k reported in https://twitter.com/not_xuwei_k/status/1153731362970644480

checking 2.12.9-bin-d386f96 against 2.12.8 using Mima 0.5.0, you get enormous amount of incompatibilities. some signatures on the contructors are gone. also its reporting the type name change as incompatibility as well.

problem

https://gist.github.com/xuwei-k/7ec6e97c5af549469ce1f4d59ab80e8a


method this(java.lang.String)Unit in class scala.throws has a different signature in current version, where it is [N/A] rather than (Ljava/lang/String;)V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.throws.this")
method this()Unit in class scala.Predef#<:< has a different signature in current version, where it is [N/A] rather than ()V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.Predef#<:<.this")
method this()Unit in class scala.Predef#=:= has a different signature in current version, where it is [N/A] rather than ()V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.Predef#=:=.this")
method this(Int)Unit in class scala.Array has a different signature in current version, where it is [N/A] rather than (I)V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.Array.this")
method this()Unit in class scala.UniquenessCache has a different signature in current version, where it is [N/A] rather than ()V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.UniquenessCache.this")
method this()Unit in class scala.Option has a different signature in current version, where it is [N/A] rather than ()V
filter with: ProblemFilters.exclude[IncompatibleSignatureProblem]("scala.Option.this")
...

expectation

Changes to signature should not be reported as an error unless jar-swap can cause linkage error. Mima 0.5.0 is stricter than Scala's bincompat requirements.

@lrytz noted:

Signatures are not covered by our binary compatibility promises, which are basically "no LinkageErrors" (https://docs.scala-lang.org/overviews/core/binary-compatibility-of-scala-releases.html).

@eed3si9n eed3si9n added the bug label Jul 26, 2019
@dwijnand
Copy link
Collaborator

Thank you for the report!

@dwijnand dwijnand added this to the 0.6.0 milestone Jul 27, 2019
@dwijnand dwijnand self-assigned this Jul 27, 2019
@raboof
Copy link
Contributor

raboof commented Jul 28, 2019

While indeed the signature is not necessarily part of the typical binary compatibility promise, a changed generic signature can cause problems (e.g. a program without casts throwing ClassCastException at run time). For example for Akka, we think it is useful to get warnings for those (and explicitly filter them after reviewing).

If you don't care about those, they are easily silenced with one ProblemFilters.exclude[IncompatibleSignatureProblem]("*").

I would be sad to see the feature (#299) disappear completely. I guess it could be made opt-in. See also #335.

@dwijnand
Copy link
Collaborator

dwijnand commented Jul 28, 2019

I think "lack of signatures" on constructors are safe to ignore (by MiMa), so we can do that. I need to still see those type name change errors.

I agree that MiMa's scope should include more than LinkageErrors. We can add opt-outs, if need-be.

@eed3si9n
Copy link
Author

Anything that can cause runtime error by a simple JAR swap is a fair game for Mima (migration-manager) to warn.

However, given many libraries adopt Mima, it should try to minimize any false positives.

@dwijnand
Copy link
Collaborator

However, given many libraries adopt Mima, it should try to minimize any false positives.

I agree. Sadly the historic status for MiMa on false positives has been: yeah, they're there (e.g. package private, @ApiMayChange, etc)... Those should be addressed.

@dwijnand
Copy link
Collaborator

I think the

method this()Unit in class scala.Option has a different signature in current version, where it is [N/A] rather than ()V

I've fixed in #345 already.

See #364, where I gave an overview and repeated @raboof's suggestion of using ProblemFilters.exclude[IncompatibleSignatureProblem]("*") if you want to opt-out of the feature.

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

No branches or pull requests

3 participants