Closed
Bug 1387207
Opened 8 years ago
Closed 8 years ago
Permared Android build bustage on Gecko 56 since the merge to Beta
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Tracking
(firefox56+ fixed, firefox57 fixed)
RESOLVED
FIXED
mozilla57
People
(Reporter: RyanVM, Assigned: kikuo)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
nalexander
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
[Tracking Requested - why for this release]: Busted builds, closed Beta tree.
+++ This bug was initially created as a clone of Bug #1368925 +++
Same as the last cycle...
https://treeherder.mozilla.org/logviewer.html#?job_id=120785385&repo=mozilla-beta
Flags: needinfo?(kikuo)
Comment 1•8 years ago
|
||
We're working on a more permanent fix in bug 1378410, but IMO if the workaround from the last cycle still applies, we can go with that for 56.
Reporter | ||
Comment 2•8 years ago
|
||
It'll require a revised patch if we want to go that route. The previous changeset has conflicts cross the board for 56.
Assignee | ||
Comment 3•8 years ago
|
||
Weird, we're waiting for Bug 1365505 to be uplifted, so that the source code of Exoplayer could be included while building beta. So in this case, it's still in nightly-only env [1].
But, even MOZ_ANDROID_HLS_SUPPORT is not set yet, those busted files [2] should not be included in build coverage and it is green in "B" stage.
I'm not familiar with how "Deps" check works.
My speculation is that those busted files such as "GeckoHlsPlayer.java" are still included during "Deps" check because they are not explicitly excluded in gradle build system [3].
I need to exclude these files specifically in build.gradle too.
Nick, is that right ?
[1] https://hg.mozilla.org/releases/mozilla-beta/file/c78102e7a5c2/mobile/android/moz.configure#l84
[2] http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/mobile/android/base/moz.build#466-473
[3] http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/mobile/android/geckoview/build.gradle#77
Flags: needinfo?(kikuo) → needinfo?(nalexander)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8893681 [details]
Bug 1387207 - Exclude code of GeckoHLS-related components if ExoPlayer source code is not included.
To fix the permared Android gradle build bustage, this patch should be uplifted to 56 Beta.
Flags: needinfo?(nalexander)
Assignee | ||
Comment 6•8 years ago
|
||
Try run on 56 beta, "deps" / "Bg" are green now !
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eebe1543ea83cb3394765f9c5641ddbf8599264f
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e45b1fa11c1089cd587fa96b7469be1f9dd71266
Reporter | ||
Comment 7•8 years ago
|
||
Looks like bug 1365505 took care of the immediate issue on Beta. Not sure if you still want to land the patch in this bug in light of that or not.
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8893681 [details]
Bug 1387207 - Exclude code of GeckoHLS-related components if ExoPlayer source code is not included.
https://reviewboard.mozilla.org/r/164798/#review170836
::: mobile/android/geckoview/build.gradle:78
(Diff revision 1)
> main {
> java {
> srcDir "${topsrcdir}/mobile/android/geckoview/src/thirdparty/java"
>
> if (!mozconfig.substs.MOZ_ANDROID_HLS_SUPPORT) {
> - exclude 'com/google/android/exoplayer2/**'
> + exclude "com/google/android/exoplayer2/**"
In Gradle, double quotes enable substitutions (like `${var}`) while single quotes do not. Therefore, prefer single quotes if you are _not_ substituting, like here.
Attachment #8893681 -
Flags: review?(nalexander) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8893681 [details]
Bug 1387207 - Exclude code of GeckoHLS-related components if ExoPlayer source code is not included.
https://reviewboard.mozilla.org/r/164798/#review170836
> In Gradle, double quotes enable substitutions (like `${var}`) while single quotes do not. Therefore, prefer single quotes if you are _not_ substituting, like here.
Thanks for the reivew.
Comment 11•8 years ago
|
||
Pushed by kikuo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/87b6d99a490b
Exclude code of GeckoHLS-related components if ExoPlayer source code is not included. r=nalexander
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8893681 [details]
Bug 1387207 - Exclude code of GeckoHLS-related components if ExoPlayer source code is not included.
Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1341990, Bug 1368954.
[User impact if declined]:
It would not impact to users.
If declined, once we decide to turn off Fennec HLS in beta again (e.g. rollback Bug 1365505), we'll still get busted for 'deps' check on try.
Considering code correctness logically, if we want to exclude the source of exoplayer by the 'MOZ_ANDROID_HLS_SUPPORT', we should also exclude those GeckoHls-related java implementation.
[Is this code covered by automated tests?]:
Yes, it would be covered while building Fennec.
[Has the fix been verified in Nightly?]:
Yes, nightly try result is green.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c8cac65ac38
Try results on Beta.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd93b24f5e081f2545b8b266630447cca81d8703
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e83d9fd36a5fe71c86cc43637ebc615ba587a905
[Needs manual test from QE? If yes, steps to reproduce]:
No.
[List of other uplifts needed for the feature/fix]:
None
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
It actually fixed a mistake that we didn't exclude some related files which should not be included in beta when MOZ_ANDROID_HLS_SUPPORT is not defined in beta. It's a correction, and not risky.
[String changes made/needed]:
None
Attachment #8893681 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → kikuo
Comment 13•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•8 years ago
|
Comment 14•8 years ago
|
||
Comment on attachment 8893681 [details]
Bug 1387207 - Exclude code of GeckoHLS-related components if ExoPlayer source code is not included.
Fix for failing tests, let's uplift this for beta 2.
Attachment #8893681 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reporter | ||
Comment 15•8 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 57 → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•