Closed Bug 1455100 Opened 2 years ago Closed 2 years ago

Make devedition its own addon id

Categories

(Firefox Build System :: General, enhancement)

3 Branch
enhancement
Not set

Tracking

(firefox-esr60 fixed, firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr60 --- fixed
firefox61 --- fixed

People

(Reporter: Callek, Assigned: Callek)

References

Details

Attachments

(1 file)

To support signing of language packs via AMO for devedition, due to branding and other differences in products we need deved to be its own addon id.
This target is for Gecko 61, not 60, and has been tested on https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6f9441c633cbd3427fe903ee912dbe7b7c1da19
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

https://reviewboard.mozilla.org/r/237740/#review243512

This looks fine by me.  I don't know the policy repurcussions of splitting the langpack IDs, though: what happens if deved loads a mainline langpack?  And vice versa?

::: python/mozbuild/mozbuild/action/langpack_manifest.py:393
(Diff revision 1)
>          defines['MOZ_LANGPACK_CONTRIBUTORS'] if 'MOZ_LANGPACK_CONTRIBUTORS' in defines else ""
>      )
>  
> +    langpack_id = 'langpack-{0}@firefox.mozilla.org'.format(main_locale)
> +    if buildconfig.substs['MOZ_DEV_EDITION'] == '1':
> +        # Devedition gets a different id to allow separate AMO listings

nit: full sentence (trailing period).  Also, is "Devedition" the most grep-friendly string?  It does appear frequently in the tree, so maybe that's yes?
Attachment #8969052 - Flags: review?(nalexander) → review+
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

https://reviewboard.mozilla.org/r/237740/#review243698

Actually, this should use MOZ_LANGPACK_EID from browser/locales/Makefile.in, and probably ifdef there. The hard-coded id is a regression, as you can see if you play around with the language packs from comm-central's Thunderbird :-/ And I think this is the right opportunity to fix that regression instead of piling up.

We also need ways for devedition langpacks to not be compatible with mainline and vice versa, and against the ones we produce for Thunderbird, and so forth.
Attachment #8969052 - Flags: review?(l10n) → review-
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

The patch has changed substantially since the reviewed copy.

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1d8b0f24cbfa2c252eb5f81381daab0b61e2600
Attachment #8969052 - Flags: review+ → review?(nalexander)
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

https://reviewboard.mozilla.org/r/237740/#review243946

Sure, this is a reasonable way to achieve this as well.
Attachment #8969052 - Flags: review?(nalexander) → review+
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

https://reviewboard.mozilla.org/r/237740/#review244246

Yep, thanks, r=me.
Attachment #8969052 - Flags: review?(l10n) → review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a603b7b4637f
Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again. r=nalexander,Pike
https://hg.mozilla.org/mozilla-central/rev/a603b7b4637f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
I think this would be good to have on esr60. At least, I need this patch downstream. What do you think?
Flags: needinfo?(bugspam.Callek)
Sure, feel free to land it with a=Callek (on behalf of releng)
Flags: needinfo?(bugspam.Callek) → needinfo?(mh+mozilla)
Can you fill an approval‑mozilla‑esr60 request?
Flags: needinfo?(mh+mozilla) → needinfo?(bugspam.Callek)
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Downstream Linux Distro's unable to have different addon id for devedition/thunderbird language packs based on esr60
Fix Landed on Version: 61
Risk to taking this patch (and alternatives if risky): Low risk, patch has vetted on other branches for a while
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

:Ryan, if this approves can you please take care of landing for me
Flags: needinfo?(bugspam.Callek) → needinfo?(ryanvm)
Attachment #8969052 - Flags: approval-mozilla-esr60?
Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

Approved for ESR 60.1
Flags: needinfo?(ryanvm)
Attachment #8969052 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.