Read /etc/locale.conf when configuring the distro locale#40940
Read /etc/locale.conf when configuring the distro locale#40940i4M1k0SU wants to merge 2 commits into
Conversation
ConfigUpdateLanguage only read /etc/default/locale to populate the $LANG environment variable. That path is used by Debian/Ubuntu, but systemd-based distributions (Fedora, Arch, openSUSE, ...) store the locale in /etc/locale.conf instead, so $LANG was never set on those distros. Check both locations and use the first one that exists. Both files share the same "LANG=" line format, so the existing parsing is reused unchanged. Fixes microsoft#13207
There was a problem hiding this comment.
Pull request overview
This PR updates Linux init locale propagation so $LANG can be sourced from both Debian/Ubuntu-style /etc/default/locale and systemd-style /etc/locale.conf, improving correctness for systemd-based distributions.
Changes:
- Added
/etc/locale.confas an additional locale configuration path. - Updated
ConfigUpdateLanguageto try multiple locale file locations and use the first successfully opened file. - Expanded function comments to document the multiple supported locations.
|
@microsoft-github-policy-service agree |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@i4M1k0SU - this change looks good, I am not sure why the policy bot check isn't working... |
|
@microsoft-github-policy-service agree |
Thanks! I re-commented and the policy bot check went green now. |
OneBlue
left a comment
OneBlue
left a comment
There was a problem hiding this comment.
LGTM. This code path is actually not very well covered by tests, but I would recommend adding a unit test in test/windows/UnitTests.cpp (we could create these files in the test distro via DistroFileChange) and validate that we set LANG correctly when either or both of these config files are present
Cover the ConfigUpdateLanguage code path that populates $LANG from the distro locale file. The test creates the locale files in the test distro and verifies that $LANG is set correctly for each supported layout: - only /etc/default/locale present (Debian/Ubuntu) - only /etc/locale.conf present (Fedora, Arch, openSUSE, ...) - both present, confirming /etc/default/locale takes precedence
@OneBlue Thanks! Added a unit test covering all three cases (either file alone, and both present) in 250c111. |
Summary of the Pull Request
initpopulates the distro's$LANGenvironment variable from/etc/default/locale, but that was the only path checked./etc/default/localeis the Debian/Ubuntu convention; systemd-based distributions (Fedora, Arch,
openSUSE, ...) store the locale in
/etc/locale.confinstead. On those distros$LANGwas never propagated from the distro's configuration.This change makes
ConfigUpdateLanguagecheck both locations and use the firstone that exists.
PR Checklist
/etc/default/locale, should fall back to/etc/locale.conf#13207Detailed Description of the Pull Request / Additional comments
LOCALE_CONF_FILE_PATH(/etc/locale.conf).ConfigUpdateLanguagenow tries/etc/default/localethen/etc/locale.conf,opening the first file that exists.
KEY=valueformat with aLANG=line, so the existingline-parsing logic is reused unchanged.
/etc/default/localetakes precedence when both files exist.ENOENT) is skipped silently; otherfopenerrors are stilllogged.
$LANGis left unchanged.Validation Steps Performed
inittarget (cross-compiled via the in-tree clang/LINUXSDKtoolchain) — compiles cleanly.
/etc/default/locale): existing behavior ispreserved —
$LANGis still propagated correctly./etc/locale.conf):$LANGis now correctlypropagated from the distro's locale configuration.