From c0e7d535967fe951a359068df11163ca59ac7459 Mon Sep 17 00:00:00 2001 From: Andy Gerlicher Date: Thu, 13 Aug 2015 09:55:02 -0700 Subject: [PATCH 1/3] Fix for Exec task with non-ansi characters in command. - On an OS >= Windows 7 (v6.1), the Exec command will now using UTF-8 (w/o BOM) encoding for Console output and writing the temporary .cmd file. This will enable non-ansi characters in the Command property to work as expected. - Added a unit test to cover the scenario. --- src/XMakeTasks/Exec.cs | 194 ++++++++----------------- src/XMakeTasks/UnitTests/Exec_Tests.cs | 26 ++++ 2 files changed, 88 insertions(+), 132 deletions(-) diff --git a/src/XMakeTasks/Exec.cs b/src/XMakeTasks/Exec.cs index aa9dd6db5be..82f1bebca97 100644 --- a/src/XMakeTasks/Exec.cs +++ b/src/XMakeTasks/Exec.cs @@ -3,14 +3,9 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Text; using System.IO; -using System.Resources; -using System.Reflection; -using System.Globalization; using System.Text.RegularExpressions; - using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using Microsoft.Build.Shared; @@ -33,71 +28,43 @@ public class Exec : ToolTaskExtension /// public Exec() { - // do nothing + Command = string.Empty; + + // Console-based output uses the current system OEM code page by default. Note that we should not use Console.OutputEncoding + // here since processes we run don't really have much to do with our console window (and also Console.OutputEncoding + // doesn't return the OEM code page if the running application that hosts MSBuild is not a console application). + // Note: 8/12/15 - Changed encoding to use UTF8 when OS is newer than 6.1 (Windows 7) + _standardOutputEncoding = GetEncodingWithOsFallback(); + _standardErrorEncoding = GetEncodingWithOsFallback(); } - #endregion + #endregion - #region Fields + #region Fields // Are the ecodings for StdErr and StdOut streams valid private bool _encodingParametersValid = true; - private string _command = String.Empty; - private string _userSpecifiedWorkingDirectory; private string _workingDirectory; - private bool _ignoreExitCode = false; - private bool _consoleToMSBuild = false; private ITaskItem[] _outputs; - internal bool workingDirectoryIsUNC = false; // internal for unit testing + internal bool workingDirectoryIsUNC; // internal for unit testing private string _batchFile; private string _customErrorRegex; private string _customWarningRegex; private bool _ignoreStandardErrorWarningFormat = false; // By default, detect standard-format errors - private List _nonEmptyOutput = new List(); + private readonly List _nonEmptyOutput = new List(); + private Encoding _standardErrorEncoding; + private Encoding _standardOutputEncoding; #endregion #region Properties [Required] - public string Command - { - get - { - return _command; - } - - set - { - _command = value; - } - } + public string Command { get; set; } - public string WorkingDirectory - { - get - { - return _userSpecifiedWorkingDirectory; - } + public string WorkingDirectory { get; set; } - set - { - _userSpecifiedWorkingDirectory = value; - } - } - - public bool IgnoreExitCode - { - get - { - return _ignoreExitCode; - } - - set - { - _ignoreExitCode = value; - } - } + public bool IgnoreExitCode { get; set; } /// /// Enable the pipe of the standard out to an item (StandardOutput). @@ -105,18 +72,7 @@ public bool IgnoreExitCode /// /// Even thought this is called a pipe, it is infact a Tee. Use StandardOutputImportance to adjust the visibility of the stdout. /// - public bool ConsoleToMSBuild - { - get - { - return _consoleToMSBuild; - } - - set - { - _consoleToMSBuild = value; - } - } + public bool ConsoleToMSBuild { get; set; } /// /// Users can supply a regular expression that we should @@ -158,14 +114,7 @@ protected override Encoding StandardOutputEncoding { get { return _standardOutputEncoding; } } - - /// - /// Console-based output uses the current system OEM code page by default. Note that we should not use Console.OutputEncoding - /// here since processes we run don't really have much to do with our console window (and also Console.OutputEncoding - /// doesn't return the OEM code page if the running application that hosts MSBuild is not a console application). - /// - private Encoding _standardOutputEncoding = EncodingUtilities.CurrentSystemOemEncoding; - + /// /// Property specifying the encoding of the captured task standard error stream /// @@ -174,13 +123,6 @@ protected override Encoding StandardErrorEncoding get { return _standardErrorEncoding; } } - /// - /// Console-based output uses the current system OEM code page by default. Note that we should not use Console.OutputEncoding - /// here since processes we run don't really have much to do with our console window (and also Console.OutputEncoding - /// doesn't return the OEM code page if the running application that hosts MSBuild is not a console application). - /// - private Encoding _standardErrorEncoding = EncodingUtilities.CurrentSystemOemEncoding; - /// /// Project visible property specifying the encoding of the captured task standard output stream /// @@ -226,22 +168,8 @@ public string StdErrEncoding [Output] public ITaskItem[] Outputs { - get - { - if (_outputs == null) - { - return new ITaskItem[0]; - } - else - { - return _outputs; - } - } - - set - { - _outputs = value; - } + get { return _outputs ?? new ITaskItem[0]; } + set { _outputs = value; } } /// @@ -252,18 +180,7 @@ public ITaskItem[] Outputs [Output] public ITaskItem[] ConsoleOutput { - get - { - if (!ConsoleToMSBuild) - { - return new ITaskItem[0]; - } - else - { - return _nonEmptyOutput.ToArray(); - } - } - set { } + get { return !ConsoleToMSBuild ? new ITaskItem[0] : _nonEmptyOutput.ToArray(); } } #endregion @@ -282,7 +199,8 @@ private void CreateTemporaryBatchFile() // We need to get the current OEM code page which will be the same language as the current ANSI code page, // just the OEM version. // See http://www.microsoft.com/globaldev/getWR/steps/wrg_codepage.mspx for a discussion of ANSI vs OEM - using (StreamWriter sw = new StreamWriter(_batchFile, false, EncodingUtilities.CurrentSystemOemEncoding)) // HIGHCHAR: Exec task batch files are in OEM code pages (not ANSI!) + // Note: 8/12/15 - Switched to use UTF8 on OS newer than 6.1 (Windows 7) + using (StreamWriter sw = new StreamWriter(_batchFile, false, GetEncodingWithOsFallback())) { // In some wierd setups, users may have set an env var actually called "errorlevel" // this would cause our "exit %errorlevel%" to return false. @@ -299,6 +217,13 @@ private void CreateTemporaryBatchFile() sw.WriteLine("set errorlevel=dummy"); sw.WriteLine("set errorlevel="); + // We probably have to change the code page to UTF8 (65001) for non-ansi characters to work. + if (EncodingUtilities.CurrentSystemOemEncoding.CodePage != sw.Encoding.CodePage) + { + // Output to nul so we don't change output and logs. + sw.WriteLine($"chcp {sw.Encoding.CodePage}>nul"); + } + // if the working directory is a UNC path, bracket the exec command with pushd and popd, because pushd // automatically maps the network path to a drive letter, and then popd disconnects it if (workingDirectoryIsUNC) @@ -359,23 +284,21 @@ protected override int ExecuteTool(string pathToTool, string responseFileCommand /// protected override bool HandleTaskExecutionErrors() { - if (_ignoreExitCode) + if (IgnoreExitCode) { Log.LogMessageFromResources(MessageImportance.Normal, "Exec.CommandFailedNoErrorCode", this.Command, ExitCode); return true; } + + if (ExitCode == NativeMethods.SE_ERR_ACCESSDENIED) + { + Log.LogErrorWithCodeFromResources("Exec.CommandFailedAccessDenied", this.Command, ExitCode); + } else { - if (ExitCode == NativeMethods.SE_ERR_ACCESSDENIED) - { - Log.LogErrorWithCodeFromResources("Exec.CommandFailedAccessDenied", this.Command, ExitCode); - } - else - { - Log.LogErrorWithCodeFromResources("Exec.CommandFailed", this.Command, ExitCode); - } - return false; + Log.LogErrorWithCodeFromResources("Exec.CommandFailed", this.Command, ExitCode); } + return false; } /// @@ -387,7 +310,6 @@ protected override bool HandleTaskExecutionErrors() protected override void LogPathToTool(string toolName, string pathToTool) { // Do nothing - return; } /// @@ -397,10 +319,7 @@ protected override void LogPathToTool(string toolName, string pathToTool) /// Overridden to log the batch file command instead of the cmd.exe command. /// /// - protected override void LogToolCommand - ( - string message - ) + protected override void LogToolCommand(string message) { //Dont print the command line if Echo is Off. if (!EchoOff) @@ -416,11 +335,7 @@ string message /// /// Overridden to handle any custom regular expressions supplied. /// - protected override void LogEventsFromTextOutput - ( - string singleLine, - MessageImportance messageImportance - ) + protected override void LogEventsFromTextOutput(string singleLine, MessageImportance messageImportance) { if (OutputMatchesRegex(singleLine, ref _customErrorRegex)) { @@ -505,8 +420,8 @@ protected override bool ValidateParameters() // determine what the working directory for the exec command is going to be -- if the user specified a working // directory use that, otherwise it's the current directory - _workingDirectory = ((_userSpecifiedWorkingDirectory != null) && (_userSpecifiedWorkingDirectory.Length > 0)) - ? _userSpecifiedWorkingDirectory + _workingDirectory = !string.IsNullOrEmpty(WorkingDirectory) + ? WorkingDirectory : Directory.GetCurrentDirectory(); // check if the working directory we're going to use for the exec command is a UNC path @@ -617,10 +532,7 @@ protected internal override void AddCommandLineCommands(CommandLineBuilderExtens /// protected override string ToolName { - get - { - return "cmd.exe"; - } + get { return "cmd.exe"; } } /// @@ -645,5 +557,23 @@ protected override MessageImportance StandardOutputLoggingImportance } #endregion + + private static readonly Encoding s_utf8WithoutBom = new UTF8Encoding(false); + + /// + /// Get encoding based on OS. This will fall back to previous behavior on any OS before Windows 7. + /// If the OS is greater than Windows 7, UTF8 encoding will be used for the cmd file. + /// + /// UTF8 w/o BOM is used because cmd.exe does not like a BOM in a .cmd file. + /// Encoding to use + private Encoding GetEncodingWithOsFallback() + { + // Windows 7 (6.1) or greater + var windows7 = new Version(6, 1); + + return Environment.OSVersion.Version >= windows7 + ? s_utf8WithoutBom + : EncodingUtilities.CurrentSystemOemEncoding; + } } } diff --git a/src/XMakeTasks/UnitTests/Exec_Tests.cs b/src/XMakeTasks/UnitTests/Exec_Tests.cs index c0eef116beb..584936a55f5 100644 --- a/src/XMakeTasks/UnitTests/Exec_Tests.cs +++ b/src/XMakeTasks/UnitTests/Exec_Tests.cs @@ -287,6 +287,32 @@ public void TempPathContainsAmpersand4() } } + /// + /// Tests that Exec still executes properly when there's a non-ansi character in the command + /// + [TestMethod] + public void ExecTaskUnicodeCharacterInCommand() + { + string nonAnsiCharacters = "创建"; + string folder = Path.Combine(Path.GetTempPath(), nonAnsiCharacters); + string command = Path.Combine(folder, "test.cmd"); + + try + { + Directory.CreateDirectory(folder); + File.WriteAllText(command, "echo [hello]"); + Exec exec = PrepareExec(command); + + Assert.IsTrue(exec.Execute(), "Task should have succeeded"); + ((MockEngine)exec.BuildEngine).AssertLogContains("[hello]"); + } + finally + { + if (Directory.Exists(folder)) + Directory.Delete(folder, true); + } + } + [TestMethod] public void InvalidUncDirectorySet() { From c95ec7a5dd1746fa5fcd00ab487d2d25d60185df Mon Sep 17 00:00:00 2001 From: Andy Gerlicher Date: Thu, 13 Aug 2015 10:02:29 -0700 Subject: [PATCH 2/3] Fix build warning - unused variable defined --- .../UnitTests/Construction/SolutionFile_Tests.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/XMakeBuildEngine/UnitTests/Construction/SolutionFile_Tests.cs b/src/XMakeBuildEngine/UnitTests/Construction/SolutionFile_Tests.cs index ba16661f1da..e52e3cb4bde 100644 --- a/src/XMakeBuildEngine/UnitTests/Construction/SolutionFile_Tests.cs +++ b/src/XMakeBuildEngine/UnitTests/Construction/SolutionFile_Tests.cs @@ -1007,14 +1007,7 @@ public void ParseSolutionConfigurationWithEmptyLines() EndGlobal "; - try - { - SolutionFile solution = ParseSolutionHelper(solutionFileContents); - } - catch (InvalidProjectFileException ex) - { - Assert.Fail(); - } + ParseSolutionHelper(solutionFileContents); } /// From 77aed76a250d734bee09d34fce2f473c1b56487b Mon Sep 17 00:00:00 2001 From: Andy Gerlicher Date: Thu, 13 Aug 2015 12:29:51 -0700 Subject: [PATCH 3/3] Fix comments and remove C#6 string interpolation. --- src/XMakeTasks/Exec.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/XMakeTasks/Exec.cs b/src/XMakeTasks/Exec.cs index 82f1bebca97..693eec05e3d 100644 --- a/src/XMakeTasks/Exec.cs +++ b/src/XMakeTasks/Exec.cs @@ -33,7 +33,7 @@ public Exec() // Console-based output uses the current system OEM code page by default. Note that we should not use Console.OutputEncoding // here since processes we run don't really have much to do with our console window (and also Console.OutputEncoding // doesn't return the OEM code page if the running application that hosts MSBuild is not a console application). - // Note: 8/12/15 - Changed encoding to use UTF8 when OS is newer than 6.1 (Windows 7) + // Note: 8/12/15 - Changed encoding to use UTF8 when OS version is greater than or equal to 6.1 (Windows 7) _standardOutputEncoding = GetEncodingWithOsFallback(); _standardErrorEncoding = GetEncodingWithOsFallback(); } @@ -221,7 +221,7 @@ private void CreateTemporaryBatchFile() if (EncodingUtilities.CurrentSystemOemEncoding.CodePage != sw.Encoding.CodePage) { // Output to nul so we don't change output and logs. - sw.WriteLine($"chcp {sw.Encoding.CodePage}>nul"); + sw.WriteLine(string.Format("chcp {0}>nul", sw.Encoding.CodePage)); } // if the working directory is a UNC path, bracket the exec command with pushd and popd, because pushd @@ -562,7 +562,7 @@ protected override MessageImportance StandardOutputLoggingImportance /// /// Get encoding based on OS. This will fall back to previous behavior on any OS before Windows 7. - /// If the OS is greater than Windows 7, UTF8 encoding will be used for the cmd file. + /// If the OS is greater than or equal to Windows 7, UTF8 encoding will be used for the cmd file. /// /// UTF8 w/o BOM is used because cmd.exe does not like a BOM in a .cmd file. /// Encoding to use