Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some tests fail when system have unconfigured locales #773

Closed
talau opened this issue Dec 12, 2021 · 5 comments · Fixed by #775
Closed

some tests fail when system have unconfigured locales #773

talau opened this issue Dec 12, 2021 · 5 comments · Fixed by #775
Milestone

Comments

@talau
Copy link

talau commented Dec 12, 2021

Hi there!

On Linux, when you run tests on a chroot/docker that do not have locales
installed and configured, some tests, like parser_test.go and interp_test.go, fail.

One example of fail:

=== CONT TestRunnerRunConfirm/#133
interp_test.go:3280: wrong bash output in "INTERP_GLOBAL=; $ENV_PROG | grep '^INTERP_GLOBAL='":
want: "INTERP_GLOBAL=\n"
got: "bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)\nINTERP_GLOBAL=\n"

As some systems, like Debian, use CI tests in an environment like that,
please, can you consider ignoring warnings like this on the tests?

Cheers,
mt

@mvdan
Copy link
Owner

mvdan commented Dec 12, 2021

Thanks for letting me know; looks like using C.UTF-8 fixes the issue. I also see that a few other tests might fail due to UID/PID assumptions.

@mvdan
Copy link
Owner

mvdan commented Dec 12, 2021

Hmm, looks like that locale isn't on my system by default, either. C might have to do.

@talau
Copy link
Author

talau commented Dec 12, 2021

Yep, I think that changing en_US.UTF-8 to C in os.Setenv on interp_test.go/parser_test.go fixes the issue.

@mvdan
Copy link
Owner

mvdan commented Dec 13, 2021

It's unfortunately not that easy, because C implies no UTF-8, and some of the tests assume it. I think I'll have to go with C.UTF-8, with perhaps some form of logic to detect whether it's present. Arch Linux doesn't have it for instance, as glibc won't ship with it until 2.35.

mvdan added a commit that referenced this issue Dec 14, 2021
Such a minimal system has no extra locales installed,
so trying to use "en_US.UTF-8" might fail.
Ideally we would want the standard "C" locale,
as it is meant for consumption by computers,
but unfortunately it also means no built-in UTF-8.

However, there's good news; "C.UTF-8" is emerging for our use case.
Many systems, like Debian, already ship with it by default.
glibc won't include it until 2.35, and Arch ships 2.33,
so for the time being, Arch Linux and some other distros lack it.
For their sake and mine, fall back to our previous locale.

While here, fix a bug I spotted via dockexec in the interpreter.
If we can't figure out who the current user is,
don't assume they have super-user powers, as that's likely untrue.
The most likely reason is a broken UID or /etc/passwd.
In such scenarios, bash assumes the user has no permissions at all;
for instance, bash will refuse to "cd" into most (all?) directories.
Do the same, for consistency.

Fixes #773.
@talau
Copy link
Author

talau commented Dec 14, 2021

Hi there!

I tested on Debian and it's fixed the issue! Thanks @mvdan!

You can see the log here: https://salsa.debian.org/talau-guest/shfmt-tests-minimal/-/jobs/2277914/raw

mvdan added a commit that referenced this issue Dec 14, 2021
Such a minimal system has no extra locales installed,
so trying to use "en_US.UTF-8" might fail.
Ideally we would want the standard "C" locale,
as it is meant for consumption by computers,
but unfortunately it also means no built-in UTF-8.

However, there's good news; "C.UTF-8" is emerging for our use case.
Many systems, like Debian, already ship with it by default.
glibc won't include it until 2.35, and Arch ships 2.33,
so for the time being, Arch Linux and some other distros lack it.
For their sake and mine, fall back to our previous locale.

While here, fix a bug I spotted via dockexec in the interpreter.
If we can't figure out who the current user is,
don't assume they have super-user powers, as that's likely untrue.
The most likely reason is a broken UID or /etc/passwd.
In such scenarios, bash assumes the user has no permissions at all;
for instance, bash will refuse to "cd" into most (all?) directories.
Do the same, for consistency.

Fixes #773.
@mvdan mvdan added this to the 3.4.2 milestone Dec 16, 2021
mvdan added a commit that referenced this issue Dec 16, 2021
Such a minimal system has no extra locales installed,
so trying to use "en_US.UTF-8" might fail.
Ideally we would want the standard "C" locale,
as it is meant for consumption by computers,
but unfortunately it also means no built-in UTF-8.

However, there's good news; "C.UTF-8" is emerging for our use case.
Many systems, like Debian, already ship with it by default.
glibc won't include it until 2.35, and Arch ships 2.33,
so for the time being, Arch Linux and some other distros lack it.
For their sake and mine, fall back to our previous locale.

Note that this backport excludes the interp/os_unix.go change in v3.5.
It seems a bit too risky for a bugfix backport.

Fixes #773.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants