Closed
Bug 1380639
Opened 7 years ago
Closed 7 years ago
Add ConstraintLayout to the build system
Categories
(Firefox for Android Graveyard :: General, enhancement, P3)
Tracking
(fennec+)
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
fennec | + | --- |
People
(Reporter: sebastian, Unassigned)
References
(Depends on 1 open bug)
Details
(Whiteboard: [MobileAS] )
Attachments
(2 files, 5 obsolete files)
https://developer.android.com/training/constraint-layout/index.html#add-a-constraint
'com.android.support.constraint:constraint-layout:1.0.2'
ConstraintLayout makes building the Activity Stream UI components much easier. And the flat layouts should have a positive impact on our UI performance.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 3•7 years ago
|
||
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)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
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)
Reporter | ||
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: [MobileAS]
Reporter | ||
Comment 7•7 years ago
|
||
Thank your for you help, Nick! I'll fold this into a nice patch and make sure that the builders are up-to-date.
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8886561 -
Attachment is obsolete: true
Reporter | ||
Updated•7 years ago
|
Whiteboard: [MobileAS] → [MobileAS] 1.26
Updated•7 years ago
|
Iteration: --- → 1.26
Priority: P2 → P1
Reporter | ||
Updated•7 years ago
|
Attachment #8886712 -
Attachment is obsolete: true
Reporter | ||
Updated•7 years ago
|
Attachment #8886713 -
Attachment is obsolete: true
Reporter | ||
Updated•7 years ago
|
Whiteboard: [MobileAS] 1.26 → [MobileAS]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 12•7 years ago
|
||
Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=404918cabae8
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 "maven.google.com" to the build.gradle files manually the deps task passes again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20f42977ddd50474fcb32402b1a493c0903e943d&selectedJob=117070142
Comment 13•7 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> Try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=404918cabae8
>
> 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 "maven.google.com" to the build.gradle files manually the deps
> task passes again:
> https://treeherder.mozilla.org/#/
> 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/maven.google.com. 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.
Comment 14•7 years ago
|
||
Comment 15•7 years ago
|
||
Comment 16•7 years ago
|
||
Comment 17•7 years ago
|
||
The try build at The try buildhttps://treeherder.mozilla.org/#/jobs?repo=try&revision=1e5f555f0dc734171094869135ce81ca6ee1fe68&selectedJob=119133313 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!
Updated•7 years ago
|
Iteration: 1.26 → 1.27
Whiteboard: [MobileAS] → [MobileAS] 1.26 1.27
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Comment 18•7 years ago
|
||
Reporter | ||
Comment 19•7 years ago
|
||
Reporter | ||
Comment 20•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Updated•7 years ago
|
Attachment #8889799 -
Attachment is obsolete: true
Attachment #8889799 -
Flags: review?(nalexander)
Comment hidden (mozreview-request) |
Comment 24•7 years ago
|
||
mozreview-review |
Comment on attachment 8893466 [details]
Bug 1380639 - Update CLOBBER file.
https://reviewboard.mozilla.org/r/164546/#review169860
::: 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 Makefile.in which should invalidate every preprocessed and generated source (including AndroidManifest.xml.in) 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-
Reporter | ||
Updated•7 years ago
|
Attachment #8893466 -
Attachment is obsolete: true
Attachment #8893466 -
Flags: review?(michael.l.comella)
Reporter | ||
Comment 25•7 years ago
|
||
..... and gone!
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8886560 [details]
Bug 1380639 - Use Android SDK that includes constraint-layout.
https://reviewboard.mozilla.org/r/157382/#review170102
I'll trust these hashes point in the right places. :P
Attachment #8886560 -
Flags: review?(michael.l.comella) → review+
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8889798 [details]
Bug 1380639 - Add contraint-layout and constraint-layer-solver dependencies.
https://reviewboard.mozilla.org/r/160868/#review170104
Build is not my specialty but this looks reasonable to me.
Attachment #8889798 -
Flags: review?(michael.l.comella) → review+
Comment 28•7 years ago
|
||
Pushed by s.kaspari@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ebb30b6d6184
Use Android SDK that includes constraint-layout. r=mcomella
https://hg.mozilla.org/integration/autoland/rev/514cf1f55c6f
Add contraint-layout and constraint-layer-solver dependencies. r=mcomella
Comment 29•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ebb30b6d6184
https://hg.mozilla.org/mozilla-central/rev/514cf1f55c6f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 30•7 years ago
|
||
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)
[1:21pm]
Status: RESOLVED → REOPENED
status-firefox57:
fixed → ---
Flags: needinfo?(s.kaspari)
Resolution: FIXED → ---
Reporter | ||
Comment 31•7 years ago
|
||
Will try again after we have a solution for bug 1388023.
Assignee: s.kaspari → nobody
Flags: needinfo?(s.kaspari)
Comment 32•7 years ago
|
||
Backout by cbook@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/3dc76a99eed0
Backed out changeset 514cf1f55c6f
https://hg.mozilla.org/mozilla-central/rev/47248637eafa
Backed out changeset ebb30b6d6184 on request from sebastian
Updated•7 years ago
|
Target Milestone: Firefox 57 → ---
Comment 33•7 years ago
|
||
https://hg.mozilla.org/projects/date/rev/ebb30b6d61849bcbd85d505c1e367528c6fb00f6
Bug 1380639 - Use Android SDK that includes constraint-layout. r=mcomella
https://hg.mozilla.org/projects/date/rev/514cf1f55c6fa80deb7490d80a59313577d625cc
Bug 1380639 - Add contraint-layout and constraint-layer-solver dependencies. r=mcomella
https://hg.mozilla.org/projects/date/rev/3dc76a99eed057365dc1fd73802d2a5d7576c5a8
Backed out changeset 514cf1f55c6f (bug 1380639)
https://hg.mozilla.org/projects/date/rev/47248637eafa9a38dade8dc3aa6c4736177c8d8d
Backed out changeset ebb30b6d6184 (bug 1380639) on request from sebastian
Updated•7 years ago
|
Iteration: 1.27 → 1.28
Updated•7 years ago
|
Iteration: 1.28 → ---
Priority: P1 → P3
Updated•7 years ago
|
tracking-fennec: --- → ?
Whiteboard: [MobileAS] 1.26 1.27 → [MobileAS]
Updated•7 years ago
|
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
Reporter | ||
Comment 36•7 years ago
|
||
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.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•