Closed
Bug 1395363
Opened 7 years ago
Closed 7 years ago
Switch to webextension-langpacks
Categories
(Core :: Internationalization, enhancement)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
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).
Assignee | ||
Comment 1•7 years ago
|
||
Pike, Kris, what do you think is needed here?
Flags: needinfo?(l10n)
Flags: needinfo?(kmaglione+bmo)
Comment 2•7 years ago
|
||
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)
Comment 3•7 years ago
|
||
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)
Updated•7 years ago
|
Flags: needinfo?(l10n)
Assignee | ||
Comment 4•7 years ago
|
||
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)
Comment 5•7 years ago
|
||
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)
Assignee | ||
Comment 6•7 years ago
|
||
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.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Comment 8•7 years ago
|
||
mozreview-review |
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-
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
This is on my radar but I'm leaving the NI as is for now. I will close it when the testing is complete.
Assignee | ||
Comment 11•7 years ago
|
||
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•7 years ago
|
||
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)
Assignee | ||
Comment 13•7 years ago
|
||
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)
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8916167 [details]
Bug 1395363 - Switch to webextension-langpacks.
https://reviewboard.mozilla.org/r/187416/#review193284
Attachment #8916167 -
Flags: review?(l10n) → review+
Comment 15•7 years ago
|
||
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 hidden (mozreview-request) |
Comment 17•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec713e74ec14
Switch to webextension-langpacks. r=Pike
Comment 18•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•7 years ago
|
Flags: needinfo?(krupa.mozbugs)
You need to log in
before you can comment on or make changes to this bug.
Description
•