Switch to webextension-langpacks

RESOLVED FIXED in Firefox 58

Status

()

enhancement
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: zbraniecki, Assigned: zbraniecki)

Tracking

(Depends on 1 bug)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

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).
Depends on: 1365709
Pike, Kris, what do you think is needed here?
Flags: needinfo?(l10n)
Flags: needinfo?(kmaglione+bmo)
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.
Flags: needinfo?(krupa.mozbugs)
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(jorge)
Depends on: 1395456
Depends on: 1395457
Depends on: 1395459
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.
Flags: needinfo?(jorge)
Depends on: 1396467
Flags: needinfo?(l10n)
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?
Flags: needinfo?(jorge)
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.
Flags: needinfo?(jorge)
Just FYI, I confirmed that the ID is the same.

Old langpack install.rdf has:

  <Description about="urn:mozilla:install-manifest"
               em:id="langpack-pl@firefox.mozilla.org"
               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": "langpack-pl@firefox.mozilla.org", 
      "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.
No longer blocks: 1365426
Depends on: 1405264
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
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.
Attachment #8916167 - Flags: review?(l10n) → review-
This is on my radar but I'm leaving the NI as is for now. I will close it when the testing is complete.
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.
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.
Flags: needinfo?(gandalf)
I added `de` and `fi` repacks and they completed green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbab41ba6423&filter-searchStr=l10n
Flags: needinfo?(gandalf)
Depends on: 1407395
Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.

https://reviewboard.mozilla.org/r/187416/#review193284
Attachment #8916167 - Flags: review?(l10n) → review+
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)
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec713e74ec14
Switch to webextension-langpacks. r=Pike
https://hg.mozilla.org/mozilla-central/rev/ec713e74ec14
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Depends on: 1411951
Depends on: 1415117
Flags: needinfo?(krupa.mozbugs)
You need to log in before you can comment on or make changes to this bug.