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

Exec: stop setting a locale on Unix. #9449

Merged
merged 1 commit into from
Dec 6, 2023
Merged

Conversation

tmds
Copy link
Member

@tmds tmds commented Nov 23, 2023

The en_US locale can't be used on systems where it is not installed. This is common in container images.

On such systems, setting the locale to en_US.UTF-8 causes bash to print unexpected warnings to standard error.
When Exec.LogStandardErrorAsError is set, these warnings cause the Task to fail due to logging errors.

This changes to no longer set the locale explicitly. The Exec command will now run under the system locale instead of US English. Most tools should functionally behave the same under any locale.

Users may still set the locale environment variables themselves through Exec.EnvironmentVariables.

The previous behavior can also be restored as it is under a changewave.

@KalleOlaviNiemitalo @rainersigwald @wfurt @danmoseley ptal.

cc @mthalman @omajid

Fixes #4194

The en_US locale can't be used on systems where it is not installed.
This is common in container images.

On such systems, setting the locale to en_US.UTF-8 causes bash to
print unexpected warnings to standard error.
When Exec.LogStandardErrorAsError is set, these warnings cause the
Task to fail due to logging errors.

This changes to no longer set the locale explicitly. The Exec command
will now run under the system locale instead of US English.
Most tools should functionally behave the same under any locale.

Users may still set the locale environment variables themselves through
Exec.EnvironmentVariables.

The previous behavior can also be restored as it is under a changewave.
@tmds
Copy link
Member Author

tmds commented Nov 24, 2023

@rainersigwald can this still be part of 8.0.2xx?

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have time for this kind of fix for 8.0.200. Thanks!

@tmds
Copy link
Member Author

tmds commented Dec 5, 2023

Is this good to merge? And will it automatically flow into 8.0.2xx?

@rainersigwald rainersigwald changed the base branch from main to vs17.9 December 5, 2023 15:33
@rainersigwald
Copy link
Member

@AR-May let's get this in for 17.9 as soon as you've got the branching stuff worked out.

@AR-May
Copy link
Member

AR-May commented Dec 6, 2023

We can merge this. I will check that it flows to 8.0.2xx asap. The automatic flow is yet not fully configured, but I am on that.

@AR-May AR-May merged commit 8d10036 into dotnet:vs17.9 Dec 6, 2023
8 checks passed
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 this pull request may close these issues.

Exec tasks warns: bash : warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
4 participants