base-files: don't overwrite existing locale and define default LANG

[oreo639]

Testing the changes

• I tested the changes in this PR: YES

Fixes overwriting user/desktop specified locale.

Currently we always overwrite LANG on login, even if it was already set by the desktop or user which causes issues with GDM.

You can see what Arch does here:
https://bugs.archlinux.org/task/42162
https://github.com/archlinux/svntogit-packages/blob/master/filesystem/trunk/locale.sh

You can see what Fedora does here:
https://gist.github.com/oreo639/3b83b852ce0db21e21eed102a57c5148
(taken from the setup package)

Closes: #15292

[github-actions]

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

[oreo639]

bump

[leahneukirchen]

Why would this work? Doesn't bash start with /etc/profile, which then sources /etc/profile.d/*?

In any case, setting it later in .profile overrides this.

[oreo639]

Because GDM sets LANG and then gnome-session launches bash as a login shell so that /etc/profile can be applied which overwrites the previous LANG preference.

This PR adds a check for if LANG is already defined (i.e. by the user/DE/etc) before applying the system-wide preference.

[github-actions]

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

[oreo639]

bump

[leahneukirchen]

This feels very crude... I wonder if this would work instead:

while read -r line < /etc/locale.conf; do
	eval ": \${{$line}}"
done

[ahesford]

I don't think we should be worrying about this. The stuff in /etc/profile and /etc/profile.d is supposed to be read for login shells, which means that the shell is expected to stand up the environment from scratch. It isn't really a bug that login shells overwrite whatever locale information already exists in the environment.

Hacking around the fact that GDM expects its forcing of these variables to be respected seems best resolved by looking to GDM_LANG in some gdm-specific profile.d snippet that runs after the default locale is configured.

[github-actions]

Pull Requests become stale 90 days after last activity and are closed 14 days after that. If this pull request is still relevant bump it or assign it.

[oreo639]

bump

[ahesford]

The parsing of locale.conf seem to be holdovers from aborted attempts to save and restore variables. Reading the lines, trying to strip out comments and then running the lines through eval doesn't really do anything different than sourcing the file, except inject some incomplete and cumbersome parsing logic here.

The locale.conf parsing should just be restored to simply sourcing the file, leavening only the default LANG and typo fix.

[oreo639]

Reading the lines, trying to strip out comments and then running the lines through eval doesn't really do anything different than sourcing the file, except inject some incomplete and cumbersome parsing logic here.

It prevents locale.sh from overwriting e.g. LANG if it was already defined and uses locale.conf as the default values if they are not defined.

e.g. : ${LANG=en_US.UTF-8}

[ahesford]

Thanks; I wasn't reasoning through the assignment properly.

There are still problems sanitizing input. For example, if somebody has a file like

LANG=en_US.UTF-8 # English

it will contain a trailing space after the change but is currently properly parsed. I don't know whether any of the consumers of locale variables would be harmed by the addition of spurious whitespace, but I'd be surprised if this didn't introduce problems.

I have not found any documentation on locale.conf besides systemd manual pages. As I said before (apparently elsewhere; I thought I commented here earlier), we should not feel compelled to abide by the restrictions imposed by systemd against using other shell features in the configuration. Because we offer no manual page on the file, the code is the documentation; the code documents that the file is simply sourced. (The limitation in systemd, I assume, is that it parses the file directly. We don't parse the file anywhere but in shell profiles.) Users may currently be exploiting that to add extra logic to the locale configuration.

What Red Hat does to interpret the file is an ugly kludge that we should avoid. If anything, I think the Arch way is preferable; just let a pre-set LANG gate the sourcing of the file and, if some users really care about more complex preservation of the whole gamut of variables, they can implement a custom post-locale.sh to do so. However, I again question the need for this. Given we have established precedent that locale.conf is just sourced by a shell, users should feel free to do things like

: ${LANG=my-lang}

if they need to preserve a pre-set value.

Whatever the final form looks like, we should include a manual page or at least fully define on the docs web site what we accept as a valid configuration.

[oreo639]

There are still problems sanitizing input. For example, if somebody has a file like

Thank you for pointing that out.

I have not found any documentation on locale.conf besides systemd manual pages.

locale.conf originates from systemd, before that distributions had their own locale configuration files, e.g. /etc/sysconfig/i18n on RHEL or /etc/default/locale on debian. Arch Linux used to recommend just setting LANG in your .bashrc (or /etc/rc.conf for system wide).

I think the Arch way is preferable; just let a pre-set LANG gate the sourcing of the file

That was the original way I implemented this PR and people didn't like it.

Given we have established precedent that locale.conf is just sourced by a shell, users should feel free to do things like

Personally, I would prefer for that to continue to work as it did before to not break things for people, although I don't think it should be promoted or listed as supported.

[oreo639]

I updated this to only replace plain declarations (e.g. VAR=x, but not : VAR=x, export VAR=x, etc)
This allows everything else to work as it did before (e.g. if statements) and you can force overriding with : LANG=en_US.UTF-8

[ahesford]

I think this is getting far too complicated for the simple intent of preserving variables that have already been set. I had to read that loop several times to figure out what it was doing even knowing your intent.

It's also fragile. You're effectively just sourcing the file after wrapping all simple variable assignments in : ${VAR=value} to preserve variables already set by the environment while trying to honor existing workflows that might try to inject more complicated behavior into locale.conf, but there's no way to guarantee that these kinds of modifications to what's executed during the eval won't have all sorts of unintended consequences.

I think there are four reasonable ways to solve your problem (with varying degrees of reasonability):

1. Leave the behavior as is and let users guard variables they might not want to overwrite, knowing that locale.conf is executed by the shell.
2. Gate the sourcing like in Arch, which you said people don't like. But do those people who objected really prefer writing trying to implement an even more complex shell parser in the shell itself?
3. Make backup copies of all relevant variables, just source the file as we already do, and restore the backup copies if they were nonempty before the source. I think you did this, in a couple of different ways, in an earlier proposal.
4. Properly validate that locale.conf contains nothing more than comments or simple VAR=value assignments. Reject it entirely if it does not conform, otherwise parse accordingly.

[oreo639]

3. I think you did this, in a couple of different ways, in an earlier proposal.

Here is what I did for that: https://gist.github.com/oreo639/850ddb140df502781f1f54fc1ffaba0c

4. Properly validate that locale.conf contains nothing more than comments or simple VAR=value assignments. Reject it entirely if it does not conform, otherwise parse accordingly.

I pushed a version using that approach, although it will still accept lines that are valid (instead of rejecting the entire file)
I also updated it to use the regex from Fedora to check for a valid assignment.

[leahneukirchen]

error messages should go to stderr