1455100 - Make devedition its own addon id

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Justin Wood (:Callek)]

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.

Comment 1

[Justin Wood (:Callek)]

Attachment: Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

Review commit: https://reviewboard.mozilla.org/r/237740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237740/

Comment 2

[Justin Wood (:Callek)]

This target is for Gecko 61, not 60, and has been tested on https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6f9441c633cbd3427fe903ee912dbe7b7c1da19

Comment 3

[Nick Alexander :nalexander [he/him]]

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?

Comment 4

[Axel Hecht [:Pike]]

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.

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 8969052 [details]
Bug 1455100 - Make devedition its own language pack addon id, by using MOZ_LANGPACK_EID again.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/237740/diff/1-2/

Comment 6

[Justin Wood (:Callek)]

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

Comment 7

[Nick Alexander :nalexander [he/him]]

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.

Comment 8

[Axel Hecht [:Pike]]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/a603b7b4637f

Comment 11

[Mike Hommey [:glandium]]

I think this would be good to have on esr60. At least, I need this patch downstream. What do you think?

Comment 12

[Justin Wood (:Callek)]

Sure, feel free to land it with a=Callek (on behalf of releng)

Comment 13

[Mike Hommey [:glandium]]

Can you fill an approval‑mozilla‑esr60 request?

Comment 14

[Justin Wood (: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

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/164918e56cb9