Merge automation should support new Android targets

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: rail, Assigned: rail)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

android-api-10
android-api-11
android-api-9-10-constrained
android-api-9-constrained

Probably should also fix the current mozconfigs.
This patch is for m-b
Attachment #8548228 - Flags: review?(mark.finkle)
Not sure if we need this (mozilla-release). Feel free to r- if we don't ship this in Firefox 35.
Attachment #8548231 - Flags: review?(mark.finkle)
For Aurora
Attachment #8548232 - Flags: review?(mark.finkle)
Comment on attachment 8548228 [details] [diff] [review]
m-b-fix_android_branding.diff

LGTM.

I'd like to verify that we can remove the android-api-9-constrained and android-api-10 folders. I don't think we need those anymore, but I need more feedback from JLund on that.
Attachment #8548228 - Flags: review?(mark.finkle) → review+
Comment on attachment 8548228 [details] [diff] [review]
m-b-fix_android_branding.diff

What about the "release" mozconfigs?
Comment on attachment 8548232 [details] [diff] [review]
m-a-fix_android_branding.diff

This patch is enough to fix bug 1120931, but I still want to check on the clean up of the other mozconfigs, and should we be updating "release" in m-a too?
Attachment #8548232 - Flags: review?(mark.finkle) → review+
Comment on attachment 8548228 [details] [diff] [review]
m-b-fix_android_branding.diff

Changin to r- for now. The split APK work is only on mozilla-aurora for now. We don't need to change mozilla-beta yet.
Attachment #8548228 - Flags: review+ → review-
Comment on attachment 8548231 [details] [diff] [review]
m-r-fix_android_branding.diff

Yeah, we don't need to touch mozilla-release yet either.
Attachment #8548231 - Flags: review?(mark.finkle) → review-
The only reason I can think of as to why mozilla-beta or mozilla-release would need to be touched would be for Try builds. We are only using split APKs for actual builds on mozilla-central and mozilla-aurora.
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Comment on attachment 8548228 [details] [diff] [review]
> m-b-fix_android_branding.diff
> 
> What about the "release" mozconfigs?

There was no "release" in those directories.

beta and release will be bumped automatically next time.

Mozconfig cleanup would be great!
Blocks: 1120931
Comment on attachment 8548232 [details] [diff] [review]
m-a-fix_android_branding.diff

Branding change not handled by merge day automation.

[Feature/regressing bug #]: split APK branding doesn't correspond to the channel, see bug 1120931
[User impact if declined]: None expected
[Describe test coverage new/current, TBPL]: What can possibly go wrong? :)
[Risks and why]: low risk
[String/UUID change made/needed]: N/A
Attachment #8548232 - Flags: approval-mozilla-aurora?
Blocks: 1121160
Comment on attachment 8548232 [details] [diff] [review]
m-a-fix_android_branding.diff

Thanks for the quick turnaround. Aurora+
Attachment #8548232 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Do we need bugs to not break beta and release?
(In reply to Kevin Brosnan [:kbrosnan] from comment #14)
> Do we need bugs to not break beta and release?

This bug (the final solution) will handle both beta and release.
(In reply to Rail Aliiev [:rail] from comment #10)
> (In reply to Mark Finkle (:mfinkle) from comment #5)
> > Comment on attachment 8548228 [details] [diff] [review]
> > m-b-fix_android_branding.diff
> > 
> > What about the "release" mozconfigs?
> 
> There was no "release" in those directories.

IIUC, the 'release' configs are used for beta and m-r. We will need to add apk-split support into our (google play) release builds before next merge. I'll be ensuring that work is in place.

> 
> beta and release will be bumped automatically next time.

great. thanks rail!

> 
> Mozconfig cleanup would be great!

It is safe to remove the android-api-9-constrained and android-api-10 folders now. I'll do that in via: Bug 1121599
Blocks: 1123369
Blocks: 1123367
This patch does work properly for central-to-aurora migration and doesn't work for aurora-to-beta (missing "release" mozconfig) and beta-to-release (missing mozconfigs).

I'd rather have these changes in place and hard fail instead of forgetting to uplift the changes. As a workaround I'm going to prepare temporary patches for next 1 or 2 uplifts.
Attachment #8551365 - Flags: review?(bhearsum)
(In reply to Rail Aliiev [:rail] from comment #17)
> Created attachment 8551365 [details] [diff] [review]
> merge_split_fennec-mozharness.diff
> 
> This patch does work properly for central-to-aurora migration and doesn't
> work for aurora-to-beta (missing "release" mozconfig) and beta-to-release
> (missing mozconfigs).

If this patch doesn't work, what's the point? Are we adding it now so we don't forget to next time?
It partially works. :)

I can change the configuration format and add something like ignore_failures=True or add check against gecko_version, but that would increase code complexity. :(

I'm on the hook for the next uplift (to handle the fallouts) and in case we fix the underlying issues (missing mozconfigs), the following uplift (apr-6) will work without any changes (with this patch applied).
Comment on attachment 8551365 [details] [diff] [review]
merge_split_fennec-mozharness.diff

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

OK, so we need it for the future, it just doesn't do everything for the next uplift. That' sfine with me if you're on the hook.
Attachment #8551365 - Flags: review?(bhearsum) → review+
Actually, I may need another (simplified) version of this patch when bug 1121599 is landed.
Depends on: 1121599
Comment on attachment 8551365 [details] [diff] [review]
merge_split_fennec-mozharness.diff

https://hg.mozilla.org/build/mozharness/rev/e85b34bfffee

I removed the platforms dropped in bug 1121599 and added some comments regarding beta-to-release. I'll attach a patch to bug 1121160 to handle that part.
Attachment #8551365 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
a mozharness patch has from this bug is in production
Blocks: 1151498
You need to log in before you can comment on or make changes to this bug.