diff --git a/src/Compilers/CSharp/csc/Program.cs b/src/Compilers/CSharp/csc/Program.cs index e7c3241c53210..e7050513c6fc7 100644 --- a/src/Compilers/CSharp/csc/Program.cs +++ b/src/Compilers/CSharp/csc/Program.cs @@ -28,7 +28,11 @@ public static int Main(string[] args) private static int MainCore(string[] args) { - using var logger = new CompilerServerLogger($"csc {Process.GetCurrentProcess().Id}"); + using var logger = new CompilerServerLogger( + $"csc {Process.GetCurrentProcess().Id}", + loggingFilePath: null, + Environment.GetEnvironmentVariable, + static path => path); #if BOOTSTRAP ExitingTraceListener.Install(logger); diff --git a/src/Compilers/Core/MSBuildTask/Csc.cs b/src/Compilers/Core/MSBuildTask/Csc.cs index fda13d7b791c4..ef0b227cd1715 100644 --- a/src/Compilers/Core/MSBuildTask/Csc.cs +++ b/src/Compilers/Core/MSBuildTask/Csc.cs @@ -24,6 +24,7 @@ namespace Microsoft.CodeAnalysis.BuildTasks /// should be significantly faster with larger projects and have a smaller memory /// footprint. /// + [MSBuildMultiThreadableTask] public class Csc : ManagedCompiler { #region Properties diff --git a/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs b/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs index d97829463bb37..04685309931c2 100644 --- a/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs +++ b/src/Compilers/Core/MSBuildTask/ManagedCompiler.cs @@ -506,7 +506,11 @@ public string GeneratePathToTool() protected override int ExecuteTool(string pathToTool, string responseFileCommands, string commandLineCommands) { - using var innerLogger = new CompilerServerLogger($"MSBuild {Process.GetCurrentProcess().Id}"); + using var innerLogger = new CompilerServerLogger( + $"MSBuild {Process.GetCurrentProcess().Id}", + loggingFilePath: null, + this.TaskEnvironment.GetEnvironmentVariable, + path => this.TaskEnvironment.GetAbsolutePath(path).Value); var logger = new TaskCompilerServerLogger(Log, innerLogger); return ExecuteTool(pathToTool, responseFileCommands, commandLineCommands, logger); } @@ -553,7 +557,10 @@ internal int ExecuteTool(string pathToTool, string responseFileCommands, string // commandLineCommands (the parameter) may have been mucked with // (to support using the dotnet cli) var buildRequestArguments = GenerateCommandLineArgsList(responseFileCommands); - CompilerOptionParseUtilities.PrependFeatureFlagFromEnvironment(buildRequestArguments, logger.Log); + CompilerOptionParseUtilities.PrependFeatureFlagFromEnvironment( + buildRequestArguments, + logger.Log, + this.TaskEnvironment.GetEnvironmentVariable); var buildRequest = BuildServerConnection.CreateBuildRequest( requestId, Language, @@ -571,8 +578,10 @@ internal int ExecuteTool(string pathToTool, string responseFileCommands, string buildRequest, pipeName, clientDirectory, - logger: logger, - cancellationToken: _sharedCompileCts.Token); + this.TaskEnvironment.GetEnvironmentVariable, + BuildServerConnection.CreateEnvironmentVariableSnapshot(this.TaskEnvironment.GetEnvironmentVariables()), + logger, + _sharedCompileCts.Token); responseTask.Wait(_sharedCompileCts.Token); @@ -628,12 +637,12 @@ public override void Cancel() /// private string CurrentDirectoryToUse() { - // ToolTask has a method for this. But it may return null. Use the process directory - // if ToolTask didn't override. MSBuild uses the process directory. + // ToolTask has a method for this. But it may return null. Use the task's project + // directory if ToolTask didn't override. This resolves against the task's execution environment. string workingDirectory = GetWorkingDirectory(); if (string.IsNullOrEmpty(workingDirectory)) { - workingDirectory = Directory.GetCurrentDirectory(); + workingDirectory = this.TaskEnvironment.ProjectDirectory; } return workingDirectory; } @@ -643,8 +652,8 @@ private string CurrentDirectoryToUse() /// private string? LibDirectoryToUse() { - // First check the real environment. - string? libDirectory = Environment.GetEnvironmentVariable("LIB"); + // First check the task environment. + string? libDirectory = this.TaskEnvironment.GetEnvironmentVariable("LIB"); // Now go through additional environment variables. string[] additionalVariables = EnvironmentVariables; @@ -1098,7 +1107,7 @@ private void NormalizePaths(ITaskItem[]? taskItems) foreach (var item in taskItems) { - item.ItemSpec = Utilities.GetFullPathNoThrow(item.ItemSpec); + item.ItemSpec = Utilities.GetFullPathNoThrow(item.ItemSpec, TaskEnvironment); } } @@ -1219,10 +1228,12 @@ protected bool CheckAllReferencesExistOnDisk() foreach (ITaskItem reference in References) { - if (!File.Exists(reference.ItemSpec)) + var itemSpec = reference.ItemSpec; + + if (string.IsNullOrEmpty(itemSpec) || !File.Exists(this.TaskEnvironment.GetAbsolutePath(itemSpec).Value)) { success = false; - Log.LogErrorWithCodeFromResources("General_ReferenceDoesNotExist", reference.ItemSpec); + Log.LogErrorWithCodeFromResources("General_ReferenceDoesNotExist", itemSpec); } } diff --git a/src/Compilers/Core/MSBuildTask/ManagedToolTask.cs b/src/Compilers/Core/MSBuildTask/ManagedToolTask.cs index a9b4353e12587..b54e06e6d9e4a 100644 --- a/src/Compilers/Core/MSBuildTask/ManagedToolTask.cs +++ b/src/Compilers/Core/MSBuildTask/ManagedToolTask.cs @@ -167,7 +167,9 @@ protected sealed override string GenerateFullPathToTool() // which means `ToolExe` is not really overridden by user (yes, the user sets it but basically to its default value). ToolExe = null; - return UseAppHost ? PathToBuiltInTool : RuntimeHostInfo.GetDotNetPathOrDefault(); + return UseAppHost + ? PathToBuiltInTool + : RuntimeHostInfo.GetDotNetPathOrDefault(this.TaskEnvironment.GetEnvironmentVariable); } return Path.Combine(ToolPath ?? "", ToolExe); @@ -319,20 +321,24 @@ protected override bool ValidateParameters() { // Set DOTNET_ROOT so that the apphost executables launch properly. // Unset all other DOTNET_ROOT* variables so for example DOTNET_ROOT_X64 does not override ours. - if (IsBuiltinToolRunningOnCoreClr && RuntimeHostInfo.GetToolDotNetRoot(Log.LogMessage) is { } dotNetRoot) + if (IsBuiltinToolRunningOnCoreClr && + RuntimeHostInfo.GetToolDotNetRoot( + this.TaskEnvironment.GetEnvironmentVariable, + Log.LogMessage) is { } dotNetRoot) { Log.LogMessage("Setting {0} to '{1}'", RuntimeHostInfo.DotNetRootEnvironmentName, dotNetRoot); EnvironmentVariables = [ .. EnvironmentVariables?.Where(static e => !e.StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase)) ?? [], - .. Environment.GetEnvironmentVariables().Cast() - .Where(e => ((string)e.Key).StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase)) + .. this.TaskEnvironment.GetEnvironmentVariables() + .Where(e => e.Key.StartsWith(RuntimeHostInfo.DotNetRootEnvironmentName, StringComparison.OrdinalIgnoreCase)) .Select(e => $"{e.Key}="), $"{RuntimeHostInfo.DotNetRootEnvironmentName}={dotNetRoot}", ]; } - if (RuntimeHostInfo.ShouldDisableTieredCompilation && Environment.GetEnvironmentVariable(RuntimeHostInfo.DotNetTieredCompilationEnvironmentName) == null) + if (RuntimeHostInfo.ShouldDisableTieredCompilation && + this.TaskEnvironment.GetEnvironmentVariable(RuntimeHostInfo.DotNetTieredCompilationEnvironmentName) == null) { var value = "0"; Log.LogMessage("Setting {0} to '{1}'", RuntimeHostInfo.DotNetTieredCompilationEnvironmentName, value); diff --git a/src/Compilers/Core/MSBuildTask/Utilities.cs b/src/Compilers/Core/MSBuildTask/Utilities.cs index 84782cf307084..686700391ebc1 100644 --- a/src/Compilers/Core/MSBuildTask/Utilities.cs +++ b/src/Compilers/Core/MSBuildTask/Utilities.cs @@ -119,6 +119,16 @@ internal static string GetFullPathNoThrow(string path) return path; } + internal static string GetFullPathNoThrow(string path, TaskEnvironment taskEnvironment) + { + try + { + path = Path.GetFullPath(taskEnvironment.GetAbsolutePath(path).Value); + } + catch (Exception e) when (IsIoRelatedException(e)) { } + return path; + } + internal static bool TryCombine(string path1, string path2, [NotNullWhen(returnValue: true)] out string? combined) { try diff --git a/src/Compilers/Core/MSBuildTaskTests/CscTests.cs b/src/Compilers/Core/MSBuildTaskTests/CscTests.cs index 3a00e57c87e20..0cff72d98bae5 100644 --- a/src/Compilers/Core/MSBuildTaskTests/CscTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/CscTests.cs @@ -3,7 +3,9 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.IO; +using Microsoft.Build.Framework; using Microsoft.CodeAnalysis.BuildTasks.UnitTests.TestUtilities; using Roslyn.Test.Utilities; using Roslyn.Utilities; @@ -513,11 +515,30 @@ public void BuiltInToolExe(bool useAppHost, bool setToolExe) } else { - AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(), csc.GeneratePathToTool()); + AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(Environment.GetEnvironmentVariable), csc.GeneratePathToTool()); AssertEx.Equal(RuntimeHostInfo.GetDotNetExecCommandLine(csc.PathToBuiltInTool, ""), csc.GenerateCommandLineContents()); } } + [Fact] + public void BuiltInToolUsesTaskEnvironmentDotNetHostPath() + { + var projectDirectory = Temp.CreateDirectory(); + var dotNetHost = projectDirectory.CreateFile("dotnet-host"); + var csc = new Csc + { + UseAppHost_TestOnly = false, + TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment( + projectDirectory.Path, + new Dictionary + { + [RuntimeHostInfo.DotNetHostPathEnvironmentName] = Path.GetFileName(dotNetHost.Path), + }), + }; + + Assert.Equal(Path.GetFileName(dotNetHost.Path), csc.GeneratePathToTool()); + } + [Fact] public void EditorConfig() { @@ -673,6 +694,76 @@ void parseRef(string refText, string alias) } } + /// + /// The multithreaded task migration requires relative references to be resolved against the + /// task's rather than the shared process + /// working directory. Two task instances pointed at different project directories must resolve + /// the same relative reference independently. + /// + [Fact] + public void ReferenceExistenceResolvesAgainstTaskProjectDirectory() + { + var directoryWithReference = Temp.CreateDirectory(); + var directoryWithoutReference = Temp.CreateDirectory(); + directoryWithReference.CreateFile("ref.dll"); + + var taskInDirectoryWithReference = new TestableCsc() + { + BuildEngine = new MockEngine(TestOutputHelper), + TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(directoryWithReference.Path), + References = MSBuildUtil.CreateTaskItems("ref.dll"), + }; + var taskInDirectoryWithoutReference = new TestableCsc() + { + BuildEngine = new MockEngine(TestOutputHelper), + TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(directoryWithoutReference.Path), + References = MSBuildUtil.CreateTaskItems("ref.dll"), + }; + + Assert.True(taskInDirectoryWithReference.CheckReferences()); + Assert.False(taskInDirectoryWithoutReference.CheckReferences()); + } + + private sealed class TestableCsc : Csc + { + public bool CheckReferences() => CheckAllReferencesExistOnDisk(); + } + + /// + /// An empty reference must be reported as a missing reference + /// (matching the pre-migration File.Exists behavior) rather than throwing from + /// . + /// + [Fact] + public void EmptyReferenceItemSpecIsReportedAsMissingWithoutThrowing() + { + var engine = new MockEngine(TestOutputHelper); + var task = new TestableCsc() + { + BuildEngine = engine, + References = MSBuildUtil.CreateTaskItems(""), + }; + + Assert.False(task.CheckReferences()); + Assert.Contains("MSB3104", engine.Log); + } + + [Fact] + public void InvalidReferencePathIsReportedAsMissingWithoutThrowing() + { + var projectDirectory = Temp.CreateDirectory(); + var engine = new MockEngine(TestOutputHelper); + var task = new TestableCsc() + { + BuildEngine = engine, + TaskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment(projectDirectory.Path), + References = MSBuildUtil.CreateTaskItems("bad|ref.dll"), + }; + + Assert.False(task.CheckReferences()); + Assert.Contains("MSB3104", engine.Log); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/79907")] public void StdLib() { diff --git a/src/Compilers/Core/MSBuildTaskTests/ManagedCompilerGlobalCacheTests.cs b/src/Compilers/Core/MSBuildTaskTests/ManagedCompilerGlobalCacheTests.cs index 05f243fd14b92..41f4d620ee5f8 100644 --- a/src/Compilers/Core/MSBuildTaskTests/ManagedCompilerGlobalCacheTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/ManagedCompilerGlobalCacheTests.cs @@ -4,7 +4,6 @@ #if NET -using System; using System.Collections.Generic; using System.IO; using Microsoft.CodeAnalysis.CSharp; @@ -190,5 +189,6 @@ environmentCachePath is null ? VisualBasicCommandLineParser.Default.Parse(arguments, Directory.GetCurrentDirectory(), sdkDirectory: null, additionalReferenceDirectories: null).ParseOptions.Features : CSharpCommandLineParser.Default.Parse(arguments, baseDirectory: Directory.GetCurrentDirectory(), sdkDirectory: null, additionalReferenceDirectories: null).ParseOptions.Features; } + } #endif diff --git a/src/Compilers/Core/MSBuildTaskTests/RuntimeHostInfoTests.cs b/src/Compilers/Core/MSBuildTaskTests/RuntimeHostInfoTests.cs index e639e27200059..47f8f8fc5be22 100644 --- a/src/Compilers/Core/MSBuildTaskTests/RuntimeHostInfoTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/RuntimeHostInfoTests.cs @@ -2,8 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; +using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; +using Microsoft.Build.Framework; using Microsoft.CodeAnalysis.CommandLine; using Microsoft.CodeAnalysis.Test.Utilities; using Roslyn.Test.Utilities; @@ -50,7 +53,7 @@ public void DotNetInPath() new(RuntimeHostInfo.DotNetHostPathEnvironmentName, ""), new(RuntimeHostInfo.DotNetExperimentalHostPathEnvironmentName, ""), ], - () => RuntimeHostInfo.GetToolDotNetRoot(_output.WriteLine)); + () => RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, _output.WriteLine)); Assert.NotNull(result); AssertEx.Equal(NormalizePath(globalDotNetDir.Path), NormalizePath(result)); @@ -65,7 +68,7 @@ public void DotNetInPath_None() new(RuntimeHostInfo.DotNetHostPathEnvironmentName, ""), new(RuntimeHostInfo.DotNetExperimentalHostPathEnvironmentName, ""), ], - () => RuntimeHostInfo.GetToolDotNetRoot(_output.WriteLine)); + () => RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, _output.WriteLine)); Assert.Null(result); } @@ -89,11 +92,47 @@ public void DotNetInPath_Symlinked() new(RuntimeHostInfo.DotNetHostPathEnvironmentName, ""), new(RuntimeHostInfo.DotNetExperimentalHostPathEnvironmentName, ""), ], - () => RuntimeHostInfo.GetToolDotNetRoot(_output.WriteLine)); + () => RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, _output.WriteLine)); Assert.NotNull(result); AssertEx.Equal(NormalizePath(globalDotNetDir.Path), NormalizePath(result)); } + + [Fact] + public void DotNetInTaskEnvironmentPath() + { + var projectDirectory = Temp.CreateDirectory(); + var binDirectory = projectDirectory.CreateDirectory("bin"); + var dotNetPath = binDirectory.CreateFile($"dotnet{PlatformInformation.ExeExtension}").Path; + var taskEnvironment = TaskEnvironment.CreateWithProjectDirectoryAndEnvironment( + projectDirectory.Path, + new Dictionary + { + ["PATH"] = binDirectory.Path, + [RuntimeHostInfo.DotNetHostPathEnvironmentName] = "", + [RuntimeHostInfo.DotNetExperimentalHostPathEnvironmentName] = "", + }); + + var result = RuntimeHostInfo.GetDotNetPathOrDefault(taskEnvironment.GetEnvironmentVariable); + + Assert.Equal(dotNetPath, result); + } + + [Fact] + public void DotNetInPath_SkipsRelativePaths() + { + var result = RuntimeHostInfo.GetDotNetPathOrDefault(name => name switch + { + "PATH" => PlatformInformation.IsWindows + ? @"relative;C:drive-relative;\root-relative" + : "relative", + RuntimeHostInfo.DotNetHostPathEnvironmentName => "", + RuntimeHostInfo.DotNetExperimentalHostPathEnvironmentName => "", + _ => null, + }); + + Assert.Equal($"dotnet{PlatformInformation.ExeExtension}", result); + } } #if !NET diff --git a/src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs b/src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs index 74b4b0994650b..63f41e59a7d4e 100644 --- a/src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs +++ b/src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs @@ -133,7 +133,11 @@ private static async Task ShutdownCompilerServerAsync(ProcessResult result, stri { var pipeName = Regex.Match(result.Output, @"Named pipe '([^']+)' connected").Groups[1].Value; AssertEx.Equal(sharedCompilationId, pipeName); - using var logger = new CompilerServerLogger("test"); + using var logger = new CompilerServerLogger( + "test", + loggingFilePath: null, + Environment.GetEnvironmentVariable, + static path => path); await BuildServerConnection.RunServerShutdownRequestAsync( pipeName, timeoutOverride: null, diff --git a/src/Compilers/Core/MSBuildTaskTests/VbcTests.cs b/src/Compilers/Core/MSBuildTaskTests/VbcTests.cs index bbdf6dff60322..f21c6d215e873 100644 --- a/src/Compilers/Core/MSBuildTaskTests/VbcTests.cs +++ b/src/Compilers/Core/MSBuildTaskTests/VbcTests.cs @@ -454,7 +454,7 @@ public void BuiltInToolExe(bool useAppHost, bool setToolExe) } else { - AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(), vbc.GeneratePathToTool()); + AssertEx.Equal(RuntimeHostInfo.GetDotNetPathOrDefault(Environment.GetEnvironmentVariable), vbc.GeneratePathToTool()); AssertEx.Equal(RuntimeHostInfo.GetDotNetExecCommandLine(vbc.PathToBuiltInTool, ""), vbc.GenerateCommandLineContents()); } } diff --git a/src/Compilers/Core/Portable/InternalUtilities/CompilerOptionParseUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/CompilerOptionParseUtilities.cs index 47ddc55ff1dce..a7d6e1c949657 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/CompilerOptionParseUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/CompilerOptionParseUtilities.cs @@ -37,9 +37,15 @@ public static void ParseFeatures(IDictionary builder, List arguments, Action? log = null) + internal static void PrependFeatureFlagFromEnvironment(List arguments, Action? log = null) => + PrependFeatureFlagFromEnvironment(arguments, log, Environment.GetEnvironmentVariable); + + internal static void PrependFeatureFlagFromEnvironment( + List arguments, + Action? log, + Func getEnvironmentVariable) { - if (Environment.GetEnvironmentVariable(CachePathEnvironmentVariable) is not { Length: > 0 } cachePath) + if (getEnvironmentVariable(CachePathEnvironmentVariable) is not { Length: > 0 } cachePath) { return; } diff --git a/src/Compilers/Server/VBCSCompiler/VBCSCompiler.cs b/src/Compilers/Server/VBCSCompiler/VBCSCompiler.cs index 66e905e87a57d..b25476281f4df 100644 --- a/src/Compilers/Server/VBCSCompiler/VBCSCompiler.cs +++ b/src/Compilers/Server/VBCSCompiler/VBCSCompiler.cs @@ -19,7 +19,11 @@ public static int Main(string[] args) return CommonCompiler.Failed; } - using var logger = new CompilerServerLogger($"VBCSCompiler {Process.GetCurrentProcess().Id}", options.LogFilePath); + using var logger = new CompilerServerLogger( + $"VBCSCompiler {Process.GetCurrentProcess().Id}", + options.LogFilePath, + Environment.GetEnvironmentVariable, + static path => path); #if BOOTSTRAP ExitingTraceListener.Install(logger); diff --git a/src/Compilers/Server/VBCSCompilerTests/BuildServerConnectionTests.cs b/src/Compilers/Server/VBCSCompilerTests/BuildServerConnectionTests.cs index 33180c0eb0e89..91030a706e4f4 100644 --- a/src/Compilers/Server/VBCSCompilerTests/BuildServerConnectionTests.cs +++ b/src/Compilers/Server/VBCSCompilerTests/BuildServerConnectionTests.cs @@ -3,7 +3,9 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Diagnostics; +using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CommandLine; @@ -183,10 +185,14 @@ public void GetServerEnvironmentVariables_IncludesDotNetRoot() // without modifying the current process environment var currentEnvironment = Environment.GetEnvironmentVariables(); var originalDotNetRoot = (string?)currentEnvironment[RuntimeHostInfo.DotNetRootEnvironmentName]; + var snapshot = BuildServerConnection.CreateEnvironmentVariableSnapshot(currentEnvironment); - var envVars = BuildServerConnection.GetServerEnvironmentVariables(currentEnvironment); + var envVars = BuildServerConnection.GetServerEnvironmentVariables( + Environment.GetEnvironmentVariable, + snapshot); - if (BuildServerConnection.IsBuiltinToolRunningOnCoreClr && RuntimeHostInfo.GetToolDotNetRoot(Logger.Log) is { } dotNetRoot) + if (BuildServerConnection.IsBuiltinToolRunningOnCoreClr && + RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, Logger.Log) is { } dotNetRoot) { // Should have environment variables including DOTNET_ROOT Assert.NotNull(envVars); @@ -226,9 +232,12 @@ public void GetServerEnvironmentVariables_ExcludesDotNetRootVariants() testEnvironment[testEnvVar] = "test_value"; } - var envVars = BuildServerConnection.GetServerEnvironmentVariables(testEnvironment); + var envVars = BuildServerConnection.GetServerEnvironmentVariables( + name => (string?)testEnvironment[name], + BuildServerConnection.CreateEnvironmentVariableSnapshot(testEnvironment)); - if (BuildServerConnection.IsBuiltinToolRunningOnCoreClr && RuntimeHostInfo.GetToolDotNetRoot(Logger.Log) != null) + if (BuildServerConnection.IsBuiltinToolRunningOnCoreClr && + RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, Logger.Log) != null) { Assert.NotNull(envVars); @@ -240,5 +249,67 @@ public void GetServerEnvironmentVariables_ExcludesDotNetRootVariants() } } } + + [Fact] + public void GetServerEnvironmentVariables_UsesProvidedSnapshot() + { + string? processOnlyKey = null; + foreach (System.Collections.DictionaryEntry entry in Environment.GetEnvironmentVariables()) + { + var key = (string)entry.Key; + if (!string.Equals(key, "TASK_ONLY_VARIABLE", StringComparison.OrdinalIgnoreCase)) + { + processOnlyKey = key; + break; + } + } + + Assert.NotNull(processOnlyKey); + + var environment = new Dictionary + { + ["TASK_ONLY_VARIABLE"] = "task-value", + }; + + var result = BuildServerConnection.GetServerEnvironmentVariables( + name => environment.TryGetValue(name, out var value) ? value : null, + [new KeyValuePair("TASK_ONLY_VARIABLE", "task-value")]); + + Assert.NotNull(result); + Assert.Equal("task-value", result["TASK_ONLY_VARIABLE"]); + Assert.False(result.ContainsKey(processOnlyKey)); + } + + [Fact] + public void GetServerProcessInfo_UsesProvidedDotNetPath() + { + var clientDirectory = TempRoot.CreateDirectory().Path; + var dotNetPath = Path.Combine(clientDirectory, "task-dotnet"); + + var result = BuildServerConnection.GetServerProcessInfo( + clientDirectory, + "pipe", + name => name == RuntimeHostInfo.DotNetHostPathEnvironmentName ? dotNetPath : null); + + Assert.Equal(dotNetPath, result.processFilePath); + } + + [Fact] + public void CompilerServerLogger_UsesInjectedEnvironmentAndPath() + { + var projectDirectory = TempRoot.CreateDirectory(); + var logDirectory = projectDirectory.CreateDirectory("logs"); + + using (var logger = new CompilerServerLogger( + "test", + loggingFilePath: null, + name => name == CompilerServerLogger.EnvironmentVariableName ? "logs" : null, + path => Path.Combine(projectDirectory.Path, path))) + { + logger.Log("message"); + } + + Assert.Single(Directory.GetFiles(logDirectory.Path)); + } } } diff --git a/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs b/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs index 5ae914e6c5c3c..e14618e59e1c5 100644 --- a/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs +++ b/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs @@ -129,7 +129,10 @@ public async Task IncorrectServerHashReturnsIncorrectHashResponse() [WorkItem(33452, "https://github.com/dotnet/roslyn/issues/33452")] public void QuotePipeName_Desktop() { - var serverInfo = BuildServerConnection.GetServerProcessInfo(@"q:\tools", "name with space"); + var serverInfo = BuildServerConnection.GetServerProcessInfo( + @"q:\tools", + "name with space", + Environment.GetEnvironmentVariable); Assert.EndsWith(@"\dotnet.exe", serverInfo.processFilePath); AssertEx.Equal(@"exec ""q:\tools\VBCSCompiler.dll"" ""-pipename:name with space""", serverInfo.commandLineArguments); } @@ -141,7 +144,10 @@ public void QuotePipeName_CoreClr() var toolDir = ExecutionConditionUtil.IsWindows ? @"q:\tools" : "/tools"; - var serverInfo = BuildServerConnection.GetServerProcessInfo(toolDir, "name with space"); + var serverInfo = BuildServerConnection.GetServerProcessInfo( + toolDir, + "name with space", + Environment.GetEnvironmentVariable); var vbcsFilePath = Path.Combine(toolDir, "VBCSCompiler.dll"); AssertEx.Equal($@"exec ""{vbcsFilePath}"" ""-pipename:name with space""", serverInfo.commandLineArguments); } diff --git a/src/Compilers/Shared/BuildClient.cs b/src/Compilers/Shared/BuildClient.cs index 258a20666b7a9..19f54efdc5cff 100644 --- a/src/Compilers/Shared/BuildClient.cs +++ b/src/Compilers/Shared/BuildClient.cs @@ -194,12 +194,17 @@ private int RunLocalCompilation(string[] arguments, BuildPaths buildPaths, TextW } public static CompileOnServerFunc GetCompileOnServerFunc(ICompilerServerLogger logger) => (buildRequest, pipeName, cancellationToken) => - BuildServerConnection.RunServerBuildRequestAsync( + { + var environmentVariables = BuildServerConnection.CreateEnvironmentVariableSnapshot(Environment.GetEnvironmentVariables()); + return BuildServerConnection.RunServerBuildRequestAsync( buildRequest, pipeName, GetClientDirectory(), + Environment.GetEnvironmentVariable, + environmentVariables, logger, cancellationToken); + }; /// /// Runs the provided compilation on the server. If the compilation cannot be completed on the server then null diff --git a/src/Compilers/Shared/BuildServerConnection.cs b/src/Compilers/Shared/BuildServerConnection.cs index c81230e483dc8..7b32c730e5eb5 100644 --- a/src/Compilers/Shared/BuildServerConnection.cs +++ b/src/Compilers/Shared/BuildServerConnection.cs @@ -178,13 +178,21 @@ internal static Task RunServerBuildRequestAsync( BuildRequest buildRequest, string pipeName, string clientDirectory, + Func getEnvironmentVariable, + IReadOnlyList> environmentVariables, ICompilerServerLogger logger, CancellationToken cancellationToken) => RunServerBuildRequestAsync( buildRequest, pipeName, timeoutOverride: null, - tryCreateServerFunc: (pipeName, logger) => TryCreateServer(clientDirectory, pipeName, logger, out int _), + tryCreateServerFunc: (pipeName, logger) => TryCreateServer( + clientDirectory, + pipeName, + getEnvironmentVariable, + environmentVariables, + logger, + out int _), logger, cancellationToken); @@ -447,7 +455,10 @@ internal static async Task MonitorDisconnectAsync( } } - internal static (string processFilePath, string commandLineArguments) GetServerProcessInfo(string clientDir, string pipeName) + internal static (string processFilePath, string commandLineArguments) GetServerProcessInfo( + string clientDir, + string pipeName, + Func getEnvironmentVariable) { var processFilePath = Path.Combine(clientDir, $"VBCSCompiler{PlatformInformation.ExeExtension}"); var commandLineArgs = $@"""-pipename:{pipeName}"""; @@ -456,7 +467,7 @@ internal static (string processFilePath, string commandLineArguments) GetServerP { // Fallback to not use the apphost if it is not present (can happen in compiler toolset scenarios for example). commandLineArgs = RuntimeHostInfo.GetDotNetExecCommandLine(Path.ChangeExtension(processFilePath, ".dll"), commandLineArgs); - processFilePath = RuntimeHostInfo.GetDotNetPathOrDefault(); + processFilePath = RuntimeHostInfo.GetDotNetPathOrDefault(getEnvironmentVariable); } return (processFilePath, commandLineArgs); @@ -491,27 +502,53 @@ private static IntPtr CreateEnvironmentBlock(Dictionary environm return Marshal.StringToHGlobalUni(envBlock.ToString()); } + internal static KeyValuePair[] CreateEnvironmentVariableSnapshot(System.Collections.IDictionary currentEnvironment) + { + var result = new KeyValuePair[currentEnvironment.Count]; + var index = 0; + foreach (System.Collections.DictionaryEntry entry in currentEnvironment) + { + result[index++] = new KeyValuePair((string)entry.Key, (string?)entry.Value); + } + + return result; + } + + internal static KeyValuePair[] CreateEnvironmentVariableSnapshot( + IEnumerable> currentEnvironment) + { + var result = new List>(); + foreach (var entry in currentEnvironment) + { + result.Add(new KeyValuePair(entry.Key, entry.Value)); + } + + return result.ToArray(); + } + /// /// Gets the environment variables that should be passed to the server process. /// /// Current environment variables to use as a base /// Optional logger for logging environment variable setup - /// Dictionary of environment variables to set, or null if no custom environment is needed - internal static Dictionary? GetServerEnvironmentVariables(System.Collections.IDictionary currentEnvironment, ICompilerServerLogger? logger = null) + /// Dictionary of environment variables to set + internal static Dictionary? GetServerEnvironmentVariables( + Func getEnvironmentVariable, + IReadOnlyList> currentEnvironment, + ICompilerServerLogger? logger = null) { - string? dotNetRoot = IsBuiltinToolRunningOnCoreClr ? RuntimeHostInfo.GetToolDotNetRoot(logger is null ? null : logger.Log) : null; - - if (dotNetRoot == null && !RuntimeHostInfo.ShouldDisableTieredCompilation) - { - return null; - } + var dotNetRoot = IsBuiltinToolRunningOnCoreClr + ? RuntimeHostInfo.GetToolDotNetRoot( + getEnvironmentVariable, + logger is null ? null : logger.Log) + : null; // Start with current environment var environmentVariables = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (System.Collections.DictionaryEntry entry in currentEnvironment) + foreach (var entry in currentEnvironment) { - var key = (string)entry.Key; - var value = (string?)entry.Value; + var key = entry.Key; + var value = entry.Value; // Clear DOTNET_ROOT* variables such as DOTNET_ROOT_X64 by setting them to empty, // as we want to set our own DOTNET_ROOT and avoid conflicts @@ -548,10 +585,19 @@ private static IntPtr CreateEnvironmentBlock(Dictionary environm /// compiler server process was successful, it does not state whether the server successfully /// started or not (it could crash on startup). /// - internal static bool TryCreateServer(string clientDirectory, string pipeName, ICompilerServerLogger logger, out int processId) + internal static bool TryCreateServer( + string clientDirectory, + string pipeName, + Func getEnvironmentVariable, + IReadOnlyList> environmentVariablesSnapshot, + ICompilerServerLogger logger, + out int processId) { processId = 0; - var serverInfo = GetServerProcessInfo(clientDirectory, pipeName); + var serverInfo = GetServerProcessInfo( + clientDirectory, + pipeName, + getEnvironmentVariable); if (!File.Exists(serverInfo.processFilePath)) { @@ -560,7 +606,10 @@ internal static bool TryCreateServer(string clientDirectory, string pipeName, IC logger.Log("Attempting to create process '{0}' {1}", serverInfo.processFilePath, serverInfo.commandLineArguments); - var environmentVariables = GetServerEnvironmentVariables(Environment.GetEnvironmentVariables(), logger); + var environmentVariables = GetServerEnvironmentVariables( + getEnvironmentVariable, + environmentVariablesSnapshot, + logger); if (PlatformInformation.IsWindows) { @@ -642,6 +691,8 @@ internal static bool TryCreateServer(string clientDirectory, string pipeName, IC // Set environment variables directly on ProcessStartInfo if (environmentVariables != null) { + // Replace the inherited process environment with the caller-provided snapshot. + startInfo.EnvironmentVariables.Clear(); foreach (var kvp in environmentVariables) { startInfo.EnvironmentVariables[kvp.Key] = kvp.Value; diff --git a/src/Compilers/Shared/CompilerServerLogger.cs b/src/Compilers/Shared/CompilerServerLogger.cs index b56691a54e4e0..33a2f0b00810e 100644 --- a/src/Compilers/Shared/CompilerServerLogger.cs +++ b/src/Compilers/Shared/CompilerServerLogger.cs @@ -106,9 +106,18 @@ internal sealed class CompilerServerLogger : ICompilerServerLogger, IDisposable public bool IsLogging => _loggingStream is object; /// - /// Static class initializer that initializes logging. + /// Initializes logging using the supplied environment variable lookup and path absolutization. /// - public CompilerServerLogger(string identifier, string? loggingFilePath = null) + /// Reads the named environment variable. + /// + /// Resolves a (possibly relative) path to an absolute one before it is used for file system + /// access. + /// + public CompilerServerLogger( + string identifier, + string? loggingFilePath, + Func getEnvironmentVariable, + Func makeAbsolutePath) { _identifier = identifier; @@ -116,14 +125,19 @@ public CompilerServerLogger(string identifier, string? loggingFilePath = null) { if (loggingFilePath is null) { - loggingFilePath = Environment.GetEnvironmentVariable(EnvironmentVariableName); - // If the environment variable contains the path of a currently existing directory, - // then use a process-specific name for the log file and put it in that directory. - // Otherwise, assume that the environment variable specifies the name of the log file. - if (Directory.Exists(loggingFilePath)) + loggingFilePath = getEnvironmentVariable(EnvironmentVariableName); + if (!string.IsNullOrEmpty(loggingFilePath)) { - var processId = Process.GetCurrentProcess().Id; - loggingFilePath = Path.Combine(loggingFilePath, $"server.{processId}.log"); + loggingFilePath = makeAbsolutePath(loggingFilePath); + + // If the environment variable contains the path of a currently existing directory, + // then use a process-specific name for the log file and put it in that directory. + // Otherwise, assume that the environment variable specifies the name of the log file. + if (Directory.Exists(loggingFilePath)) + { + var processId = Process.GetCurrentProcess().Id; + loggingFilePath = Path.Combine(loggingFilePath, $"server.{processId}.log"); + } } } diff --git a/src/Compilers/Shared/RuntimeHostInfo.cs b/src/Compilers/Shared/RuntimeHostInfo.cs index 4a54e22a09798..3786bc551ec01 100644 --- a/src/Compilers/Shared/RuntimeHostInfo.cs +++ b/src/Compilers/Shared/RuntimeHostInfo.cs @@ -44,10 +44,13 @@ internal static class RuntimeHostInfo /// /// The DOTNET_ROOT that should be used when launching executable tools. /// - internal static string? GetToolDotNetRoot(Action? logger) - { - var dotNetPath = GetDotNetPathOrDefault(); + internal static string? GetToolDotNetRoot( + Func getEnvironmentVariable, + Action? logger) => + GetToolDotNetRoot(GetDotNetPathOrDefault(getEnvironmentVariable), logger); + internal static string? GetToolDotNetRoot(string dotNetPath, Action? logger) + { // Resolve symlinks to dotnet try { @@ -77,14 +80,14 @@ internal static class RuntimeHostInfo /// in the environment this tries to find "dotnet" on the PATH. In the case it is not found, /// this will return simply "dotnet". /// - internal static string GetDotNetPathOrDefault() + internal static string GetDotNetPathOrDefault(Func getEnvironmentVariable) { - if (Environment.GetEnvironmentVariable(DotNetHostPathEnvironmentName) is { Length: > 0 } pathToDotNet) + if (getEnvironmentVariable(DotNetHostPathEnvironmentName) is { Length: > 0 } pathToDotNet) { return pathToDotNet; } - if (Environment.GetEnvironmentVariable(DotNetExperimentalHostPathEnvironmentName) is { Length: > 0 } pathToDotNetExperimental) + if (getEnvironmentVariable(DotNetExperimentalHostPathEnvironmentName) is { Length: > 0 } pathToDotNetExperimental) { return pathToDotNetExperimental; } @@ -93,11 +96,16 @@ internal static string GetDotNetPathOrDefault() ? ("dotnet.exe", new char[] { ';' }) : ("dotnet", new char[] { ':' }); - var path = Environment.GetEnvironmentVariable("PATH") ?? ""; + var path = getEnvironmentVariable("PATH") ?? ""; foreach (var item in path.Split(sep, StringSplitOptions.RemoveEmptyEntries)) { try { + if (!IsPathFullyQualified(item)) + { + continue; + } + var filePath = Path.Combine(item, fileName); if (File.Exists(filePath)) { @@ -115,5 +123,26 @@ internal static string GetDotNetPathOrDefault() internal static string GetDotNetExecCommandLine(string toolFilePath, string commandLineArguments) => $@"exec ""{toolFilePath}"" {commandLineArguments}"; + + private static bool IsPathFullyQualified(string path) + { +#if NET + return Path.IsPathFullyQualified(path); +#else + if (!Path.IsPathRooted(path)) + { + return false; + } + + if (!PlatformInformation.IsWindows) + { + return true; + } + + var root = Path.GetPathRoot(path); + // Return false for rooted but not fully qualified paths (root-relative "\foo" and drive-relative "C:foo"). + return root is { Length: > 1 } && root[root.Length - 1] != Path.VolumeSeparatorChar; +#endif + } } } diff --git a/src/Compilers/VisualBasic/vbc/Program.cs b/src/Compilers/VisualBasic/vbc/Program.cs index 21fbf7ae33955..a54a1e5bb5797 100644 --- a/src/Compilers/VisualBasic/vbc/Program.cs +++ b/src/Compilers/VisualBasic/vbc/Program.cs @@ -30,7 +30,11 @@ public static int Main(string[] args) private static int MainCore(string[] args) { - using var logger = new CompilerServerLogger($"vbc {Process.GetCurrentProcess().Id}"); + using var logger = new CompilerServerLogger( + $"vbc {Process.GetCurrentProcess().Id}", + loggingFilePath: null, + Environment.GetEnvironmentVariable, + static path => path); #if BOOTSTRAP ExitingTraceListener.Install(logger); diff --git a/src/LanguageServer/roslyn-language-server/ServerExecutable.cs b/src/LanguageServer/roslyn-language-server/ServerExecutable.cs index 7c865493f97a5..31a7cee1dbe7c 100644 --- a/src/LanguageServer/roslyn-language-server/ServerExecutable.cs +++ b/src/LanguageServer/roslyn-language-server/ServerExecutable.cs @@ -87,7 +87,7 @@ private Process Start(IReadOnlyList arguments, bool suppressStandardHand // only .NET install is reachable via PATH, the child apphost would otherwise fail to start. // Point the child at the runtime that is hosting us so it launches against the // same .NET. - if (RuntimeHostInfo.GetToolDotNetRoot(logger: null) is { } dotNetRoot) + if (RuntimeHostInfo.GetToolDotNetRoot(Environment.GetEnvironmentVariable, logger: null) is { } dotNetRoot) { // Clear any inherited DOTNET_ROOT* variants (e.g. DOTNET_ROOT_X64) so they can't override the value we set. foreach (var key in startInfo.Environment.Keys diff --git a/src/Tools/Replay/Replay.cs b/src/Tools/Replay/Replay.cs index 1be4d17081362..d9e940be7f9a6 100644 --- a/src/Tools/Replay/Replay.cs +++ b/src/Tools/Replay/Replay.cs @@ -129,8 +129,19 @@ static async Task RunAsync(ReplayOptions options) Directory.CreateDirectory(options.OutputDirectory); Directory.CreateDirectory(options.TempDirectory); - using var compilerServerLogger = new CompilerServerLogger("replay", Path.Combine(options.OutputDirectory, "server.log")); - if (!BuildServerConnection.TryCreateServer(options.ClientDirectory, options.PipeName, compilerServerLogger, out int serverProcessId)) + using var compilerServerLogger = new CompilerServerLogger( + "replay", + Path.Combine(options.OutputDirectory, "server.log"), + Environment.GetEnvironmentVariable, + static path => path); + var environmentVariables = BuildServerConnection.CreateEnvironmentVariableSnapshot(Environment.GetEnvironmentVariables()); + if (!BuildServerConnection.TryCreateServer( + options.ClientDirectory, + options.PipeName, + Environment.GetEnvironmentVariable, + environmentVariables, + compilerServerLogger, + out int serverProcessId)) { throw new Exception("Failed to create server"); } @@ -149,7 +160,7 @@ static async Task RunAsync(ReplayOptions options) try { - await foreach (var buildData in BuildAllAsync(options, compilerCalls, compilerServerLogger, CancellationToken.None).ConfigureAwait(false)) + await foreach (var buildData in BuildAllAsync(options, compilerCalls, environmentVariables, compilerServerLogger, CancellationToken.None).ConfigureAwait(false)) { if (buildData.BuildResponse is not CompletedBuildResponse completedBuildResponse) { @@ -190,6 +201,7 @@ static List ReadAllCompilerCalls(string binlogPath) static async IAsyncEnumerable BuildAllAsync( ReplayOptions options, List compilerCalls, + IReadOnlyList> environmentVariables, CompilerServerLogger compilerServerLogger, [EnumeratorCancellation] CancellationToken cancellationToken) { @@ -204,7 +216,7 @@ static async IAsyncEnumerable BuildAllAsync( while (tasks.Count < maxParallel && index < compilerCalls.Count) { var compilerCall = compilerCalls[index]; - tasks.Add(BuildAsync(options, compilerCall, GetOutputName(compilerCall), compilerServerLogger, cancellationToken)); + tasks.Add(BuildAsync(options, compilerCall, GetOutputName(compilerCall), environmentVariables, compilerServerLogger, cancellationToken)); index++; } @@ -247,6 +259,7 @@ static async Task BuildAsync( ReplayOptions options, CompilerCall compilerCall, string outputName, + IReadOnlyList> environmentVariables, CompilerServerLogger compilerServerLogger, CancellationToken cancellationToken) { @@ -267,6 +280,8 @@ static async Task BuildAsync( request, options.PipeName, options.ClientDirectory, + Environment.GetEnvironmentVariable, + environmentVariables, compilerServerLogger, cancellationToken).ConfigureAwait(false); return new BuildData(compilerCall, response);