1107134 - Classycle build error

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Chris Kitching [:ckitching]]

My builds fail reliably with this error:

 7:27.99 show onlyShortestPaths allResults
 7:27.99 Set [lib] has 119 classes.
 7:27.99 Set [middle] has 27 classes.
 7:27.99 Set [main] has 679 classes.
 7:27.99 check [lib] directlyIndependentOf [main]
 7:27.99   Unexpected dependencies found:
 7:27.99   org.mozilla.gecko.GeckoProfile
 7:27.99     -> org.mozilla.gecko.BrowserApp
 7:27.99   org.mozilla.gecko.GeckoThread
 7:27.99     -> org.mozilla.gecko.BrowserApp
 7:27.99   org.mozilla.gecko.GeckoEvent
 7:27.99     -> org.mozilla.gecko.GeckoHalDefines
 7:28.01 Makefile:151: recipe for target '.geckoview.deps' failed


Talking to Nick, not everyone experiences this problem.

Looking into it, the only dependencies that exist between these classes are constants. 
Applying Proguard before Classycle would inline these constants and stop Classycle from being able to detect a dependency, but it looks like Proguard is made to run after Classycle (which is correct), so that doesn't seem to be the method by which others can avoid this problem.

Anyway - what do you want to do, Nick? Ignore dependencies that are just constants? Move the constants? Something else?

Comment 1

[Chris Kitching [:ckitching]]

Move the constants it is, then.

Unfortunately, each appears to be a symptom of a larger problem. For instance, the

org.mozilla.gecko.GeckoEvent -> org.mozilla.gecko.GeckoHalDefines

dependency is causing upset due to createSensorEvent:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoEvent.java#530

The fundamental problem here appears to be that GeckoEvent really wants to be more than one class. At the moment it's one gigantic class with zillions of fields only a few of which are ever in use. It also contains all the factory methods for every possible kind of event... It's pretty gnarly.
Since some events (such as these sensor events) don't matter to GeckoView, what we really want to do is just omit GeckoSensorEvent from GeckoView-land. Unfortunately there's no such thing as a GeckoSensorEvent.

It appears to be time to refactor the universe.

Comment 2

[Nick Alexander :nalexander [he/him]]

This needs to be disabled, unfortunately -- we're seeing too many problems in the wild.  I'll prep the patch.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: bz://1107134/nalexander

Comment 4

[Nick Alexander :nalexander [he/him]]

/r/1317 - Bug 1107134 - Disable GeckoView independence testing due to Classycle inlining bug. r=ckitching

Pull down this commit:

hg pull review -r 9040c77c604c8b76c4bfe1142c7efa015a82dcb5

Comment 5

[Chris Kitching [:ckitching]]

https://reviewboard.mozilla.org/r/1315/#review695

Comment 6

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/8717995e0514

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8717995e0514

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8533305 [details]
MozReview Request: bz://1107134/nalexander

Requesting uplift to disable this check on mozilla-beta. This makes it much harder for devs to verify changes before uplift.

(The fix is already in 37.)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

It'll show up on the regular uplift queries once approved. No need to ni? me for that :)

Comment 10

[Richard Newman [:rnewman]]

Figured you might want to a=kindaNPOTB it or something!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

(In reply to Richard Newman [:rnewman] from comment #10)
> Figured you might want to a=kindaNPOTB it or something!

I'm not someone who can officially approve patches, so there's nothing I can do with that request :P

https://hg.mozilla.org/releases/mozilla-beta/rev/663d2c344792

Comment 13

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1107134 - Disable GeckoView independence testing due to Classycle inlining bug. r=ckitching