Closed Bug 1380639 Opened 5 years ago Closed 5 years ago

Add ConstraintLayout to the build system


(Firefox for Android Graveyard :: General, enhancement, P3)




Tracking Status
fennec + ---


(Reporter: sebastian, Unassigned)


(Depends on 1 open bug)


(Whiteboard: [MobileAS] )


(2 files, 5 obsolete files)


ConstraintLayout makes building the Activity Stream UI components much easier. And the flat layouts should have a positive impact on our UI performance.
Nick! I tried to add the constraint layout support library to the build system. First the library is in a different directory than all the other support libs and then it's split into an AAR and a JAR. I got it to build, but at runtime the app crashes because it can't find classes from the JAR. I attached my super hacky patches (Copied, unclean code, I placed the JAR everywhere, ..). Do you have an idea why the classes from the JAR do not end up in our APK?

Coming back from Focus I again realize how painful our old build system is.. :)
Flags: needinfo?(nalexander)
sebastian: you were really close to success; just a few small tweaks, although the missing class took some time for me to get "just so".
Flags: needinfo?(nalexander)
Priority: -- → P2
Whiteboard: [MobileAS]
Thank your for you help, Nick! I'll fold this into a nice patch and make sure that the builders are up-to-date.
Attachment #8886561 - Attachment is obsolete: true
Whiteboard: [MobileAS] → [MobileAS] 1.26
Iteration: --- → 1.26
Priority: P2 → P1
Attachment #8886712 - Attachment is obsolete: true
Attachment #8886713 - Attachment is obsolete: true
Whiteboard: [MobileAS] 1.26 → [MobileAS]

All builds passed. But "deps" is broken because it doesn't seem to find constraint-layout - even though it has been added to the SDK. Once I add jcenter and "" to the build.gradle files manually the deps task passes again:
(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> Try:
> All builds passed. But "deps" is broken because it doesn't seem to find
> constraint-layout - even though it has been added to the SDK. Once I add
> jcenter and "" to the build.gradle files manually the deps
> task passes again:
> jobs?repo=try&revision=20f42977ddd50474fcb32402b1a493c0903e943d&selectedJob=1
> 17070142

I can't explain this broken Deps task -- certainly, just jcenter was working in my earlier push.  I wonder if Google removed libraries from jcenter very recently?

In any case, I think our approach has to be to generalize GRADLE_MAVEN_REPOSITORY to be a set of URLs, and to accept central/jcenter/  Then we build multiple Maven file:/// archives in the Deps task.  This work isn't wasted 'cuz it's specifically for --with-gradle builds.

However, I'm now confused.  How are the constraint-layout{-*} libraries getting into the Android SDK archive used by non-Gradle builds?  It must be that _something_ (the sdk-manager-plugin?) is downloading them during the Gradle build and putting them into place -- and indeed, the logs show:

[task 2017-07-25T09:16:29.975450Z] 09:16:29     INFO -  checking for constraint-layout AAR... /home/worker/workspace/build/src/android-sdk-linux/extras/m2repository/com/android/support/constraint/constraint-layout/1.0.2/constraint-layout-1.0.2.aar
[task 2017-07-25T09:16:30.043912Z] 09:16:30     INFO -  checking for support-annotations JAR... /home/worker/workspace/build/src/android-sdk-linux/extras/android/m2repository/com/android/support/support-annotations/23.4.0/support-annotations-23.4.0.jar

so we're getting them in the regular build from the SDK.  I can only imagine that our ancient Android-Gradle plugin or build-tools version don't know about the new `extras/m2repository`.  That leads me to believe we could (temporarily) add the new maven file:/// URL for the builders.  But the Deps task must not be getting that.

To get more clarity on this, I've added the android-api-15-gradle build job.  I expect it to fail, since the jcenter archive doesn't include the constraint-layout assets, and because our Android-Gradle plugin doesn't know about the SDK location.

Stepping back, I think the real solution here is to stop using the sdk-plugin-manager to download these things (it's already deprecated and new Android-Gradle plugins now do this downloading) and to use |mach bootstrap| to install the Android SDK bits in the Deps task.  I see that the sdkmanager allows to add the constraint-layout library directly, like:

~/.mozbuild/android-sdk-macosx/tools/bin/sdkmanager "extras;m2repository;com;android;support;constraint;constraint-layout;1.0.2"

I have significant work in progress towards this whole upgrade process, but it won't be ready to address this ticket.  We'll have to make one of the approaches -- multiple Maven URLs or special-casing `extras/m2repository` -- work for Deps and the android-api-15-gradle job.
The try build at The try build is looking happy.  I think those patches are all that's needed.

sebastian, you have my r+ for whatever is left after you look over those two patches!
Iteration: 1.26 → 1.27
Whiteboard: [MobileAS] → [MobileAS] 1.26 1.27
Blocks: 1383735
No longer blocks: 1379021
Attachment #8889799 - Attachment is obsolete: true
Attachment #8889799 - Flags: review?(nalexander)
Comment on attachment 8893466 [details]
Bug 1380639 - Update CLOBBER file.

::: CLOBBER:25
(Diff revision 1)
>  # Are you updating CLOBBER because you think it's needed for your WebIDL
>  # changes to stick? As of bug 928195, this shouldn't be necessary! Please
>  # don't change CLOBBER for WebIDL changes any more.
> -Merge day clobber
> +Bug 1380639 - Update dependencies of Fennec build

I really don't expect this to require a clobber, given that the configure changes should invalidate which should invalidate every preprocessed and generated source (including which should invalidate omg.R which should invaliate the entire source tree.

Please land without and if it needs a _follow-up_ clobber file a ticket, since we should solve that problem.
Attachment #8893466 - Flags: review-
Attachment #8893466 - Attachment is obsolete: true
Attachment #8893466 - Flags: review?(michael.l.comella)
..... and gone!
Comment on attachment 8886560 [details]
Bug 1380639 - Use Android SDK that includes constraint-layout.

I'll trust these hashes point in the right places. :P
Attachment #8886560 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8889798 [details]
Bug 1380639 - Add contraint-layout and constraint-layer-solver dependencies.

Build is not my specialty but this looks reasonable to me.
Attachment #8889798 - Flags: review?(michael.l.comella) → review+
Pushed by
Use Android SDK that includes constraint-layout. r=mcomella
Add contraint-layout and constraint-layer-solver dependencies. r=mcomella
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
backed out on request from sebastian:  I want to get this out again. It's green - but it's making landing the Photon work impossible. So I want to get it out again. (Bug 1388023)
Flags: needinfo?(s.kaspari)
Resolution: FIXED → ---
Depends on: 1388023
Will try again after we have a solution for bug 1388023.
Assignee: s.kaspari → nobody
Flags: needinfo?(s.kaspari)
Backout by
Backed out changeset 514cf1f55c6f
Backed out changeset ebb30b6d6184 on request from sebastian
Target Milestone: Firefox 57 → ---
Iteration: 1.27 → 1.28
Iteration: 1.28 → ---
Priority: P1 → P3
tracking-fennec: --- → ?
Whiteboard: [MobileAS] 1.26 1.27 → [MobileAS]
tracking-fennec: ? → +
All open Activity Stream bugs are moving from the whiteboard tag, "[mobileAS]", to the Firefox for Android component, "Activity Stream", so that I can keep better track of these bugs as the new triage owner; I will send out an email shortly with additional details, caveats, etc.
Component: General → Activity Stream
Didn't mean to move this: this isn't required by Activity Stream anymore.
Component: Activity Stream → General
Closing this as won't fix: There are no plans to use this layout. We can re-open or re-file if we want to use it in the future.
Closed: 5 years ago5 years ago
Resolution: --- → WONTFIX
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.