From d1d23944df460b88b4620813df9b119f2860000d Mon Sep 17 00:00:00 2001 From: Bryan Thornbury Date: Fri, 22 Jan 2016 03:59:04 -0800 Subject: [PATCH] Changes to fix publish tests --- .../ArgumentEscaper.cs | 184 +++++++++--------- .../CommandResolver.cs | 15 +- .../ScriptExecutor.cs | 22 +-- .../AssemblyInfoFileGenerator.cs | 2 +- .../CompileContext.cs | 2 +- .../Program.cs | 12 +- .../Program.cs | 10 +- .../project.json | 2 +- .../CompileFail/project.json | 6 +- .../IncrementalTestBase.cs | 2 +- .../Commands/TestCommand.cs | 5 +- test/TestProjects/TestApp/project.json | 2 +- 12 files changed, 124 insertions(+), 140 deletions(-) diff --git a/src/Microsoft.DotNet.Cli.Utils/ArgumentEscaper.cs b/src/Microsoft.DotNet.Cli.Utils/ArgumentEscaper.cs index b16a70ae7..808ad1e14 100644 --- a/src/Microsoft.DotNet.Cli.Utils/ArgumentEscaper.cs +++ b/src/Microsoft.DotNet.Cli.Utils/ArgumentEscaper.cs @@ -20,30 +20,9 @@ namespace Microsoft.DotNet.Cli.Utils /// /// /// - public static string EscapeAndConcatenateArgArray(IEnumerable args, bool cmd=false) + public static string EscapeAndConcatenateArgArray(IEnumerable args) { - var sb = new StringBuilder(); - var first = false; - - foreach (var arg in args) - { - if (first) - { - first = false; - } - else - { - sb.Append(' '); - } - sb.Append(EscapeArg(arg, cmd)); - } - - return sb.ToString(); - } - - public static string EscapeAndConcatenateArgArrayForBash(IEnumerable args) - { - return EscapeAndConcatenateArgArray(EscapeArgArrayForBash(args)); + return string.Join(" ", EscapeArgArray(args)); } /// @@ -57,7 +36,7 @@ namespace Microsoft.DotNet.Cli.Utils /// public static string EscapeAndConcatenateArgArrayForCmd(IEnumerable args) { - return EscapeAndConcatenateArgArray(EscapeArgArrayForCmd(args), true); + return string.Join(" ", EscapeArgArrayForCmd(args)); } /// @@ -81,18 +60,6 @@ namespace Microsoft.DotNet.Cli.Utils return escapedArgs; } - public static IEnumerable EscapeArgArrayForBash(IEnumerable arguments) - { - var escapedArgs = new List(); - - foreach (var arg in arguments) - { - escapedArgs.Add(EscapeArgForBash(arg)); - } - - return escapedArgs; - } - /// /// This prefixes every character with the '^' character to force cmd to /// interpret the argument string literally. An alternative option would @@ -115,78 +82,52 @@ namespace Microsoft.DotNet.Cli.Utils return escapedArgs; } - private static string EscapeArg(string arg, bool cmd=false) + private static string EscapeArg(string arg) { var sb = new StringBuilder(); - // Always quote beginning and end to account for possible spaces - if (cmd) sb.Append('^'); - sb.Append('"'); + var quoted = ShouldSurroundWithQuotes(arg); + if (quoted) sb.Append("\""); - if (!cmd) + for (int i = 0; i < arg.Length; ++i) { - for (int i = 0; i < arg.Length; ++i) + var backslashCount = 0; + + // Consume All Backslashes + while (i < arg.Length && arg[i] == '\\') { - var backslashCount = 0; - - // Consume All Backslashes - while (i < arg.Length && arg[i] == '\\') - { - backslashCount++; - i++; - } - - // Escape any backslashes at the end of the arg - // This ensures the outside quote is interpreted as - // an argument delimiter - if (i == arg.Length) - { - sb.Append('\\', 2 * backslashCount); - } - - // Escape any preceding backslashes and the quote - else if (arg[i] == '"') - { - sb.Append('\\', (2 * backslashCount) + 1); - sb.Append('"'); - } - - // Output any consumed backslashes and the character - else - { - sb.Append('\\', backslashCount); - sb.Append(arg[i]); - } + backslashCount++; + i++; } - } - else - { - for (int i = 0; i < arg.Length; ++i) + + // Escape any backslashes at the end of the arg + // This ensures the outside quote is interpreted as + // an argument delimiter + if (i == arg.Length) { - if (arg[i] == '"') - { - sb.Append('"'); - sb.Append('^'); - sb.Append(arg[i]); - } - else - { - sb.Append(arg[i]); - } + sb.Append('\\', 2 * backslashCount); + } + + // Escape any preceding backslashes and the quote + else if (arg[i] == '"') + { + sb.Append('\\', (2 * backslashCount) + 1); + sb.Append('"'); + } + + // Output any consumed backslashes and the character + else + { + sb.Append('\\', backslashCount); + sb.Append(arg[i]); } } - if (cmd) sb.Append('^'); - sb.Append('"'); + if (quoted) sb.Append("\""); return sb.ToString(); } - private static string EscapeArgForBash(string arguments) - { - throw new NotImplementedException(); - } - /// /// Prepare as single argument to /// roundtrip properly through cmd. @@ -200,17 +141,66 @@ namespace Microsoft.DotNet.Cli.Utils /// /// /// - private static string EscapeArgForCmd(string arguments) + private static string EscapeArgForCmd(string argument) { var sb = new StringBuilder(); - foreach (var character in arguments) + var quoted = ShouldSurroundWithQuotes(argument); + + if (quoted) sb.Append("^\""); + + foreach (var character in argument) { - sb.Append('^'); - sb.Append(character); + + if (character == '"') + { + + sb.Append('^'); + sb.Append('"'); + sb.Append('^'); + sb.Append(character); + } + else + { + sb.Append("^"); + sb.Append(character); + } } + if (quoted) sb.Append("^\""); + return sb.ToString(); } + + /// + /// Prepare as single argument to + /// roundtrip properly through cmd. + /// + /// This prefixes every character with the '^' character to force cmd to + /// interpret the argument string literally. An alternative option would + /// be to do this only for cmd metacharacters. + /// + /// See here for more info: + /// http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx + /// + /// + /// + internal static bool ShouldSurroundWithQuotes(string argument) + { + // Don't quote already quoted strings + if (argument.StartsWith("\"", StringComparison.Ordinal) && + argument.EndsWith("\"", StringComparison.Ordinal)) + { + return false; + } + + // Only quote if whitespace exists in the string + if (argument.Contains(" ") || argument.Contains("\t") || argument.Contains("\n")) + { + return true; + } + + return true; + } } } diff --git a/src/Microsoft.DotNet.Cli.Utils/CommandResolver.cs b/src/Microsoft.DotNet.Cli.Utils/CommandResolver.cs index 00e967b44..9b722614e 100644 --- a/src/Microsoft.DotNet.Cli.Utils/CommandResolver.cs +++ b/src/Microsoft.DotNet.Cli.Utils/CommandResolver.cs @@ -24,7 +24,6 @@ namespace Microsoft.DotNet.Cli.Utils private static CommandSpec ResolveFromPath(string commandName, IEnumerable args, bool useComSpec = false) { var commandPath = Env.GetCommandPath(commandName); - if (commandPath != null) Console.WriteLine("path?"); return commandPath == null ? null : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.Path, useComSpec); @@ -33,7 +32,6 @@ namespace Microsoft.DotNet.Cli.Utils private static CommandSpec ResolveFromAppBase(string commandName, IEnumerable args, bool useComSpec = false) { var commandPath = Env.GetCommandPathFromAppBase(AppContext.BaseDirectory, commandName); - if (commandPath != null) Console.WriteLine("appbase?"); return commandPath == null ? null : CreateCommandSpecPreferringExe(commandName, args, commandPath, CommandResolutionStrategy.BaseDirectory, useComSpec); @@ -43,8 +41,6 @@ namespace Microsoft.DotNet.Cli.Utils { if (Path.IsPathRooted(commandName)) { - Console.WriteLine("rooted?"); - if (useComSpec) { return CreateComSpecCommandSpec(commandName, args, CommandResolutionStrategy.Path); @@ -196,8 +192,6 @@ namespace Microsoft.DotNet.Cli.Utils fileName = Path.Combine(packageDir, commandPath); } - - if (useComSpec) { return CreateComSpecCommandSpec(fileName, args, CommandResolutionStrategy.NugetPackage); @@ -256,7 +250,6 @@ namespace Microsoft.DotNet.Cli.Utils IEnumerable args, CommandResolutionStrategy resolutionStrategy) { - // To prevent Command Not Found, comspec gets passed in as // the command already in some cases var comSpec = Environment.GetEnvironmentVariable("ComSpec"); @@ -266,7 +259,13 @@ namespace Microsoft.DotNet.Cli.Utils args = args.Skip(1); } var cmdEscapedArgs = ArgumentEscaper.EscapeAndConcatenateArgArrayForCmd(args); - var escapedArgString = $"/s /c \"\"{command}\" {cmdEscapedArgs}\""; + + if (ArgumentEscaper.ShouldSurroundWithQuotes(command)) + { + command = $"\"{command}\""; + } + + var escapedArgString = $"/s /c \"{command} {cmdEscapedArgs}\""; return new CommandSpec(comSpec, escapedArgString, resolutionStrategy); } diff --git a/src/Microsoft.DotNet.Cli.Utils/ScriptExecutor.cs b/src/Microsoft.DotNet.Cli.Utils/ScriptExecutor.cs index 99a23e438..2454a04d0 100644 --- a/src/Microsoft.DotNet.Cli.Utils/ScriptExecutor.cs +++ b/src/Microsoft.DotNet.Cli.Utils/ScriptExecutor.cs @@ -22,7 +22,7 @@ namespace Microsoft.DotNet.Cli.Utils var scriptArguments = CommandGrammar.Process( scriptCommandLine, GetScriptVariable(project, getVariable), - preserveSurroundingQuotes: false); + preserveSurroundingQuotes: true); // Ensure the array won't be empty and the elements won't be null or empty strings. scriptArguments = scriptArguments.Where(argument => !string.IsNullOrEmpty(argument)).ToArray(); @@ -59,12 +59,14 @@ namespace Microsoft.DotNet.Cli.Utils else { // Special-case a script name that, perhaps with added .sh, matches an existing file. + var surroundWithQuotes = false; var scriptCandidate = scriptArguments[0]; if (scriptCandidate.StartsWith("\"", StringComparison.Ordinal) && scriptCandidate.EndsWith("\"", StringComparison.Ordinal)) { // Strip surrounding quotes; they were required in project.json to keep the script name // together but confuse File.Exists() e.g. "My Script", lacking ./ prefix and .sh suffix. + surroundWithQuotes = true; scriptCandidate = scriptCandidate.Substring(1, scriptCandidate.Length - 2); } @@ -78,22 +80,16 @@ namespace Microsoft.DotNet.Cli.Utils // scriptCandidate may be a path relative to the project root. If so, likely will not be found // in the $PATH; add ./ to let bash know where to look. var prefix = Path.IsPathRooted(scriptCandidate) ? string.Empty : "./"; - scriptArguments[0] = $"{prefix}{scriptCandidate}"; + var quote = surroundWithQuotes ? "\"" : string.Empty; + scriptArguments[0] = $"{ quote }{ prefix }{ scriptCandidate }{ quote }"; } // Always use /usr/bin/env bash -c in order to support redirection and so on; similar to Windows case. // Unlike Windows, must escape quotation marks within the newly-quoted string. - - // TODO change this back to original, not doing anything special for bash - var bashArgs = ArgumentEscaper.EscapeArgArrayForBash(scriptArguments); - - List concatenatedArgs = new List(); - concatenatedArgs.Add("/usr/bin/env"); - concatenatedArgs.Add("bash"); - concatenatedArgs.Add("-c"); - concatenatedArgs.AddRange(bashArgs); - - scriptArguments = concatenatedArgs.ToArray(); + scriptArguments = new[] { "/usr/bin/env", "bash", "-c", "\"" } + .Concat(scriptArguments.Select(argument => argument.Replace("\"", "\\\""))) + .Concat(new[] { "\"" }) + .ToArray(); } return Command.Create(scriptArguments.FirstOrDefault(), scriptArguments.Skip(1), useComSpec: useComSpec) diff --git a/src/Microsoft.DotNet.Compiler.Common/AssemblyInfoFileGenerator.cs b/src/Microsoft.DotNet.Compiler.Common/AssemblyInfoFileGenerator.cs index ab24413fb..96f07be9b 100644 --- a/src/Microsoft.DotNet.Compiler.Common/AssemblyInfoFileGenerator.cs +++ b/src/Microsoft.DotNet.Compiler.Common/AssemblyInfoFileGenerator.cs @@ -22,7 +22,7 @@ namespace Microsoft.DotNet.Cli.Compiler.Common var existingAttributes = new List(); foreach (var sourceFile in sourceFiles) { - var tree = CSharpSyntaxTree.ParseText(File.ReadAllText(sourceFile)); + var tree = CSharpSyntaxTree.ParseText(File.ReadAllText(sourceFile.Trim('"'))); var root = tree.GetRoot(); // assembly attributes can be only on first level diff --git a/src/Microsoft.DotNet.Tools.Builder/CompileContext.cs b/src/Microsoft.DotNet.Tools.Builder/CompileContext.cs index eea02115a..d64cdd395 100644 --- a/src/Microsoft.DotNet.Tools.Builder/CompileContext.cs +++ b/src/Microsoft.DotNet.Tools.Builder/CompileContext.cs @@ -125,7 +125,7 @@ namespace Microsoft.DotNet.Tools.Build if (!newInputs.Any()) { - Reporter.Output.WriteLine($"\nProject {project.ProjectName()} was previoulsy compiled. Skipping compilation."); + Reporter.Output.WriteLine($"\nProject {project.ProjectName()} was previously compiled. Skipping compilation."); return false; } diff --git a/src/Microsoft.DotNet.Tools.Compiler.Csc/Program.cs b/src/Microsoft.DotNet.Tools.Compiler.Csc/Program.cs index 8e67b1afd..28183205b 100644 --- a/src/Microsoft.DotNet.Tools.Compiler.Csc/Program.cs +++ b/src/Microsoft.DotNet.Tools.Compiler.Csc/Program.cs @@ -79,26 +79,28 @@ namespace Microsoft.DotNet.Tools.Compiler.Csc return returnCode; } - var translated = TranslateCommonOptions(commonOptions, outputName); + var translated = TranslateCommonOptions(commonOptions, outputName.Trim('"')); var allArgs = new List(translated); allArgs.AddRange(GetDefaultOptions()); // Generate assembly info - var assemblyInfo = Path.Combine(tempOutDir, $"dotnet-compile.assemblyinfo.cs"); + var tempOutputStrippedSpaces = tempOutDir.Trim('"'); + var assemblyInfo = Path.Combine(tempOutputStrippedSpaces, $"dotnet-compile.assemblyinfo.cs"); + File.WriteAllText(assemblyInfo, AssemblyInfoFileGenerator.Generate(assemblyInfoOptions, sources)); allArgs.Add($"\"{assemblyInfo}\""); if (outputName != null) { - allArgs.Add($"-out:\"{outputName}\""); + allArgs.Add($"-out:\"{outputName.Trim('"')}\""); } allArgs.AddRange(references.Select(r => $"-r:\"{r.Trim('"')}\"")); allArgs.AddRange(resources.Select(resource => $"-resource:{resource.Trim('"')}")); allArgs.AddRange(sources.Select(s => $"\"{s.Trim('"')}\"")); - var rsp = Path.Combine(tempOutDir, "dotnet-compile-csc.rsp"); + var rsp = Path.Combine(tempOutputStrippedSpaces, "dotnet-compile-csc.rsp"); File.WriteAllLines(rsp, allArgs, Encoding.UTF8); @@ -192,7 +194,7 @@ namespace Microsoft.DotNet.Tools.Compiler.Csc if (options.GenerateXmlDocumentation == true) { - commonArgs.Add($"-doc:{Path.ChangeExtension(outputName, "xml")}"); + commonArgs.Add($"-doc:\"{Path.ChangeExtension(outputName.Trim('"'), "xml")}\""); } if (options.EmitEntryPoint != true) diff --git a/src/Microsoft.DotNet.Tools.Compiler/Program.cs b/src/Microsoft.DotNet.Tools.Compiler/Program.cs index 697f2b835..7e3369006 100644 --- a/src/Microsoft.DotNet.Tools.Compiler/Program.cs +++ b/src/Microsoft.DotNet.Tools.Compiler/Program.cs @@ -207,8 +207,8 @@ namespace Microsoft.DotNet.Tools.Compiler // Assemble args var compilerArgs = new List() { - $"--temp-output:{intermediateOutputPath}", - $"--out:{outputName}" + $"--temp-output:\"{intermediateOutputPath}\"", + $"--out:\"{outputName}\"" }; var compilationOptions = CompilerUtil.ResolveCompilationOptions(context, args.ConfigValue); @@ -258,7 +258,7 @@ namespace Microsoft.DotNet.Tools.Compiler writer.Write(dependencyContext, fileStream); } - compilerArgs.Add($"--resource:\"{depsJsonFile}\",{context.ProjectFile.Name}.deps.json"); + compilerArgs.Add($"--resource:\"{depsJsonFile},{context.ProjectFile.Name}.deps.json\""); var refsFolder = Path.Combine(outputPath, "refs"); if (Directory.Exists(refsFolder)) @@ -440,11 +440,11 @@ namespace Microsoft.DotNet.Tools.Compiler return false; } - compilerArgs.Add($"--resource:\"{resgenFile.OutputFile}\",{Path.GetFileName(resgenFile.MetadataName)}"); + compilerArgs.Add($"--resource:\"{resgenFile.OutputFile},{Path.GetFileName(resgenFile.MetadataName)}\""); } else { - compilerArgs.Add($"--resource:\"{resgenFile.InputFile}\",{Path.GetFileName(resgenFile.MetadataName)}"); + compilerArgs.Add($"--resource:\"{resgenFile.InputFile},{Path.GetFileName(resgenFile.MetadataName)}\""); } } diff --git a/src/Microsoft.Extensions.Testing.Abstractions/project.json b/src/Microsoft.Extensions.Testing.Abstractions/project.json index 7d9ed8ae0..1ffb5ef8d 100644 --- a/src/Microsoft.Extensions.Testing.Abstractions/project.json +++ b/src/Microsoft.Extensions.Testing.Abstractions/project.json @@ -12,7 +12,7 @@ "dependencies": { "Newtonsoft.Json": "7.0.1", "Microsoft.DotNet.ProjectModel": "1.0.0-*", - "Microsoft.Extensions.Logging.Abstractions": "1.0.0-rc2-16023", + "Microsoft.Extensions.Logging.Abstractions": "1.0.0-rc2-16011", "NETStandard.Library": "1.0.0-rc2-23704", "System.Resources.ResourceManager": "4.0.1-rc2-23704", "System.Runtime.Serialization.Primitives": "4.1.0-rc2-23704" diff --git a/test/FSharpTestProjects/CompileFail/project.json b/test/FSharpTestProjects/CompileFail/project.json index 682cb3cd3..69e62522b 100644 --- a/test/FSharpTestProjects/CompileFail/project.json +++ b/test/FSharpTestProjects/CompileFail/project.json @@ -9,9 +9,9 @@ ], "dependencies": { "Microsoft.FSharp.Core.netcore": "1.0.0-alpha-151221", - "Microsoft.NETCore.ConsoleHost": "1.0.0-23428", - "Microsoft.NETCore.Runtime": "1.0.1-23428", - "System.Console": "4.0.0-beta-23109" + "Microsoft.NETCore.ConsoleHost": "1.0.0-rc2-23704", + "Microsoft.NETCore.Runtime": "1.0.1-rc2-23704", + "System.Console": "4.0.0-rc2-23704" }, "frameworks": { diff --git a/test/Microsoft.DotNet.Tools.Builder.Tests/IncrementalTestBase.cs b/test/Microsoft.DotNet.Tools.Builder.Tests/IncrementalTestBase.cs index 1d1751ed5..e2defc0a5 100644 --- a/test/Microsoft.DotNet.Tools.Builder.Tests/IncrementalTestBase.cs +++ b/test/Microsoft.DotNet.Tools.Builder.Tests/IncrementalTestBase.cs @@ -75,7 +75,7 @@ namespace Microsoft.DotNet.Tools.Builder.Tests protected static void AssertProjectSkipped(string skippedProject, CommandResult buildResult) { - Assert.Contains($"Project {skippedProject} was previoulsy compiled. Skipping compilation.", buildResult.StdOut, StringComparison.OrdinalIgnoreCase); + Assert.Contains($"Project {skippedProject} was previously compiled. Skipping compilation.", buildResult.StdOut, StringComparison.OrdinalIgnoreCase); } protected static void AssertProjectCompiled(string rebuiltProject, CommandResult buildResult) diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs index f236e8a55..be35cbfca 100644 --- a/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs @@ -44,10 +44,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities var commandPath = Env.GetCommandPath(_command, ".exe", ".cmd", "") ?? Env.GetCommandPathFromAppBase(AppContext.BaseDirectory, _command, ".exe", ".cmd", ""); - - Console.Write("command"); - Console.WriteLine(commandPath); - + var stdOut = new StreamForwarder(); var stdErr = new StreamForwarder(); diff --git a/test/TestProjects/TestApp/project.json b/test/TestProjects/TestApp/project.json index 2e8f39167..b12a3990b 100644 --- a/test/TestProjects/TestApp/project.json +++ b/test/TestProjects/TestApp/project.json @@ -5,7 +5,7 @@ }, "dependencies": { - "TestLibrary": "1.0.0-*", + "TestLibrary": { "target":"project"}, "NETStandard.Library": "1.0.0-rc2-23704", "Microsoft.NETCore.Platforms": "1.0.1-rc2-23704"