1621074 - fix l10n in Flatpaks

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, task)

Description

[Mihai Tabara [:mtabara]⌚️GMT]

Currently we bake all the langpacks as part of the Firefox Flatpak (kind of like a multi-locale APK like we do in the mobile world). I remember Rob mentioning that we should build them separately so that a user requesting (e.g.) en-GB only downloads that specific Flatpak, and not the entire bunch.

I can't remember exactly the details here but @nedrichards or @ramq can help with advice as to how we should proceed going further.

Comment 1

[Mihai Tabara [:mtabara]⌚️GMT]

Dropping here some context as @nedricharsds and @barthalion have been really kind to help.

@barthalion: I assume the idea was to put actual files to /app/share/locale/$LANG_CODE and symlink that directory to where Firefox looks them up – this way only subset configured on user system is downloaded vs all locales
but I will wait for Nick as I don't iknow the entire context

@nedrichards: this is exactly correct. we want them to be baked in the correct directories then flatpak will automatically slice them up and deliver the user what they expect on their system by default (a user can always override and get everything if they want)
as an aside, I had a read of the phoronix comments thread about this (I know, I know, never read the comments). Intermittently interesting amidst the FUD and flames ;-) https://www.phoronix.com/forums/forum/phoronix/latest-phoronix-articles/1164494-mozilla-making-progress-with-offering-firefox-as-a-flatpak-on-linux

Comment 2

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: l10n_fix.diff

Comment 3

[Mihai Tabara [:mtabara]⌚️GMT]

@barthalion provided the above patch, I'm dropping it here for context! ^

Comment 4

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 9132239 [details] [diff] [review]
l10n_fix.diff

>+for locale in $locales; do

or "${locales[@]}" better here?

Comment 5

[Emerson Bernier]

@mtabara can dictionaries be fixed similarly by creating symlink from ff dict path to /usr/share/hunspell where runtime dictionaries exist?

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Bug 1621074 - fix l10n in Flatpaks. r=rail DONTBUILD

Comment 7

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Emerson Bernier from comment #5)

@mtabara can dictionaries be fixed similarly by creating symlink from ff dict path to /usr/share/hunspell where runtime dictionaries exist?

Hm, sure. Can you though give me please an example?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7437f73e00e7
fix l10n in Flatpaks. r=rail DONTBUILD

Comment 9

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 9132361 [details]
Bug 1621074 - fix l10n in Flatpaks. r=rail DONTBUILD

Beta/Release Uplift Approval Request

• User impact if declined: None, this fixes configuration under the hood in building flatpaks
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: 1591387
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): RelEng automation change
• String changes made/needed:

Comment 10

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7437f73e00e7

Comment 11

[Mihai Tabara [:mtabara]⌚️GMT]

Latest staging release pushed to beta channel so latest Flatpak on Flathub should have this fix.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 9132361 [details]
Bug 1621074 - fix l10n in Flatpaks. r=rail DONTBUILD

approved for 75.0b3

Comment 13

[Axel Hecht [:Pike]]

Does this work for locales with region codes? I.e., en-GB? Asking as I wouldn't be surprised if /app/share/locale/ was expecting POSIX locale codes and not BCP-47 ones.

Comment 14

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Axel Hecht [:Pike][back 03/16] from comment #13)

Does this work for locales with region codes? I.e., en-GB? Asking as I wouldn't be surprised if /app/share/locale/ was expecting POSIX locale codes and not BCP-47 ones.

That's a great question, I have no idea. Let me ask around and get back to you here.
Leaving a NI: for me here.

Comment 15

[Emerson Bernier]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #7)

(In reply to Emerson Bernier from comment #5)

@mtabara can dictionaries be fixed similarly by creating symlink from ff dict path to /usr/share/hunspell where runtime dictionaries exist?

Hm, sure. Can you though give me please an example?

This was a question rather than proposal. I don't know if it can work. Maybe flathub folks will knew?

Comment 16

[Emerson Bernier]

@mtabara locale symlinks seems broken. They point to non-existent paths and have random languages avalaible :

[📦 org.mozilla.firefox extensions]$ ls -al
total 28
drwxr-xr-x 2 nfsnobody nfsnobody 4096 Jan  1  1970 .
drwxr-xr-x 3 nfsnobody nfsnobody 4096 Jan  1  1970 ..
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ar/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   70 Mar 11 15:14 [e-mail] -> build/files/share/locales/en-CA/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/he/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/it/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ja/[e-mail]

Comment 17

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Emerson Bernier from comment #16)

@mtabara locale symlinks seems broken. They point to non-existent paths and have random languages avalaible :

[📦 org.mozilla.firefox extensions]$ ls -al
total 28
drwxr-xr-x 2 nfsnobody nfsnobody 4096 Jan  1  1970 .
drwxr-xr-x 3 nfsnobody nfsnobody 4096 Jan  1  1970 ..
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ar/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   70 Mar 11 15:14 [e-mail] -> build/files/share/locales/en-CA/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/he/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/it/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ja/[e-mail]

The "random" locales are expected. We haven't started publishing from mozilla-beta just yet, the patches haven't been uplifted (but I'm confident they will for this Friday's 75.0b3) so the flatpak that is currently in Flathub is generated from a try staging release which trims the number of locales to a limited set (because 100 locales x 6 platforms adds up to a lot of compute time).

With respect to non-existent paths, hm, let me investigate that, might be what @barthalion suggested that we should've used s/locales/locale when baking them in app/share.

Comment 18

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Emerson Bernier from comment #15)

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #7)

(In reply to Emerson Bernier from comment #5)

@mtabara can dictionaries be fixed similarly by creating symlink from ff dict path to /usr/share/hunspell where runtime dictionaries exist?

Hm, sure. Can you though give me please an example?

This was a question rather than proposal. I don't know if it can work. Maybe flathub folks will knew?

Flathub folks confirmed that if Firefox doesn't look at /usr/share/hunspell automatically, then yes.

Comment 19

[Mihai Tabara [:mtabara]⌚️GMT]

Adding :leave-open as more work is needed here, aside from the existing patch.

Comment 20

[Mihai Tabara [:mtabara]⌚️GMT]

via @barthalion and @alarsson

1. just notices it should use locale, not locales for directory name
2. indeed we may need to replace dash with underscore in /app/share/locale/$locale

I also suspect we may need to add something like this to build/metadata to have the Locale extension created at all:

[Extension org.mozilla.firefox.Locale]
directory=share/runtime/locale
autodelete=true
locale-subset=true

as it is now, no Locale extension is created and just all locales from l10n_changesets.json is shipped.

Comment 21

[Alexander Larsson]

Lemme explain how locale extensions work for flatpak, taking an app like gedit as an example.
The gedit metadata file contains this definition of the .Locale extenstion

[Extension org.gnome.gedit.Locale]
directory=share/runtime/locale
autodelete=true
locale-subset=true

This means, in order of the above lines:

• If an extension is installed with the name "org.gnome.gedit.Locale", matching the branchname of the gedit app, then the contents (i.e. files) of said extension will be mounted in /app/share/runtime/locale.
• Such extensions will be auto-installed with the app if they exist in the same repository.
• It will be automatically deleted when the app is deleted.
• When installing we don't download the entire thing, only the parts where the toplevel directory matches the first element of one of the configured locales to use.

Inside the extension we can really put whatever data we want, but for a typical gettext using system it would look something like:

• sv/lib/sv_FI/... swedish-finnish locale files here...
• sv/lib/sv_SE/... swedish locale files here...
• sv/share/sv/LD_MESSAGEs/gedit.mo

This is a bit weird, and not how gettext finds things. However, it is done this way so that all the files for one language can be in one directory for easy filtering at download time.

To make this work, the actual app contains symlinks into the extension.

• /app/share/locale/sv -> ../../share/runtime/locale/sv/share/sv
• /app/lib/locale/sv_SE -> ../../share/runtime/locale/sv/lib/sv_SE
• /app/lib/locale/sv_FI -> ../../share/runtime/locale/sv/lib/sv_FI

This way gettext will find these files if the right extension directory is available and get broken symlinks if not.

You don't have to use the exact same file layout for your extension, the important thing is to realize that the first part of the locale (i.e the "sv" part of "sv_SE") will be used to filter what is downloaded of the extension, and if your locale layout is such that you need to spread files in more than one directory you can use symlinks to work around this.

The "first part of local" limitation means you will have to ship e.g. the en_CA and en_GB langpacks in the same directory and all your english users will download both. It was done this way to limit the complexity of the filtering system.

Comment 22

[Alexander Larsson]

Also, I have not followed exactly how you're building this. But by default the regular flatpak commands don't automatically create a locale extension. That is done on a higher level by flatpak-builder. So, if you're not using that, or if you're not following the layout above you need to manually create the Locale extension.

Flatpak-builder does this by creating during build the full hierarchy, but then it passed --exclude=/share/runtime/locale/*/* to flatpak build-export when building the main app, and then it runs an extra build-export for the locale with --metadata=metatdata.locale --files=files/share/runtime/locale , having created a custom metadata.locale file for the locale extension which looks like:

[Runtime]
name=org.gnome.gedit.Locale

[ExtensionOf]
ref=app/org.gnome.gedit/x86_64/stable

Comment 23

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Alexander Larsson from comment #22)

Also, I have not followed exactly how you're building this. But by default the regular flatpak commands don't automatically create a locale extension. That is done on a higher level by flatpak-builder. So, if you're not using that, or if you're not following the layout above you need to manually create the Locale extension.

Flatpak-builder does this by creating during build the full hierarchy, but then it passed --exclude=/share/runtime/locale/*/* to flatpak build-export when building the main app, and then it runs an extra build-export for the locale with --metadata=metatdata.locale --files=files/share/runtime/locale , having created a custom metadata.locale file for the locale extension which looks like:

[Runtime]
name=org.gnome.gedit.Locale

[ExtensionOf]
ref=app/org.gnome.gedit/x86_64/stable

Thanks for all your elaborated input, this is great. To be honest, this goes beyond my knowledge so it's good to try to understand some of the internal mechanics.

As far as I know, we intentionally skipped using the flatpak-builder because it required all the privileges that were incompatible with our CI system. Instead, we're creating that build/metadata file manually along with other things that normally, flatpak-builder would've taken care of - see https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/docker/firefox-flatpak/runme.sh#l69 for more details.

Comment 24

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f164f7c201c4

Comment 25

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Bug 1621074 - fix Flatpak l10n properly. r=jcristau

Comment 26

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 9132964 [details]
Bug 1621074 - fix Flatpak l10n properly. r=jcristau

Beta/Release Uplift Approval Request

• User impact if declined: No user impact, fixes Flatpak l10n story for users consuming Firefox Flatpak beta.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 27

[Julien Cristau [:jcristau]]

Comment on attachment 9132964 [details]
Bug 1621074 - fix Flatpak l10n properly. r=jcristau

let's get this one uplifted as well.

Comment 28

[Mihai Tabara [:mtabara]⌚️GMT]

The patch that is soon landing on autoland + beta is fixing l10n, courtesy to @barthalion.
Am removing the leave-open tag for now, will let the sheriffs close this and reopen if there are still issues.

Comment 29

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Emerson Bernier from comment #16)

@mtabara locale symlinks seems broken. They point to non-existent paths and have random languages avalaible :

[📦 org.mozilla.firefox extensions]$ ls -al
total 28
drwxr-xr-x 2 nfsnobody nfsnobody 4096 Jan  1  1970 .
drwxr-xr-x 3 nfsnobody nfsnobody 4096 Jan  1  1970 ..
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ar/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   70 Mar 11 15:14 [e-mail] -> build/files/share/locales/en-CA/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/he/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/it/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ja/[e-mail]

Last flatpak from Flathub should fix this, can you please confirm whenever you have a minute?

Comment 30

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

https://hg.mozilla.org/releases/mozilla-beta/rev/602d9f467457

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb35fa037fb4
fix Flatpak l10n properly. r=jcristau

Comment 32

[Daniel Varga [:dvarga]]

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

Comment 33

[Emerson Bernier]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #29)

(In reply to Emerson Bernier from comment #16)

@mtabara locale symlinks seems broken. They point to non-existent paths and have random languages avalaible :

[📦 org.mozilla.firefox extensions]$ ls -al
total 28
drwxr-xr-x 2 nfsnobody nfsnobody 4096 Jan  1  1970 .
drwxr-xr-x 3 nfsnobody nfsnobody 4096 Jan  1  1970 ..
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ar/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   70 Mar 11 15:14 [e-mail] -> build/files/share/locales/en-CA/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/he/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/it/[e-mail]
lrwxrwxrwx 1 nfsnobody nfsnobody   64 Mar 11 15:14 [e-mail] -> build/files/share/locales/ja/[e-mail]

Last flatpak from Flathub should fix this, can you please confirm whenever you have a minute?

Unfortunately it's not fixed. There are symlinks for all locales in "/app/lib/firefox/distribution/extensions/" now but they point to non-existing paths like "${appdir}/share/runtime" which translates to "build/files/share/runtime/". Keep in mind that this path must exist inside runtime sandbox, not during build.

Also I can't find real langpack file anywhere in sandbox while I have locale extension (en) installed.

Comment 34

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Bug 1621074 - flatpak symlinks l10n fix and other improvements. r=jcristau

Bug 1621074 - install Flatpak policy properly..

Bug 1621074 - use dictionaries provided-by-runtime

Bug 1621074 - disable defaultbrowsercheck as being useless on flatpaks

Comment 35

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 9133177 [details]
Bug 1621074 - flatpak symlinks l10n fix and other improvements. r=jcristau

Beta/Release Uplift Approval Request

• User impact if declined: More fixes in the way we build locales on Flatpaks
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 36

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Emerson Bernier from comment #5)

@mtabara can dictionaries be fixed similarly by creating symlink from ff dict path to /usr/share/hunspell where runtime dictionaries exist?

There's now a pref("spellchecker.dictionary_path", "/usr/share/hunspell"); within the default-preferences.js baked in the flatpak, hopefully that will help.

Also, the l10n should be fixed now courtesy to @barthalion's patches. The flatpak is updated so please let me know if this works this time.

Comment 37

[Emerson Bernier]

All reported things looks ok right now, thx for fixing.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28379b25e441
flatpak symlinks l10n fix and other improvements. r=jcristau

Comment 39

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/28379b25e441

Comment 40

[Axel Hecht [:Pike]]

Can you check tr and trs? Also pt-BR and pt-PT and zh-CN vs zh-TW?

Comment 41

[Julien Cristau [:jcristau]]

Comment on attachment 9133177 [details]
Bug 1621074 - flatpak symlinks l10n fix and other improvements. r=jcristau

ok let's try this again for 75.0b5.

At some point it'd be better to have one patch per issue, and followup bugs for things that still don't work.

Comment 42

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/b0462a387ec5

Comment 43

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Julien Cristau [:jcristau] from comment #41)

Comment on attachment 9133177 [details]
Bug 1621074 - flatpak symlinks l10n fix and other improvements. r=jcristau

ok let's try this again for 75.0b5.

At some point it'd be better to have one patch per issue, and followup bugs for things that still don't work.

Sounds good, thanks for uplifting the patch. 75.0b5 should look good with that change.
I'll track separate bugs for l10n from now on, this bug is indeed getting clogged.

Comment 44

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Axel Hecht [:Pike][back 03/16] from comment #40)

Can you check tr and trs? Also pt-BR and pt-PT and zh-CN vs zh-TW?

@barthalion helped me out with double-checking these:

[📦 org.mozilla.firefox runtime]$ cd langpack/
[📦 org.mozilla.firefox langpack]$ ls
ac  as	bn  cs	ds  es	ff  ga	gu  hs	id  ka	ko  mk	nb  oc	rm  sk	sr  th	ur  zh
af  az	br  cy	el  et	fi  gd	he  hu	is  kk	li  mr	ne  pa	ro  sl	sv  tl	uz
an  be	bs  da	en  eu	fr  gl	hi  hy	it  km	lt  ms	nl  pl	ru  so	ta  tr	vi
ar  bg	ca  de	eo  fa	fy  gn	hr  ia	ja  kn	lv  my	nn  pt	si  sq	te  uk	xh
[📦 org.mozilla.firefox langpack]$ ls zh/
[e-mail]	[e-mail]
[📦 org.mozilla.firefox langpack]$ ls tr/
[e-mail]  [e-mail]

Same for pt related ones. The trs one depends on what language user has configured on their system.