Bug 212459 - [Flatpak SDK] Missing locale warnings
Summary: [Flatpak SDK] Missing locale warnings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alicia Boya García
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-28 03:23 PDT by Alicia Boya García
Modified: 2020-06-23 08:48 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.90 KB, patch)
2020-06-23 05:28 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff
Patch (2.72 KB, patch)
2020-06-23 08:24 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2020-05-28 03:23:27 PDT
Running any perl script under flatpak spams lots of locale warnings. We should have a locale set inside flatpak. Probably en_US.UTF-8 for consistency. 

$ build-webkit --gtk --debug
Building flatpak based environment
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_MONETARY = "es_ES.UTF-8",
	LC_PAPER = "es_ES.UTF-8",
	LC_MEASUREMENT = "es_ES.UTF-8",
	LC_TIME = "es_ES.UTF-8",
	LC_NUMERIC = "es_ES.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to a fallback locale ("en_US.UTF-8").
+  cmake --build /app/webkit/WebKitBuild/Debug --config Debug -- 
[1/377] Generate bindings (WebKitTestRunnerInjectedBundleBindings)
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_MONETARY = "es_ES.UTF-8",
	LC_PAPER = "es_ES.UTF-8",
	LC_MEASUREMENT = "es_ES.UTF-8",
	LC_TIME = "es_ES.UTF-8",
	LC_NUMERIC = "es_ES.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to a fallback locale ("en_US.UTF-8").
[...]

====================================================================
 WebKit is now built (00m:02s). 
 To run MiniBrowser with this newly-built code, use the
 "Tools/Scripts/run-minibrowser" script.
====================================================================
Comment 1 Philippe Normand 2020-05-29 05:26:23 PDT
LANG is set already, to "en_US.UTF-8". I don't see any Perl warning on the bots. 
Seems like you have "es_ES.UTF-8" configured in your shell profile, that seems to conflict with en_US... Can you try to set LANGUAGE from flatpakutils.py, look for en_EN and add the LANGUAGE variable in `sandbox_environment`.
Comment 2 Philippe Normand 2020-05-29 05:28:25 PDT
or set LC_ALL ? There's too many locale env vars. :)
Comment 3 Alicia Boya García 2020-06-12 02:56:52 PDT
The problem is not that the locale is not being set, the problem is the locale I'm using in my computer (es_ES.UTF-8) is not built inside the flatpak environment.

Setting these settings to en_US.UTF-8 in the shell avoids the warning.

I think it's reasonable that development versions of WebKit, especially the tests, run on the same locale for everybody, but we can probably agree telling people not to set their computer locale to something else would not be an appropriate solution.

What could we do?

Some helpful info: I found this doc explaining the different locale variables. https://help.ubuntu.com/community/Locale The priority goes as follows:

LC_ALL > LC_[specific locale aspect] > LANG

LANGUAGE is used by gettext, and in the case of translations using that library, it takes priority over the other variables including LC_ALL. https://www.gnu.org/software/gettext/manual/html_node/The-LANGUAGE-variable.html#The-LANGUAGE-variable
Comment 4 Philippe Normand 2020-06-12 04:21:25 PDT
(In reply to Alicia Boya García from comment #3)
> The problem is not that the locale is not being set, the problem is the
> locale I'm using in my computer (es_ES.UTF-8) is not built inside the
> flatpak environment.
> 
> Setting these settings to en_US.UTF-8 in the shell avoids the warning.
> 
> I think it's reasonable that development versions of WebKit, especially the
> tests, run on the same locale for everybody, but we can probably agree
> telling people not to set their computer locale to something else would not
> be an appropriate solution.
> 

For the record this isn't what I was suggesting either. We agree.

> What could we do?
> 

Currently LANG is forwarded from the host to the sandbox. Can you check if things work better if we don't do that?
Comment 5 Philippe Normand 2020-06-12 07:38:56 PDT
I could reproduce the bug and came up with this, might need more work, but can you check/test?

diff --git a/Tools/flatpak/webkit-bwrap b/Tools/flatpak/webkit-bwrap
index b342a1da7e29..970b671cad6a 100755
--- a/Tools/flatpak/webkit-bwrap
+++ b/Tools/flatpak/webkit-bwrap
@@ -58,6 +58,10 @@ def main(args: list) -> int:
     for dst, src in try_bind_mounts.items():
         bwrap_args.extend(["--bind-try", src, dst])
 
+    for env in os.environ.keys():
+        if env.startswith("LC_"):
+            bwrap_args.extend(["--unsetenv", env])
+
     command_line = ' '.join(shlex.quote(a) for a in bwrap_args + args)
     # os.system return code behaves like os.wait. A 16 bit number with the
     # signal in the lower byte and, if the signal is zero, the exit code in
Comment 6 Alicia Boya García 2020-06-21 02:53:07 PDT
The proposed change almost works! It's just missing a LANG and LANGUAGE, that should also be resetted. Otherwise you would get the warnings if you have any other locale set.

diff --git a/Tools/flatpak/webkit-bwrap b/Tools/flatpak/webkit-bwrap
index b342a1da7e29..a286dd1377a3 100755
--- a/Tools/flatpak/webkit-bwrap
+++ b/Tools/flatpak/webkit-bwrap
@@ -58,6 +58,10 @@ def main(args: list) -> int:
     for dst, src in try_bind_mounts.items():
         bwrap_args.extend(["--bind-try", src, dst])
 
+    for env in os.environ.keys():
+        if env.startswith("LC_") or env in {"LANG", "LANGUAGE"}:
+            bwrap_args.extend(["--unsetenv", env])
+
     command_line = ' '.join(shlex.quote(a) for a in bwrap_args + args)
     # os.system return code behaves like os.wait. A 16 bit number with the
     # signal in the lower byte and, if the signal is zero, the exit code in
Comment 7 Philippe Normand 2020-06-21 08:46:01 PDT
(In reply to Alicia Boya García from comment #6)
> The proposed change almost works! It's just missing a LANG and LANGUAGE,
> that should also be resetted. Otherwise you would get the warnings if you
> have any other locale set.
> 

The en_US locale needs to be set for the layout tests and jsc tests though. Otherwise some intl-related tests will fail.
Comment 8 Alicia Boya García 2020-06-21 12:00:42 PDT
(In reply to Philippe Normand from comment #7)
> (In reply to Alicia Boya García from comment #6)
> > The proposed change almost works! It's just missing a LANG and LANGUAGE,
> > that should also be resetted. Otherwise you would get the warnings if you
> > have any other locale set.
> > 
> 
> The en_US locale needs to be set for the layout tests and jsc tests though.
> Otherwise some intl-related tests will fail.

Oh, right. It's not being set in all cases though and it should. If you remove LANG from the list and run `LANG=es_ES.UTF-8 webkit-flatpak --gtk --debug` and inside that, call `perl`, you'll see the locale warnings since the LANG from outside got inside.
Comment 9 Philippe Normand 2020-06-21 12:26:20 PDT
Yeah I suspect this is a regression introduced when we moved from `flatpak build` to `flatpak run`.

Would it be OK if our flatpak-bwrap script unconditionally sets $LANG?
Comment 10 Alicia Boya García 2020-06-21 12:45:07 PDT
I hope so. What do you think about this?

diff --git a/Tools/flatpak/webkit-bwrap b/Tools/flatpak/webkit-bwrap
index b342a1da7e29..31d3894e04c2 100755
--- a/Tools/flatpak/webkit-bwrap
+++ b/Tools/flatpak/webkit-bwrap
@@ -58,6 +58,11 @@ def main(args: list) -> int:
     for dst, src in try_bind_mounts.items():
         bwrap_args.extend(["--bind-try", src, dst])
 
+    for env in os.environ.keys():
+        if env.startswith("LC_") or env == "LANGUAGE":
+            bwrap_args.extend(["--unsetenv", env])
+    bwrap_args.extend(["--setenv", "LANG", "en_US.UTF-8"])
+
     command_line = ' '.join(shlex.quote(a) for a in bwrap_args + args)
     # os.system return code behaves like os.wait. A 16 bit number with the
     # signal in the lower byte and, if the signal is zero, the exit code in
Comment 11 Philippe Normand 2020-06-21 12:49:16 PDT
LGTM, if it doesn't break tests :)
r=me also if you remove if from flatpakutils as it would now be un-needed there.
Comment 12 Alicia Boya García 2020-06-23 05:28:31 PDT
Created attachment 402547 [details]
Patch
Comment 13 Philippe Normand 2020-06-23 05:54:06 PDT
Comment on attachment 402547 [details]
Patch

Thanks! Can you remove the LANG reference from flatpakutils too please?
Comment 14 Alicia Boya García 2020-06-23 08:24:58 PDT
Created attachment 402560 [details]
Patch
Comment 15 EWS 2020-06-23 08:48:25 PDT
Committed r263397: <https://trac.webkit.org/changeset/263397>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402560 [details].