diff --git a/documentation/specs/multithreading/thread-safe-tasks.md b/documentation/specs/multithreading/thread-safe-tasks.md index cdd7dd3d7b0..a983fda8ef7 100644 --- a/documentation/specs/multithreading/thread-safe-tasks.md +++ b/documentation/specs/multithreading/thread-safe-tasks.md @@ -109,15 +109,17 @@ To prevent common thread-safety issues related to path handling, we introduce pa ```csharp namespace Microsoft.Build.Framework; -public readonly struct AbsolutePath +public readonly struct AbsolutePath : IEquatable { // Default value returns string.Empty for Path property - public string Path { get; } + public string Value { get; } internal AbsolutePath(string path, bool ignoreRootedCheck) { } public AbsolutePath(string path); // Checks Path.IsPathRooted public AbsolutePath(string path, AbsolutePath basePath) { } public static implicit operator string(AbsolutePath path) { } - public override string ToString() => Path; + public override string ToString() => Value; + + // overrides for equality and hashcode } ``` diff --git a/src/Build.UnitTests/BackEnd/TaskEnvironment_Tests.cs b/src/Build.UnitTests/BackEnd/TaskEnvironment_Tests.cs new file mode 100644 index 00000000000..1a325dcc9b8 --- /dev/null +++ b/src/Build.UnitTests/BackEnd/TaskEnvironment_Tests.cs @@ -0,0 +1,410 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Microsoft.Build.BackEnd; +using Microsoft.Build.Framework; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.UnitTests +{ + public class TaskEnvironment_Tests + { + private const string StubEnvironmentName = "Stub"; + private const string MultithreadedEnvironmentName = "Multithreaded"; + + public static TheoryData EnvironmentTypes => + new TheoryData + { + StubEnvironmentName, + MultithreadedEnvironmentName + }; + + private static TaskEnvironment CreateTaskEnvironment(string environmentType) + { + return environmentType switch + { + StubEnvironmentName => new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance), + MultithreadedEnvironmentName => new TaskEnvironment(new MultiThreadedTaskEnvironmentDriver(GetResolvedTempPath())), + _ => throw new ArgumentException($"Unknown environment type: {environmentType}") + }; + } + + /// + /// Gets the fully resolved temp directory path. On macOS, Path.GetTempPath() returns paths starting with "/var/..." + /// which is a symbolic link that resolves to "/private/var/...". + /// The StubTaskEnvironmentDriver uses Directory.SetCurrentDirectory which resolves symlinks, while + /// MultiThreadedTaskEnvironmentDriver stores the path as-is. + /// This method ensures we get the canonical path to avoid test failures when comparing paths between the two drivers. + /// + /// The fully resolved temp directory path + private static string GetResolvedTempPath() + { + string tempPath = Path.GetFullPath(Path.GetTempPath()); + + // On macOS, /tmp is typically a symbolic link to /private/tmp + // And /var is typically a symbolic link to /private/var + // We need to manually resolve this because StubTaskEnvironmentDriver (via Directory.SetCurrentDirectory) + // will resolve the symlink, but MultiThreadedTaskEnvironmentDriver will not. + // By resolving it here, we ensure both drivers operate on the same canonical path string. +#if NET5_0_OR_GREATER + bool IsMacOS = OperatingSystem.IsMacOS(); +#else + bool IsMacOS = Microsoft.Build.Framework.OperatingSystem.IsMacOS(); +#endif + if (IsMacOS) + { + // On macOS, /tmp -> /private/tmp and /var -> /private/var + if (tempPath.StartsWith("/tmp/", StringComparison.OrdinalIgnoreCase) || string.Equals(tempPath, "/tmp", StringComparison.OrdinalIgnoreCase)) + { + tempPath = "/private" + tempPath; + } + else if (tempPath.StartsWith("/var/", StringComparison.OrdinalIgnoreCase) || string.Equals(tempPath, "/var", StringComparison.OrdinalIgnoreCase)) + { + tempPath = "/private" + tempPath; + } + } + + return tempPath; + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_SetAndGetEnvironmentVariable_ShouldWork(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string testVarName = $"MSBUILD_TEST_VAR_{environmentType}_{Guid.NewGuid():N}"; + string testVarValue = $"test_value_{environmentType}"; + + try + { + taskEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + var retrievedValue = taskEnvironment.GetEnvironmentVariable(testVarName); + + retrievedValue.ShouldBe(testVarValue); + + var allVariables = taskEnvironment.GetEnvironmentVariables(); + allVariables.TryGetValue(testVarName, out string? actualValue).ShouldBeTrue(); + actualValue.ShouldBe(testVarValue); + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_SetEnvironmentVariableToNull_ShouldRemoveVariable(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string testVarName = $"MSBUILD_REMOVE_TEST_{environmentType}_{Guid.NewGuid():N}"; + string testVarValue = "value_to_remove"; + + try + { + taskEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + taskEnvironment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + + taskEnvironment.SetEnvironmentVariable(testVarName, null); + taskEnvironment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + + var allVariables = taskEnvironment.GetEnvironmentVariables(); + allVariables.TryGetValue(testVarName, out string? actualValue).ShouldBeFalse(); + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_SetEnvironment_ShouldReplaceAllVariables(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string prefix = $"MSBUILD_SET_ENV_TEST_{environmentType}_{Guid.NewGuid():N}"; + string var1Name = $"{prefix}_VAR1"; + string var2Name = $"{prefix}_VAR2"; + string var3Name = $"{prefix}_VAR3"; + + var originalEnvironment = taskEnvironment.GetEnvironmentVariables().ToDictionary(k => k.Key, v => v.Value); + + try + { + taskEnvironment.SetEnvironmentVariable(var1Name, "initial_value1"); + taskEnvironment.SetEnvironmentVariable(var2Name, "initial_value2"); + + var newEnvironment = new Dictionary + { + [var2Name] = "updated_value2", // Update existing + [var3Name] = "new_value3" // Add new + // var1Name is intentionally omitted to test removal + }; + + taskEnvironment.SetEnvironment(newEnvironment); + + taskEnvironment.GetEnvironmentVariable(var1Name).ShouldBeNull(); // Should be removed + taskEnvironment.GetEnvironmentVariable(var2Name).ShouldBe("updated_value2"); // Should be updated + taskEnvironment.GetEnvironmentVariable(var3Name).ShouldBe("new_value3"); // Should be added + } + finally + { + taskEnvironment.SetEnvironment(originalEnvironment); + Environment.SetEnvironmentVariable(var1Name, null); + Environment.SetEnvironmentVariable(var2Name, null); + Environment.SetEnvironmentVariable(var3Name, null); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_SetAndGetProjectDirectory_ShouldWork(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string originalDirectory = Directory.GetCurrentDirectory(); + string testDirectory = GetResolvedTempPath().TrimEnd(Path.DirectorySeparatorChar); + string alternateDirectory = Path.GetDirectoryName(testDirectory)!; + + try + { + // Set project directory + taskEnvironment.ProjectDirectory = new AbsolutePath(testDirectory, ignoreRootedCheck: true); + var retrievedDirectory = taskEnvironment.ProjectDirectory; + + retrievedDirectory.Value.ShouldBe(testDirectory); + + // Change to alternate directory + taskEnvironment.ProjectDirectory = new AbsolutePath(alternateDirectory, ignoreRootedCheck: true); + var newRetrievedDirectory = taskEnvironment.ProjectDirectory; + + newRetrievedDirectory.Value.ShouldBe(alternateDirectory); + + // Verify behavior differs based on environment type + if (environmentType == StubEnvironmentName) + { + // Stub should change system current directory + Directory.GetCurrentDirectory().ShouldBe(alternateDirectory); + } + else + { + // Multithreaded should not change system current directory + Directory.GetCurrentDirectory().ShouldBe(originalDirectory); + } + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_GetAbsolutePath_ShouldResolveCorrectly(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string baseDirectory = GetResolvedTempPath(); + string relativePath = Path.Combine("subdir", "file.txt"); + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + taskEnvironment.ProjectDirectory = new AbsolutePath(baseDirectory, ignoreRootedCheck: true); + + var absolutePath = taskEnvironment.GetAbsolutePath(relativePath); + + Path.IsPathRooted(absolutePath.Value).ShouldBeTrue(); + string expectedPath = Path.Combine(baseDirectory, relativePath); + absolutePath.Value.ShouldBe(expectedPath); + } + finally + { + Directory.SetCurrentDirectory(originalDirectory); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_GetAbsolutePath_WithAlreadyAbsolutePath_ShouldReturnUnchanged(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string absoluteInputPath = Path.Combine(GetResolvedTempPath(), "already", "absolute", "path.txt"); + + var resultPath = taskEnvironment.GetAbsolutePath(absoluteInputPath); + + resultPath.Value.ShouldBe(absoluteInputPath); + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_GetProcessStartInfo_ShouldConfigureCorrectly(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string testDirectory = GetResolvedTempPath(); + string testVarName = $"MSBUILD_PROCESS_TEST_{environmentType}_{Guid.NewGuid():N}"; + string testVarValue = "process_test_value"; + string originalDirectory = Directory.GetCurrentDirectory(); + + try + { + taskEnvironment.ProjectDirectory = new AbsolutePath(testDirectory, ignoreRootedCheck: true); + taskEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + + var processStartInfo = taskEnvironment.GetProcessStartInfo(); + + processStartInfo.ShouldNotBeNull(); + + if (environmentType == StubEnvironmentName) + { + // Stub should reflect system environment, but working directory should be empty + processStartInfo.WorkingDirectory.ShouldBe(string.Empty); + } + else + { + // Multithreaded should reflect isolated environment + processStartInfo.WorkingDirectory.ShouldBe(testDirectory); + } + + processStartInfo.Environment.TryGetValue(testVarName, out string? actualValue).ShouldBeTrue(); + actualValue.ShouldBe(testVarValue); + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + Directory.SetCurrentDirectory(originalDirectory); + } + } + + [Fact] + public void TaskEnvironment_StubEnvironment_ShouldAffectSystemEnvironment() + { + string testVarName = $"MSBUILD_STUB_ISOLATION_TEST_{Guid.NewGuid():N}"; + string testVarValue = "stub_test_value"; + + var stubEnvironment = new TaskEnvironment(MultiProcessTaskEnvironmentDriver.Instance); + + try + { + // Set variable in stub environment + stubEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + + // Stub should affect system environment + Environment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + stubEnvironment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + + // Remove from stub environment + stubEnvironment.SetEnvironmentVariable(testVarName, null); + + // System environment should also be affected + Environment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + stubEnvironment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + } + } + + [Theory] + [MemberData(nameof(EnvironmentTypes))] + public void TaskEnvironment_EnvironmentVariableCaseSensitivity_ShouldMatchPlatform(string environmentType) + { + var taskEnvironment = CreateTaskEnvironment(environmentType); + string testVarName = $"MSBUILD_case_TEST_{environmentType}_{Guid.NewGuid():N}"; + string testVarValue = "case_test_value"; + + try + { + taskEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + + // Test GetEnvironmentVariables() + var envVars = taskEnvironment.GetEnvironmentVariables(); + + // Test GetEnvironmentVariable() + string upperVarName = testVarName.ToUpperInvariant(); + string lowerVarName = testVarName.ToLowerInvariant(); + + if (Microsoft.Build.Framework.NativeMethods.IsWindows) + { + // On Windows, environment variables should be case-insensitive + + // Test GetEnvironmentVariables() + envVars.TryGetValue(testVarName, out string? exactValue).ShouldBeTrue(); + exactValue.ShouldBe(testVarValue); + + envVars.TryGetValue(upperVarName, out string? upperValue).ShouldBeTrue(); + upperValue.ShouldBe(testVarValue); + + envVars.TryGetValue(lowerVarName, out string? lowerValue).ShouldBeTrue(); + lowerValue.ShouldBe(testVarValue); + + // Test GetEnvironmentVariable() + taskEnvironment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + taskEnvironment.GetEnvironmentVariable(upperVarName).ShouldBe(testVarValue); + taskEnvironment.GetEnvironmentVariable(lowerVarName).ShouldBe(testVarValue); + } + else + { + // On Unix-like systems, environment variables should be case-sensitive + + // Test GetEnvironmentVariables() + envVars.TryGetValue(testVarName, out string? exactValue).ShouldBeTrue(); + exactValue.ShouldBe(testVarValue); + + // Different case should not match on Unix-like systems + envVars.TryGetValue(upperVarName, out string? upperValue).ShouldBe(upperVarName == testVarName); + envVars.TryGetValue(lowerVarName, out string? lowerValue).ShouldBe(lowerVarName == testVarName); + + // Test GetEnvironmentVariable() + taskEnvironment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + + // Different case should only work if they're actually the same string + var expectedUpperValue = upperVarName == testVarName ? testVarValue : null; + var expectedLowerValue = lowerVarName == testVarName ? testVarValue : null; + + taskEnvironment.GetEnvironmentVariable(upperVarName).ShouldBe(expectedUpperValue); + taskEnvironment.GetEnvironmentVariable(lowerVarName).ShouldBe(expectedLowerValue); + } + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + } + } + + [Fact] + public void TaskEnvironment_MultithreadedEnvironment_ShouldBeIsolatedFromSystem() + { + string testVarName = $"MSBUILD_MULTITHREADED_ISOLATION_TEST_{Guid.NewGuid():N}"; + string testVarValue = "multithreaded_test_value"; + + // On Windows, environment variables are case-insensitive; on Unix-like systems, they are case-sensitive + var comparer = Microsoft.Build.Framework.NativeMethods.IsWindows ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; + var multithreadedEnvironment = new TaskEnvironment(new MultiThreadedTaskEnvironmentDriver( + GetResolvedTempPath(), + new Dictionary(comparer))); + + try + { + // Verify system and environement doesn't have the test variable initially + Environment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + multithreadedEnvironment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + + // Set variable in multithreaded environment + multithreadedEnvironment.SetEnvironmentVariable(testVarName, testVarValue); + + // Multithreaded should have the value but system should not + multithreadedEnvironment.GetEnvironmentVariable(testVarName).ShouldBe(testVarValue); + Environment.GetEnvironmentVariable(testVarName).ShouldBeNull(); + } + finally + { + Environment.SetEnvironmentVariable(testVarName, null); + } + } + } +} diff --git a/src/Build/BackEnd/TaskExecutionHost/MultiProcessTaskEnvironmentDriver.cs b/src/Build/BackEnd/TaskExecutionHost/MultiProcessTaskEnvironmentDriver.cs new file mode 100644 index 00000000000..3f8468e1f5b --- /dev/null +++ b/src/Build/BackEnd/TaskExecutionHost/MultiProcessTaskEnvironmentDriver.cs @@ -0,0 +1,77 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using Microsoft.Build.Framework; +using Microsoft.Build.Internal; + +namespace Microsoft.Build.BackEnd +{ + /// + /// Default implementation of that directly interacts with the file system + /// and environment variables. Used in multi-process mode of execution. + /// + /// + /// Implemented as a singleton since it has no instance state. + /// + internal sealed class MultiProcessTaskEnvironmentDriver : ITaskEnvironmentDriver + { + /// + /// The singleton instance. + /// + private static readonly MultiProcessTaskEnvironmentDriver s_instance = new MultiProcessTaskEnvironmentDriver(); + + /// + /// Gets the singleton instance of . + /// + public static MultiProcessTaskEnvironmentDriver Instance => s_instance; + + private MultiProcessTaskEnvironmentDriver() { } + + /// + public AbsolutePath ProjectDirectory + { + get => new AbsolutePath(Directory.GetCurrentDirectory(), ignoreRootedCheck: true); + set => Directory.SetCurrentDirectory(value.Value); + } + + /// + public AbsolutePath GetAbsolutePath(string path) + { + return new AbsolutePath(Path.GetFullPath(path), ignoreRootedCheck: true); + } + + /// + public string? GetEnvironmentVariable(string name) + { + return Environment.GetEnvironmentVariable(name); + } + + /// + public IReadOnlyDictionary GetEnvironmentVariables() + { + return CommunicationsUtilities.GetEnvironmentVariables(); + } + + /// + public void SetEnvironmentVariable(string name, string? value) + { + CommunicationsUtilities.SetEnvironmentVariable(name, value); + } + + /// + public void SetEnvironment(IDictionary newEnvironment) + { + CommunicationsUtilities.SetEnvironment(newEnvironment); + } + + /// + public ProcessStartInfo GetProcessStartInfo() + { + return new ProcessStartInfo(); + } + } +} diff --git a/src/Build/BackEnd/TaskExecutionHost/MultiThreadedTaskEnvironmentDriver.cs b/src/Build/BackEnd/TaskExecutionHost/MultiThreadedTaskEnvironmentDriver.cs new file mode 100644 index 00000000000..5ed733e627b --- /dev/null +++ b/src/Build/BackEnd/TaskExecutionHost/MultiThreadedTaskEnvironmentDriver.cs @@ -0,0 +1,123 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using Microsoft.Build.Framework; +using Microsoft.Build.Internal; + +namespace Microsoft.Build.BackEnd +{ + /// + /// Implementation of that virtualizes environment variables and current directory + /// for use in multithreaded mode where tasks may be executed in parallel. This allows each project to maintain its own + /// isolated environment state without affecting other concurrently building projects. + /// + /// + /// This class is not accessed from multiple threads. Each msbuild thread node has its own instance to work with. + /// + internal sealed class MultiThreadedTaskEnvironmentDriver : ITaskEnvironmentDriver + { + private readonly Dictionary _environmentVariables; + private AbsolutePath _currentDirectory; + + /// + /// Initializes a new instance of the class + /// with the specified working directory and optional environment variables. + /// + /// The initial working directory. + /// Dictionary of environment variables to use. + public MultiThreadedTaskEnvironmentDriver( + string currentDirectoryFullPath, + IDictionary environmentVariables) + { + _environmentVariables = new Dictionary(environmentVariables, CommunicationsUtilities.EnvironmentVariableComparer); + _currentDirectory = new AbsolutePath(currentDirectoryFullPath, ignoreRootedCheck: true); + } + + /// + /// Initializes a new instance of the class + /// with the specified working directory and environment variables from the current process. + /// + /// The initial working directory. + public MultiThreadedTaskEnvironmentDriver(string currentDirectoryFullPath) + { + IDictionary variables = Environment.GetEnvironmentVariables(); + _environmentVariables = new Dictionary(variables.Count, CommunicationsUtilities.EnvironmentVariableComparer); + foreach (DictionaryEntry entry in variables) + { + _environmentVariables[(string)entry.Key] = (string)entry.Value!; + } + + _currentDirectory = new AbsolutePath(currentDirectoryFullPath, ignoreRootedCheck: true); + } + + /// + public AbsolutePath ProjectDirectory + { + get => _currentDirectory; + set => _currentDirectory = value; + } + + /// + public AbsolutePath GetAbsolutePath(string path) + { + return new AbsolutePath(path, ProjectDirectory); + } + + /// + public string? GetEnvironmentVariable(string name) + { + return _environmentVariables.TryGetValue(name, out string? value) ? value : null; + } + + /// + public IReadOnlyDictionary GetEnvironmentVariables() + { + return _environmentVariables; + } + + /// + public void SetEnvironmentVariable(string name, string? value) + { + if (value == null) + { + _environmentVariables.Remove(name); + } + else + { + _environmentVariables[name] = value; + } + } + + /// + public void SetEnvironment(IDictionary newEnvironment) + { + // Simply replace the entire environment dictionary + _environmentVariables.Clear(); + foreach (KeyValuePair entry in newEnvironment) + { + _environmentVariables[entry.Key] = entry.Value; + } + } + + /// + public ProcessStartInfo GetProcessStartInfo() + { + var startInfo = new ProcessStartInfo + { + WorkingDirectory = ProjectDirectory.Value + }; + + // Set environment variables + foreach (var kvp in _environmentVariables) + { + startInfo.EnvironmentVariables[kvp.Key] = kvp.Value; + } + + return startInfo; + } + } +} diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index d11075fd13b..d53ecf9d743 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -374,6 +374,8 @@ + + diff --git a/src/Framework.UnitTests/AbsolutePath_Tests.cs b/src/Framework.UnitTests/AbsolutePath_Tests.cs new file mode 100644 index 00000000000..3a099ef36c5 --- /dev/null +++ b/src/Framework.UnitTests/AbsolutePath_Tests.cs @@ -0,0 +1,193 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Microsoft.Build.Framework; +using Shouldly; +using Xunit; + +namespace Microsoft.Build.UnitTests +{ + public class AbsolutePath_Tests + { + private static void ValidatePathAcceptance(string path, bool shouldBeAccepted) + { + if (shouldBeAccepted) + { + // Should not throw - these are truly absolute paths + var absolutePath = new AbsolutePath(path); + absolutePath.Value.ShouldBe(path); + } + else + { + // Should throw ArgumentException for any non-absolute path + Should.Throw(() => new AbsolutePath(path, ignoreRootedCheck: false), + $"Path '{path}' should be rejected as it's not a true absolute path"); + } + } + + [Fact] + public void AbsolutePath_FromAbsolutePath_ShouldPreservePath() + { + string absolutePathString = Path.GetTempPath(); + var absolutePath = new AbsolutePath(absolutePathString); + + absolutePath.Value.ShouldBe(absolutePathString); + Path.IsPathRooted(absolutePath.Value).ShouldBeTrue(); + } + + [Theory] + [InlineData("subfolder")] + [InlineData("deep/nested/path")] + [InlineData(".")] + [InlineData("")] + [InlineData("..")] + public void AbsolutePath_FromRelativePath_ShouldResolveAgainstBase(string relativePath) + { + string baseDirectory = Path.Combine(Path.GetTempPath(), "testfolder"); + var basePath = new AbsolutePath(baseDirectory); + var absolutePath = new AbsolutePath(relativePath, basePath); + + Path.IsPathRooted(absolutePath.Value).ShouldBeTrue(); + + string expectedPath = Path.Combine(baseDirectory, relativePath); + absolutePath.Value.ShouldBe(expectedPath); + } + + [Fact] + public void AbsolutePath_Equality_ShouldWorkCorrectly() + { + string testPath = Path.GetTempPath(); + var path1 = new AbsolutePath(testPath); + var path2 = new AbsolutePath(testPath); + var differentPath = new AbsolutePath(Path.Combine(testPath, "different")); + + path1.ShouldBe(path2); + (path1 == path2).ShouldBeTrue(); + path1.ShouldNotBe(differentPath); + (path1 == differentPath).ShouldBeFalse(); + } + + [Fact] + public void AbsolutePath_Inequality_ShouldWorkCorrectly() + { + string testPath = Path.GetTempPath(); + var path1 = new AbsolutePath(testPath); + var differentPath = new AbsolutePath(Path.Combine(testPath, "different")); + + (path1 != differentPath).ShouldBeTrue(); +#pragma warning disable CS1718 // Comparison made to same variable + (path1 != path1).ShouldBeFalse(); +#pragma warning restore CS1718 // Comparison made to same variable + } + + [Fact] + public void AbsolutePath_GetHashCode_ShouldBeConsistentWithEquals() + { + string testPath = Path.GetTempPath(); + var path1 = new AbsolutePath(testPath); + var path2 = new AbsolutePath(testPath); + + // Equal objects must have equal hash codes + path1.Equals(path2).ShouldBeTrue(); + path1.GetHashCode().ShouldBe(path2.GetHashCode()); + } + + [Fact] + public void AbsolutePath_Equals_WithObject_ShouldWorkCorrectly() + { + string testPath = Path.GetTempPath(); + var path1 = new AbsolutePath(testPath); + object path2 = new AbsolutePath(testPath); + object notAPath = "not a path"; + + path1.Equals(path2).ShouldBeTrue(); + path1.Equals(notAPath).ShouldBeFalse(); + path1.Equals(null).ShouldBeFalse(); + } + + [WindowsOnlyFact] + public void AbsolutePath_CaseInsensitive_OnWindows() + { + // On Windows, paths are case-insensitive + var lowerPath = new AbsolutePath("C:\\foo\\bar", ignoreRootedCheck: true); + var upperPath = new AbsolutePath("C:\\FOO\\BAR", ignoreRootedCheck: true); + + lowerPath.Equals(upperPath).ShouldBeTrue(); + (lowerPath == upperPath).ShouldBeTrue(); + lowerPath.GetHashCode().ShouldBe(upperPath.GetHashCode()); + } + + [LinuxOnlyFact] + public void AbsolutePath_CaseSensitive_OnLinux() + { + // On Linux, paths are case-sensitive + var lowerPath = new AbsolutePath("/foo/bar"); + var upperPath = new AbsolutePath("/FOO/BAR"); + + lowerPath.Equals(upperPath).ShouldBeFalse(); + (lowerPath == upperPath).ShouldBeFalse(); + } + + [Theory] + [InlineData("not/rooted/path", false, true)] + [InlineData("not/rooted/path", true, false)] + public void AbsolutePath_RootedValidation_ShouldBehaveProperly(string path, bool ignoreRootedCheck, bool shouldThrow) + { + if (shouldThrow) + { + Should.Throw(() => new AbsolutePath(path, ignoreRootedCheck: ignoreRootedCheck)); + } + else + { + var absolutePath = new AbsolutePath(path, ignoreRootedCheck: ignoreRootedCheck); + absolutePath.Value.ShouldBe(path); + } + } + + [WindowsOnlyTheory] + // True Windows absolute paths - should be accepted + [InlineData("C:\\foo", true)] // Standard Windows absolute path + [InlineData("C:\\foo\\bar", true)] // Another Windows absolute path + [InlineData("D:\\foo\\bar", true)] // Different drive Windows path + [InlineData("C:\\foo\\bar\\.", true)] // Windows absolute path with current directory + [InlineData("C:\\foo\\bar\\..", true)] // Windows absolute path with parent directory + // Windows rooted but NOT absolute paths - should be rejected + [InlineData("\\foo", false)] // Root-relative (missing drive) + [InlineData("\\foo\\bar", false)] // Root-relative (missing drive) + [InlineData("C:foo", false)] // Drive-relative (no backslash after colon) + [InlineData("C:1\\foo", false)] // Drive-relative with unexpected character + // Relative paths - should be rejected + [InlineData("foo", false)] // Simple relative path + [InlineData("foo/bar", false)] // Forward slash relative path + [InlineData("foo\\bar", false)] // Backslash relative path + [InlineData(".", false)] // Current directory + [InlineData("..", false)] // Parent directory + [InlineData("../parent", false)] // Parent relative path + [InlineData("subfolder/file.txt", false)] // Nested relative path + public void AbsolutePath_WindowsPathValidation_ShouldAcceptOnlyTrueAbsolutePaths(string path, bool shouldBeAccepted) + { + ValidatePathAcceptance(path, shouldBeAccepted); + } + + [UnixOnlyTheory] + // True Unix absolute paths - should be accepted + [InlineData("/foo", true)] // Standard Unix absolute path + [InlineData("/foo/bar", true)] // Nested Unix absolute path + [InlineData("/", true)] // Root directory + [InlineData("/foo/bar/.", true)] // Unix absolute path with current directory + [InlineData("/foo/bar/..", true)] // Unix absolute path with parent directory + // Relative paths - should be rejected (same on all platforms) + [InlineData("foo", false)] // Simple relative path + [InlineData("foo/bar", false)] // Forward slash relative path + [InlineData("foo\\bar", false)] // Backslash relative path (unusual on Unix but still relative) + [InlineData(".", false)] // Current directory + [InlineData("..", false)] // Parent directory + [InlineData("../parent", false)] // Parent relative path + [InlineData("subfolder/file.txt", false)] // Nested relative path + public void AbsolutePath_UnixPathValidation_ShouldAcceptOnlyTrueAbsolutePaths(string path, bool shouldBeAccepted) + { + ValidatePathAcceptance(path, shouldBeAccepted); + } + } +} diff --git a/src/Framework/FileClassifier.cs b/src/Framework/FileClassifier.cs index f6f79baafb1..642b0db8735 100644 --- a/src/Framework/FileClassifier.cs +++ b/src/Framework/FileClassifier.cs @@ -65,6 +65,8 @@ private set /// /// /// TODO: Replace RuntimeInformation.IsOSPlatform(OSPlatform.Linux) by NativeMethodsShared.OSUsesCaseSensitivePaths once it is moved out from Shared + /// FIXME: shared code should be consolidated to Framework https://github.com/dotnet/msbuild/issues/6984 + /// this is a subtle bug, MacOS may have both case-insensitive and sensitive file system depending on configuration /// private static readonly StringComparison PathComparison = RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase; diff --git a/src/Framework/ITaskEnvironmentDriver.cs b/src/Framework/ITaskEnvironmentDriver.cs new file mode 100644 index 00000000000..1bb46b47d8d --- /dev/null +++ b/src/Framework/ITaskEnvironmentDriver.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; + +namespace Microsoft.Build.Framework +{ + /// + /// Internal interface for managing task execution environment, including environment variables and working directory. + /// + /// + /// If we ever consider making any part of this API public, strongly consider making an abstract class instead of a public interface. + /// + internal interface ITaskEnvironmentDriver + { + /// + /// Gets or sets the current working directory for the task environment. + /// + AbsolutePath ProjectDirectory { get; internal set; } + + /// + /// Gets an absolute path from the specified path, resolving relative paths against the current project directory. + /// + /// The path to convert to absolute. + /// An absolute path representation. + AbsolutePath GetAbsolutePath(string path); + + /// + /// Gets the value of the specified environment variable. + /// + /// The name of the environment variable. + /// The value of the environment variable, or null if not found. + string? GetEnvironmentVariable(string name); + + /// + /// Gets all environment variables for this task environment. + /// + /// A read-only dictionary of environment variable names and values. + IReadOnlyDictionary GetEnvironmentVariables(); + + /// + /// Sets an environment variable to the specified value. + /// + /// The name of the environment variable. + /// The value to set, or null to remove the variable. + void SetEnvironmentVariable(string name, string? value); + + /// + /// Sets the environment to match the specified collection of variables. + /// Removes variables not present in the new environment and updates or adds those that are. + /// + /// The new environment variable collection. + void SetEnvironment(IDictionary newEnvironment); + + /// + /// Gets a ProcessStartInfo configured with the current environment and working directory. + /// + /// A ProcessStartInfo with the current environment settings. + ProcessStartInfo GetProcessStartInfo(); + } +} \ No newline at end of file diff --git a/src/Framework/NativeMethods.cs b/src/Framework/NativeMethods.cs index 23d09a573fd..e5858135cf0 100644 --- a/src/Framework/NativeMethods.cs +++ b/src/Framework/NativeMethods.cs @@ -865,6 +865,31 @@ internal static bool OSUsesCaseSensitivePaths get { return IsLinux; } } +#if !CLR2COMPATIBILITY + /// + /// Determines whether the file system is case sensitive by creating a test file. + /// Copied from FileUtilities.GetIsFileSystemCaseSensitive() in Shared. + /// FIXME: shared code should be consolidated to Framework https://github.com/dotnet/msbuild/issues/6984 + /// + private static readonly Lazy s_isFileSystemCaseSensitive = new(() => + { + try + { + string pathWithUpperCase = Path.Combine(Path.GetTempPath(), $"INTCASESENSITIVETEST{Guid.NewGuid():N}"); + using (new FileStream(pathWithUpperCase, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, 0x1000, FileOptions.DeleteOnClose)) + { + return !File.Exists(pathWithUpperCase.ToLowerInvariant()); + } + } + catch + { + return OSUsesCaseSensitivePaths; + } + }); + + internal static bool IsFileSystemCaseSensitive => s_isFileSystemCaseSensitive.Value; +#endif + /// /// The base directory for all framework paths in Mono /// diff --git a/src/Framework/PathHelpers/AbsolutePath.cs b/src/Framework/PathHelpers/AbsolutePath.cs index 9b8c39143a3..1c5cc585e5b 100644 --- a/src/Framework/PathHelpers/AbsolutePath.cs +++ b/src/Framework/PathHelpers/AbsolutePath.cs @@ -2,6 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +#if NETFRAMEWORK +using Microsoft.IO; +#else +using System.IO; +#endif namespace Microsoft.Build.Framework { @@ -9,54 +14,125 @@ namespace Microsoft.Build.Framework /// Represents an absolute file system path. /// /// - /// This struct ensures that paths are always in absolute form and properly formatted. + /// This struct wraps a string representing an absolute file system path. + /// Path equality comparisons are case-sensitive or case-insensitive depending on the operating system's + /// file system conventions (case-sensitive on Linux, case-insensitive on Windows and macOS). + /// Does not perform any normalization beyond validating the path is fully qualified. + /// A default instance (created via default(AbsolutePath)) has a null Value + /// and should not be used. Two default instances are considered equal. /// - public readonly struct AbsolutePath + public readonly struct AbsolutePath : IEquatable { /// - /// Gets the string representation of this path. + /// The string comparer to use for path comparisons, based on OS file system case sensitivity. /// - public string Path { get; } + private static readonly StringComparer s_pathComparer = NativeMethods.IsFileSystemCaseSensitive ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase; /// - /// Creates a new instance of AbsolutePath. + /// The normalized string representation of this path. + /// + public string Value { get; } + + /// + /// Initializes a new instance of the struct. /// /// The absolute path string. public AbsolutePath(string path) { - throw new NotImplementedException(); + ValidatePath(path); + Value = path; } /// - /// Creates a new instance of AbsolutePath. + /// Initializes a new instance of the struct. /// /// The absolute path string. /// If true, skips checking whether the path is rooted. + /// For internal and testing use, when we want to force bypassing the rooted check. internal AbsolutePath(string path, bool ignoreRootedCheck) { - throw new NotImplementedException(); + if (!ignoreRootedCheck) + { + ValidatePath(path); + } + Value = path; } /// - /// Creates a new absolute path by combining a absolute path with a relative path. + /// Validates that the specified file system path is non-empty and rooted. /// - /// The path to combine with the base path. - /// The base path to combine with. - public AbsolutePath(string path, AbsolutePath basePath) + /// The file system path to validate. Must not be null, empty, or a relative path. + /// Thrown if is null, empty, or not a rooted path. + private static void ValidatePath(string path) { - throw new NotImplementedException(); + if (string.IsNullOrEmpty(path)) + { + throw new ArgumentException("Path must not be null or empty.", nameof(path)); + } + + // Path.IsPathFullyQualified is not available in .NET Standard 2.0 + // in .NET Framework it's provided by package and in .NET it's built-in +#if NETFRAMEWORK || NET + if (!Path.IsPathFullyQualified(path)) + { + throw new ArgumentException("Path must be rooted.", nameof(path)); + } +#endif } + /// + /// Initializes a new instance of the struct by combining an absolute path with a relative path. + /// + /// The path to combine with the base path. + /// The base path to combine with. + public AbsolutePath(string path, AbsolutePath basePath) => Value = Path.Combine(basePath.Value, path); + /// /// Implicitly converts an AbsolutePath to a string. /// /// The path to convert. - public static implicit operator string(AbsolutePath path) => throw new NotImplementedException(); + public static implicit operator string(AbsolutePath path) => path.Value; + + /// + /// Determines whether two instances are equal. + /// + /// The first path to compare. + /// The second path to compare. + /// true if the paths are equal; otherwise, false. + public static bool operator ==(AbsolutePath left, AbsolutePath right) => left.Equals(right); + + /// + /// Determines whether two instances are not equal. + /// + /// The first path to compare. + /// The second path to compare. + /// true if the paths are not equal; otherwise, false. + public static bool operator !=(AbsolutePath left, AbsolutePath right) => !left.Equals(right); + + /// + /// Determines whether the specified object is equal to the current . + /// + /// The object to compare with the current instance. + /// true if the specified object is an and is equal to the current instance; otherwise, false. + public override bool Equals(object? obj) => obj is AbsolutePath other && Equals(other); + + /// + /// Determines whether the specified is equal to the current instance. + /// + /// The to compare with the current instance. + /// true if the paths are equal according to the operating system's file system case sensitivity rules; otherwise, false. + public bool Equals(AbsolutePath other) => s_pathComparer.Equals(Value, other.Value); + + /// + /// Returns a hash code for this . + /// + /// A hash code that is consistent with the equality comparison. + public override int GetHashCode() => Value is null ? 0 : s_pathComparer.GetHashCode(Value); /// /// Returns the string representation of this path. /// /// The path as a string. - public override string ToString() => throw new NotImplementedException(); + public override string ToString() => Value; } } diff --git a/src/Framework/TaskEnvironment.cs b/src/Framework/TaskEnvironment.cs index 3e3417f5172..e4ba0284c45 100644 --- a/src/Framework/TaskEnvironment.cs +++ b/src/Framework/TaskEnvironment.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Collections.Generic; using System.Diagnostics; @@ -13,13 +12,23 @@ namespace Microsoft.Build.Framework /// public sealed class TaskEnvironment { + private readonly ITaskEnvironmentDriver _driver; + + /// + /// Initializes a new instance of the TaskEnvironment class. + /// + internal TaskEnvironment(ITaskEnvironmentDriver driver) + { + _driver = driver; + } + /// /// Gets or sets the project directory for the task execution. /// public AbsolutePath ProjectDirectory { - get => throw new NotImplementedException(); - internal set => throw new NotImplementedException(); + get => _driver.ProjectDirectory; + internal set => _driver.ProjectDirectory = value; } /// @@ -28,32 +37,39 @@ public AbsolutePath ProjectDirectory /// /// The path to convert. /// An absolute path representation. - public AbsolutePath GetAbsolutePath(string path) => throw new NotImplementedException(); + public AbsolutePath GetAbsolutePath(string path) => _driver.GetAbsolutePath(path); /// /// Gets the value of an environment variable. /// /// The name of the environment variable. /// The value of the environment variable, or null if it does not exist. - public string? GetEnvironmentVariable(string name) => throw new NotImplementedException(); + public string? GetEnvironmentVariable(string name) => _driver.GetEnvironmentVariable(name); /// /// Gets a dictionary containing all environment variables. /// /// A read-only dictionary of environment variables. - public IReadOnlyDictionary GetEnvironmentVariables() => throw new NotImplementedException(); + public IReadOnlyDictionary GetEnvironmentVariables() => _driver.GetEnvironmentVariables(); /// /// Sets the value of an environment variable. /// /// The name of the environment variable. /// The value to set, or null to remove the environment variable. - public void SetEnvironmentVariable(string name, string? value) => throw new NotImplementedException(); + public void SetEnvironmentVariable(string name, string? value) => _driver.SetEnvironmentVariable(name, value); + + /// + /// Updates the environment to match the provided dictionary. + /// This mirrors the behavior of CommunicationsUtilities.SetEnvironment but operates on this TaskEnvironment. + /// + /// The new environment variables to set. + internal void SetEnvironment(IDictionary newEnvironment) => _driver.SetEnvironment(newEnvironment); /// /// Creates a new ProcessStartInfo configured for the current task execution environment. /// /// A ProcessStartInfo object configured for the current task execution environment. - public ProcessStartInfo GetProcessStartInfo() => throw new NotImplementedException(); + public ProcessStartInfo GetProcessStartInfo() => _driver.GetProcessStartInfo(); } -} +} \ No newline at end of file diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index 530b23b164b..9be025ffadd 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -373,6 +373,13 @@ internal static class CommunicationsUtilities /// internal delegate void LogDebugCommunications(string format, params object[] stuff); + /// + /// Gets the string comparer for environment variable names based on the current platform. + /// On Windows, environment variables are case-insensitive; on Unix-like systems, they are case-sensitive. + /// + internal static StringComparer EnvironmentVariableComparer => + NativeMethodsShared.IsWindows ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal; + /// /// Gets or sets the node connection timeout. /// @@ -622,7 +629,7 @@ internal static FrozenDictionary GetEnvironmentVariables() } // Otherwise, allocate and update with the current state. - Dictionary table = new(vars.Count, StringComparer.OrdinalIgnoreCase); + Dictionary table = new(vars.Count, EnvironmentVariableComparer); enumerator.Reset(); while (enumerator.MoveNext()) @@ -633,7 +640,7 @@ internal static FrozenDictionary GetEnvironmentVariables() table[key] = value; } - EnvironmentState newState = new(table.ToFrozenDictionary(StringComparer.OrdinalIgnoreCase)); + EnvironmentState newState = new(table.ToFrozenDictionary(EnvironmentVariableComparer)); s_environmentState = newState; return newState.EnvironmentVariables;