1395363 - Switch to webextension-langpacks

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

This is meta bug for the decision to switch to the new language packs.

We're almost done with the coding part, but I'll need help identifying what else has to happen for us to be able to switch and who should be the stakeholders in the decision.

The new language packs are:

1) Based on WebExtensions allowing us to remove langpacks from requiring the old addons ecosystem
2) Safer (no direct access to chrome registry)
3) Enable L10nRegistry (bug 1333980) which is required for transitioning to the new l10n API (bug 1365426).

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Pike, Kris, what do you think is needed here?

Comment 2

[Kris Maglione [:kmag]]

This will probably need some work on the AMO side for validating and hosting the new langpacks. And once we've started shipping them, we'll want to turn off support for legacy langpacks as soon as possible.

Aside from that, I don't have any particular concerns, as long as the new langpacks are working and QAed.

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

I filed this issue to track the AMO side: https://github.com/mozilla/addons-server/issues/6297. We need input from someone working on this so we can start looking into it and not cause delays.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

I just landed the consumption patch. Assuming it'll stick, we do not have more blockers on the gecko side.

Can you help us evaluate how much work and time you'd need to get the AMO side operational?

What would have to happen to switch users of the current langpacks to the new system? Some sort of preserving the langpack ID?

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

It doesn't look like a lot of work. I'll update the issues so the dev team can start looking at them. As for timing, I think we can have this done by the release of 57 in November, but we're also juggling other WebExtensions transition tasks, so I can't guarantee it now.

> What would have to happen to switch users of the current langpacks to the new system? Some sort of preserving the langpack ID?

Yes, the IDs should be the same in the manifest in order to avoid any update issues. Like Andreas mentioned on the Github issue, this would be a good opportunity to fully automate the process so he doesn't have to kick-start the update process every release cycle.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Just FYI, I confirmed that the ID is the same.

Old langpack install.rdf has:

  <Description about="urn:mozilla:install-manifest"
               em:id="[e-mail]"
               em:name="Polski Language Pack"
               em:version="55.0"
               em:type="8"
               em:creator="Aviary.pl">



New langpack manifest.json has:

  "applications": {
    "gecko": {
      "strict_min_version": "57.0a1", 
      "id": "[e-mail]", 
      "strict_max_version": "57.0a1"
    }
  }, 
  "langpack_id": "pl", 
  "version": "57.0a1", 
  "name": "Polski Language Pack", 


I hope that's enough to make the transition happen fluently from old to new langpacks for the current users.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1395363 - Switch to webextension-langpacks.

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

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.

https://reviewboard.mozilla.org/r/187416/#review192690

Found a couple of left-overs, and general comments to make the code easier to understand after this patch lands.

Looking at the generic toolkit stuff, I also found bug 1406904.

::: toolkit/locales/l10n.mk:168
(Diff revision 1)
>  	if test -f '$(DIST)/l10n-stage/$(PACKAGE).asc'; then mv -f '$(DIST)/l10n-stage/$(PACKAGE).asc' '$(ZIP_OUT).asc'; fi
>  
>  repackage-zip-%: unpack
>  	@$(MAKE) repackage-zip AB_CD=$* ZIP_IN='$(ZIP_IN)'
>  
> -APP_DEFINES = $(firstword $(wildcard $(LOCALE_SRCDIR)/defines.inc) \
> +APP_DEFINES = $(TK_DEFINES) $(firstword $(wildcard $(LOCALE_SRCDIR)/defines.inc) \

Make this LANGPACK_DEFINES? That way, in the future, folks don't have to search the full tree for traces of uses of this.

Also, let's use $(call MERGE_FILE) for the local and toolkit defines.inc resp. The firstword logic here is an over-optimization (that we might get rid of soon anyway) of l10n-merge not being able to handle .inc files.

::: toolkit/locales/l10n.mk:170
(Diff revision 1)
> -APP_DEFINES = $(firstword $(wildcard $(LOCALE_SRCDIR)/defines.inc) \
> +APP_DEFINES = $(TK_DEFINES) $(firstword $(wildcard $(LOCALE_SRCDIR)/defines.inc) \
> -                          $(srcdir)/en-US/defines.inc)
> -
> -NEW_APP_DEFINES = $(TK_DEFINES) $(firstword $(wildcard $(LOCALE_SRCDIR)/defines.inc) \
>                            $(srcdir)/en-US/defines.inc)
>  TK_DEFINES = $(firstword \

TK_DEFINES should be removed

::: toolkit/locales/l10n.mk
(Diff revision 1)
>  langpack-%: IS_LANGPACK=1
>  langpack-%: libs-%
>  	@echo 'Making langpack $(LANGPACK_FILE)'
>  	$(NSINSTALL) -D $(DIST)/$(PKG_LANGPACK_PATH)
> -	$(call py_action,preprocessor,$(DEFINES) $(ACDEFINES) \
> +	$(call py_action,langpack_manifest,--locales $(AB_CD) --min-app-ver $(MOZ_APP_VERSION) --max-app-ver $(MOZ_APP_MAXVERSION) --app-name "$(MOZ_APP_DISPLAYNAME)" --l10n-basedir "$(L10NBASEDIR)" --defines $(APP_DEFINES) --input $(DIST)/xpi-stage/locale-$(AB_CD))
> -	  -DTK_DEFINES=$(TK_DEFINES) -DAPP_DEFINES=$(APP_DEFINES) $(MOZILLA_DIR)/toolkit/locales/generic/install.rdf -o $(DIST)/xpi-stage/$(XPI_NAME)/install.rdf)

Now that we've dropped using toolkit/locales/generic/install.rdf, please also remove the file.

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.

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

Comment 10

[krupa raj [:krupa--use this to needinfo]]

This is on my radar but I'm leaving the NI as is for now. I will close it when the testing is complete.

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Axel,

I ran a try run which came "green": https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbab41ba6423&selectedJob=135811829

I also manually built a langpack and a repackage and both look good.

Comment 12

[Axel Hecht [:Pike]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbab41ba6423&filter-searchStr=l10n shows that there's not a single l10n job run.

Also, reminder to look at https://groups.google.com/d/msg/mozilla.dev.platform/nuNmunFsb0E/tZ1GZ5XpAAAJ for fair usage of try.

Please let me know when there are actual l10n jobs on try so that we actually have data to inspect. One locale should be finnish, then one other. Finnish is the only locale building right now that actually has an empty MOZ_LANGPACK_CONTRIBUTORS, so a good test.

Comment 13

[Zibi Braniecki [:zbraniecki][:gandalf]]

I added `de` and `fi` repacks and they completed green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbab41ba6423&filter-searchStr=l10n

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.

https://reviewboard.mozilla.org/r/187416/#review193284

Comment 15

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 4538cc66e877 -d 2ab5ef946a40: rebasing 426090:4538cc66e877 "Bug 1395363 - Switch to webextension-langpacks. r=Pike" (tip)
merging toolkit/locales/l10n.mk
warning: conflicts while merging toolkit/locales/l10n.mk! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.

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

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec713e74ec14
Switch to webextension-langpacks. r=Pike

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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