mima failed to detect a removed inner object of an object

[johnynek]

This diff:

twitter/algebird@v0.12.0...v0.12.1#diff-cc08cecfae24bee3eeb817251357478dL1349

in retrospect, pretty clearly breaks binary compatibility: it is removing an object. It is not a source incompatibility if people used it implicitly. We run mima in CI (0.1.9 for this one) and it passed this for algebird 0.12.0 -> 0.12.1. I just tried mima 0.1.10, and it still passes too.

We discovered this with a failure to compile when a target that was expecting 0.12.0 got 0.12.2 (which is mima clean, but not binary compatible).

It is a bit worrying, since we have been delegating this thinking to mima.

[dotta]

@johnynek Thanks for reporting, this is definitely a serious bug.

I believe the issue has to do with how accessible classes are derived. Specifically, the implementation of PackageInfo#accessibleClasses is clearly returning the wrong answer for inner objects. The current implementation expects that for any object Foo compiled by scalac, both Foo.class and Foo$.class classfiles are produced. Because Foo$.class and Foo.class are essentially mirrors, and users never access directly Foo$.class, MiMa will only traverse Foo.class. This works well for top-level objects, but it breaks for inner objects, as demonstrated by this ticket. The reason is that for an inner object, no mirror class seems to be produced by scalac. For instance, if you compile the following with Scala 2.11:

object A {
  object B
}

Three classfiles are produced: A.class, A$.class and A$B$.class. Note that A$B.class is missing, and that's why MiMa ignores the inner object, i.e., MiMa believes the inner object is not reachable from user code, and hence it won't explore it.

[dotta]

@adriaanm @lrytz Why is scalac in the above example not producing a A$B.class? What's the rationale for compiling inner objects differently?

[johnynek]

Thanks for looking at this issue and isolating the bug. How hard will it be
to fix? (Apologies for not looking in detail to answer that question
myself).

I really appreciate Mima and would love to see it get more adoption so
scala libraries can have better long term compatibility. As a scala user in
large industrial code bases, transitive version conflicts are a pain point
I'd love to see an incremental solution on.
On Fri, Sep 30, 2016 at 22:57 Mirco Dotta [email protected] wrote:

@adriaanm https://github.com/adriaanm @lrytz https://github.com/lrytz
Why is Scala not producing a A$B.class? What's the rationale for
compiling inner objects differently?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#127 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEJdoHWyAn665q5LTusiSCTaxBbcBWjks5qviB9gaJpZM4KLWU_
.

[dotta]

@johnynek There is already a PR with a fix ;-) See #128

[johnynek]

So good! Sorry for not reading more carefully! Amazing work.
On Sun, Oct 2, 2016 at 10:15 Mirco Dotta [email protected] wrote:

@johnynek https://github.com/johnynek There is already a PR with a fix
;-) See #128 #128

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#127 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEJdipC-i5iRHApptiGNyKiJCBLPsGgks5qwBD4gaJpZM4KLWU_
.

[dotta]

The fix in #128 wasn't good, as pointed out by @retronym (thanks!). Unfortunately, I can't dedicate more time to this atm. For whoever is going to work on this, make sure to look at the pointed PR as it contains relevant info for how to properly fix the issue.