Closed Bug 1096627 Opened 5 years ago Closed 5 years ago

[geckoview] Add build step preventing GeckoView library from depending on more of Fennec

Categories

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

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(1 file)

A while back, I wrote a mailing post [1] about using a free tool called Classycle to investigate the dependencies between what we might deem GeckoView and the rest of Fennec.  I got interested in this again this weekend, and repeated the analysis.

In the interim, the situation has deteriated.  We have several new and large dependencies that I don't think we had before.  I now find that what I think of as "GeckoView" depends on the classes listed as [middle] (see [2]).  I'm not sure whether these "middle" classes are really part of GV; are really part of Fennec; or need to be divided in some way between the two.  I don't think we can afford to tease these things out, at least not all at once.

So I propose that we set down the current situation and ensure it does not regress.  This ticket tracks landing a Classycle definition file and the build support to do this.

[1] https://mail.mozilla.org/pipermail/mobile-firefox-dev/2014-February/000538.html

[2] [middle] = \
  org.mozilla.gecko.prompts.* \
  org.mozilla.gecko.AlertNotification \
  org.mozilla.gecko.FormAssistPopup \
  org.mozilla.gecko.GeckoActivity \
  org.mozilla.gecko.GeckoApp \
  org.mozilla.gecko.GeckoProfileDirectories \
  org.mozilla.gecko.GuestSession \
  org.mozilla.gecko.R \
  org.mozilla.gecko.Tab \
  org.mozilla.gecko.Tabs \
  org.mozilla.gecko.Telemetry \
  org.mozilla.gecko.TelemetryContract \
  org.mozilla.gecko.ThumbnailHelper \
  org.mozilla.gecko.db.BrowserDB \
  org.mozilla.gecko.db.LocalBrowserDB \
  org.mozilla.gecko.distribution.Distribution \
  org.mozilla.gecko.favicons.Favicons \
  org.mozilla.gecko.favicons.OnFaviconLoadedListener
Notes:

* The Classycle definition file was built by hand via a process of trial
  and error.
* The Classycle JAR file is used only at build time.  I don't want to
  land it near code because it shouldn't be included in any build.  In
  particular, landing in mobile/android/base can foul up IDE
  configurations.
* The Classycle license requires redistributing the README.txt with the
  binary assets.
* Even when Proguard is "disabled", the target still runs, so this is a
  good place to put a build step that the built JARs, as a collective,
  are healthy.

gps: I'd appreciate your review on the implementation here.  I'm going
to discuss and socialize this approach over the next days/weeks with
the Fennec team.
Classycle is hosted at http://classycle.sourceforge.net/index.html

(I will include this in the commit message.)
Comment on attachment 8520244 [details] [diff] [review]
Fail Fennec build if GeckoView library depends on more of Fennec.

Review of attachment 8520244 [details] [diff] [review]:
-----------------------------------------------------------------

Code is good and I applaud what you are doing. I love static enforcement checks like this. Just watch out for things that break too often and become annoying. That can be mitigated through better documentation or build output (not an obscure make error).
Attachment #8520244 - Flags: review?(gps) → review+
Thanks, gps!  For the record, this patch built fine in

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=fe49202182f0

(The try build failed during some unrelated package-tests code.)
https://hg.mozilla.org/mozilla-central/rev/a82804264e6e
Assignee: nobody → nalexander
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 36 → mozilla36
You need to log in before you can comment on or make changes to this bug.