Skip to content

Commit

Permalink
Exec: stop setting a locale on Unix. (#9449)
Browse files Browse the repository at this point in the history
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.

Fixes #4194
  • Loading branch information
tmds authored Dec 6, 2023
1 parent 1616515 commit 8d10036
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
36 changes: 36 additions & 0 deletions src/Tasks.UnitTests/Exec_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,42 @@ public void EscapeSpecifiedCharactersInPathToGeneratedBatchFile()
}
}

[UnixOnlyTheory]
[InlineData(true)]
[InlineData(false)]
public void ExecSetsLocaleOnUnix(bool enableChangeWave)
{
using (var env = TestEnvironment.Create())
{
env.SetEnvironmentVariable("LANG", null);
env.SetEnvironmentVariable("LC_ALL", null);

if (enableChangeWave)
{
ChangeWaves.ResetStateForTests();
// Important: use the version here
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_10.ToString());
BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly();
}

Exec exec = PrepareExec("echo LANG=$LANG; echo LC_ALL=$LC_ALL;");
bool result = exec.Execute();
Assert.True(result);

MockEngine engine = (MockEngine)exec.BuildEngine;
if (enableChangeWave)
{
engine.AssertLogContains("LANG=en_US.UTF-8");
engine.AssertLogContains("LC_ALL=en_US.UTF-8");
}
else
{
engine.AssertLogDoesntContain("LANG=en_US.UTF-8");
engine.AssertLogDoesntContain("LC_ALL=en_US.UTF-8");
}
}
}

/// <summary>
/// Ensures that calling the Exec task does not leave any extra TEMP files
/// lying around.
Expand Down
7 changes: 6 additions & 1 deletion src/Tasks/Exec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,12 @@ protected internal override void AddCommandLineCommands(CommandLineBuilderExtens
{
commandLine.AppendSwitch("-c");
commandLine.AppendTextUnquoted(" \"");
commandLine.AppendTextUnquoted("export LANG=en_US.UTF-8; export LC_ALL=en_US.UTF-8; . ");
bool setLocale = !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10);
if (setLocale)
{
commandLine.AppendTextUnquoted("export LANG=en_US.UTF-8; export LC_ALL=en_US.UTF-8; ");
}
commandLine.AppendTextUnquoted(". ");
commandLine.AppendFileNameIfNotNull(batchFileForCommandLine);
commandLine.AppendTextUnquoted("\"");
}
Expand Down

0 comments on commit 8d10036

Please sign in to comment.