Enlighten FindUnderPath and AssignTargetPath tasks

documentation/wiki/ChangeWaves.md

@@ -24,6 +24,10 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 
 ## Current Rotation of Change Waves
 
+### 18.5
+- [FindUnderPath and AssignTargetPath tasks no longer throw on invalid path characters when using TaskEnvironment.GetAbsolutePath](https://github.com/dotnet/msbuild/pull/13069)
+- [AssignTargetPath on Linux respects case sensitivity of the file system instead of always ignoring case](https://github.com/dotnet/msbuild/pull/13069)
+
 ### 18.4
 - [Start throwing on null or empty paths in MultiProcess and MultiThreaded Task Environment Drivers.](https://github.com/dotnet/msbuild/pull/12914)
 

src/Build/BackEnd/TaskExecutionHost/MultiThreadedTaskEnvironmentDriver.cs

@@ -60,10 +60,10 @@ public AbsolutePath ProjectDirectory
             get => _currentDirectory;
             set
             {
-                _currentDirectory = value;
+                _currentDirectory = value.GetCanonicalForm();
                 // Keep the thread-static in sync for use by Expander and Modifiers during property/item expansion.
                 // This allows Path.GetFullPath and %(FullPath) functions used in project files to resolve relative paths correctly in multithreaded mode.
-                FrameworkFileUtilities.CurrentThreadWorkingDirectory = value.Value;
+                FrameworkFileUtilities.CurrentThreadWorkingDirectory = _currentDirectory.Value;
             }
         }
 

src/Framework/ChangeWaves.cs

@@ -32,7 +32,8 @@ internal static class ChangeWaves
         internal static readonly Version Wave17_14 = new Version(17, 14);
         internal static readonly Version Wave18_3 = new Version(18, 3);
         internal static readonly Version Wave18_4 = new Version(18, 4);
-        internal static readonly Version[] AllWaves = [Wave17_10, Wave17_12, Wave17_14, Wave18_3, Wave18_4];
+        internal static readonly Version Wave18_5 = new Version(18, 5);
+        internal static readonly Version[] AllWaves = [Wave17_10, Wave17_12, Wave17_14, Wave18_3, Wave18_4, Wave18_5];
 
         /// <summary>
         /// Special value indicating that all features behind all Change Waves should be enabled.

src/Tasks.UnitTests/AssignTargetPath_Tests.cs

@@ -18,6 +18,7 @@ public sealed class AssignTargetPath_Tests
         public void Regress314791()
         {
             AssignTargetPath t = new AssignTargetPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
             t.Files = new ITaskItem[]
                           { new TaskItem(NativeMethodsShared.IsWindows ? @"c:\bin2\abc.efg" : "/bin2/abc.efg") };
@@ -33,6 +34,7 @@ public void Regress314791()
         public void AtConeRoot()
         {
             AssignTargetPath t = new AssignTargetPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
             t.Files = new ITaskItem[]
                           { new TaskItem(NativeMethodsShared.IsWindows ? @"c:\f1\f2\file.txt" : "/f1/f2/file.txt") };
@@ -47,6 +49,7 @@ public void AtConeRoot()
         public void OutOfCone()
         {
             AssignTargetPath t = new AssignTargetPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
             t.Files = new ITaskItem[]
                           {
@@ -68,6 +71,7 @@ public void OutOfCone()
         public void InConeButAbsolute()
         {
             AssignTargetPath t = new AssignTargetPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
             t.Files = new ITaskItem[]
                           {
@@ -90,6 +94,7 @@ public void InConeButAbsolute()
         public void TargetPathAlreadySet(string targetPath)
         {
             AssignTargetPath t = new AssignTargetPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
             Dictionary<string, string> metaData = new Dictionary<string, string>();
             metaData.Add("TargetPath", targetPath);

src/Tasks.UnitTests/FindUnderPath_Tests.cs

@@ -6,6 +6,7 @@
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 using Microsoft.Build.Tasks;
+using Microsoft.Build.UnitTests;
 using Microsoft.Build.Utilities;
 using Xunit;
 
@@ -21,6 +22,7 @@ public sealed class FindUnderPath_Tests
         public void BasicFilter()
         {
             FindUnderPath t = new FindUnderPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
 
             t.Path = new TaskItem(@"C:\MyProject");
@@ -35,10 +37,21 @@ public void BasicFilter()
             Assert.Equal(FrameworkFileUtilities.FixFilePath(@"C:\SomeoneElsesProject\File2.txt"), t.OutOfPath[0].ItemSpec);
         }
 
+        /// <summary>
+        /// Tests that invalid file path characters cause the task to fail.
+        /// This only applies when Wave18_5 is disabled, as the new behavior doesn't throw on invalid path characters.
+        /// </summary>
         [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486. On Unix there is no invalid file name characters.")]
         public void InvalidFile()
         {
+            using TestEnvironment env = TestEnvironment.Create();
+
+            // TODO: Remove test when Wave18_5 rotates out - new behavior doesn't throw on invalid path characters
+            ChangeWaves.ResetStateForTests();
+            env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_5.ToString());
+
             FindUnderPath t = new FindUnderPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
 
             t.Path = new TaskItem(@"C:\MyProject");
@@ -48,13 +61,26 @@ public void InvalidFile()
 
             Assert.False(success);
 
+            ChangeWaves.ResetStateForTests();
+
             // Don't crash
         }
 
+        /// <summary>
+        /// Tests that invalid path characters cause the task to fail.
+        /// This only applies when Wave18_5 is disabled, as the new behavior doesn't throw on invalid path characters.
+        /// </summary>
         [WindowsFullFrameworkOnlyFact(additionalMessage: ".NET Core 2.1+ no longer validates paths: https://github.com/dotnet/corefx/issues/27779#issuecomment-371253486. On Unix there is no invalid file name characters.")]
         public void InvalidPath()
         {
+            using TestEnvironment env = TestEnvironment.Create();
+
+            // TODO: Remove test when Wave18_5 rotates out - new behavior doesn't throw on invalid path characters
+            ChangeWaves.ResetStateForTests();
+            env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_5.ToString());
+
             FindUnderPath t = new FindUnderPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
 
             t.Path = new TaskItem(@"||::||");
@@ -64,6 +90,8 @@ public void InvalidPath()
 
             Assert.False(success);
 
+            ChangeWaves.ResetStateForTests();
+
             // Don't crash
         }
 
@@ -96,6 +124,7 @@ private static void RunTask(FindUnderPath t, out FileInfo testFile, out bool suc
         public void VerifyFullPath()
         {
             FindUnderPath t = new FindUnderPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
 
             t.UpdateToAbsolutePaths = true;
@@ -118,6 +147,7 @@ public void VerifyFullPath()
         public void VerifyFullPathNegative()
         {
             FindUnderPath t = new FindUnderPath();
+            t.TaskEnvironment = TaskEnvironmentHelper.CreateForTest();
             t.BuildEngine = new MockEngine();
 
             t.UpdateToAbsolutePaths = false;

src/Tasks/AssignTargetPath.cs

@@ -15,10 +15,17 @@ namespace Microsoft.Build.Tasks
     /// Create a new list of items that have <TargetPath> attributes if none was present in
     /// the input.
     /// </summary>
-    public class AssignTargetPath : TaskExtension
+    [MSBuildMultiThreadableTask]
+    public class AssignTargetPath : TaskExtension, IMultiThreadableTask
     {
         #region Properties
 
+        /// <summary>
+        /// Gets or sets the task execution environment for thread-safe path resolution.
+        /// </summary>
+        public TaskEnvironment TaskEnvironment { get; set; }
+
+
         /// <summary>
         /// The folder to make the links relative to.
         /// </summary>
@@ -49,27 +56,42 @@ public override bool Execute()
 
             if (Files.Length > 0)
             {
-                // Compose a file in the root folder.
-                // NOTE: at this point fullRootPath may or may not have a trailing
-                // slash because Path.GetFullPath() does not add or remove it
-                string fullRootPath = Path.GetFullPath(RootFolder);
-
-                // Ensure trailing slash otherwise c:\bin appears to match part of c:\bin2\foo
-                fullRootPath = FrameworkFileUtilities.EnsureTrailingSlash(fullRootPath);
-
-                string currentDirectory = Directory.GetCurrentDirectory();
-
-                // check if the root folder is the same as the current directory
-                // NOTE: the path returned from Directory.GetCurrentDirectory()
-                // does not have a trailing slash, but fullRootPath does
-                bool isRootFolderSameAsCurrentDirectory =
-                    ((fullRootPath.Length - 1 /* exclude trailing slash */) == currentDirectory.Length)
-                &&
-                    (String.Compare(
-                        fullRootPath, 0,
-                        currentDirectory, 0,
-                        fullRootPath.Length - 1 /* don't compare trailing slash */,
-                        StringComparison.OrdinalIgnoreCase) == 0);
+                AbsolutePath fullRootPath = default;
+                string fullRootPathString = null;
+                bool isRootFolderSameAsCurrentDirectory;
+
+                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5))
+                {
+                    // Compose a file in the root folder.
+                    // NOTE: at this point fullRootPath may or may not have a trailing slash
+                    // Ensure trailing slash otherwise c:\bin appears to match part of c:\bin2\foo
+                    // Also ensure that relative segments in the path are resolved.
+                    fullRootPath =
+                        TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.EnsureTrailingSlash(RootFolder)).GetCanonicalForm();
+
+                    // Ensure trailing slash for comparison. Current directory is already canonical, so we don't need to call GetCanonicalForm on it.
+                    AbsolutePath currentDirectory = FrameworkFileUtilities.EnsureTrailingSlash(TaskEnvironment.ProjectDirectory);
+
+                    // Check if the root folder is the same as the current directory - AbsolutePath handles OS-aware case sensitivity.
+                    isRootFolderSameAsCurrentDirectory = fullRootPath == currentDirectory;
+                }
+                else
+                {
+                    // Compose a file in the root folder.
+                    // NOTE: at this point fullRootPath may or may not have a trailing slash
+                    // Ensure trailing slash otherwise c:\bin appears to match part of c:\bin2\foo
+                    // Also ensure that relative segments in the path are resolved and throw on illegal characters in Path.GetFullPath to preserve pre-existing behavior.
+                    fullRootPathString = 
+                        Path.GetFullPath(TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.EnsureTrailingSlash(RootFolder)));
+
+                    // Ensure trailing slash for comparison. Current directory is already canonical, so we don't need to call GetCanonicalForm on it.
+                    AbsolutePath currentDirectory = FrameworkFileUtilities.EnsureTrailingSlash(TaskEnvironment.ProjectDirectory);
+
+                    // Check if the root folder is the same as the current directory. 
+                    // Perform a case-insensitive comparison to match Path.GetFullPath behavior on Windows, even on case-sensitive file systems, 
+                    // since that's what we're using to determine whether to preserve relative paths or not.
+                    isRootFolderSameAsCurrentDirectory = String.Compare(fullRootPathString, currentDirectory, StringComparison.OrdinalIgnoreCase) == 0;
+                }
 
                 for (int i = 0; i < Files.Length; ++i)
                 {
@@ -95,26 +117,44 @@ public override bool Execute()
                             isRootFolderSameAsCurrentDirectory)
                         {
                             // then just use the file path as-is
-                            // PERF NOTE: we do this to avoid calling Path.GetFullPath() below,
-                            // because that method consumes a lot of memory, esp. when we have
-                            // a lot of items coming through this task
+                            // PERF NOTE: we do this to avoid calling Path.GetFullPath() below (which is called by GetCanonicalForm method),
+                            // because that method consumes a lot of memory, esp. when we have a lot of items coming through this task
                             targetPath = Files[i].ItemSpec;
                         }
                         else
                         {
-                            // PERF WARNING: Path.GetFullPath() is expensive in terms of memory;
-                            // we should avoid calling it whenever possible
-                            string itemSpecFullFileNamePath = Path.GetFullPath(Files[i].ItemSpec);
-
-                            if (String.Compare(fullRootPath, 0, itemSpecFullFileNamePath, 0, fullRootPath.Length, StringComparison.CurrentCultureIgnoreCase) == 0)
+                            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5)) 
                             {
-                                // The item spec file is in the "cone" of the RootFolder. Return the relative path from the cone root.
-                                targetPath = itemSpecFullFileNamePath.Substring(fullRootPath.Length);
+                                AbsolutePath itemSpecFullFileNamePath = 
+                                    TaskEnvironment.GetAbsolutePath(Files[i].ItemSpec).GetCanonicalForm();
+
+
+                                if (itemSpecFullFileNamePath.Value.StartsWith(fullRootPath.Value, FileUtilities.PathComparison))
+                                {
+                                    // The item spec file is in the "cone" of the RootFolder. Return the relative path from the cone root.
+                                    targetPath = itemSpecFullFileNamePath.Value.Substring(fullRootPath.Value.Length);
+                                }
+                                else
+                                {
+                                    // The item spec file is not in the "cone" of the RootFolder. Return the filename only.
+                                    targetPath = Path.GetFileName(Files[i].ItemSpec);
+                                }
                             }
-                            else
+                            else 
                             {
-                                // The item spec file is not in the "cone" of the RootFolder. Return the filename only.
-                                targetPath = Path.GetFileName(Files[i].ItemSpec);
+                                string itemSpecFullFileNamePath = 
+                                    Path.GetFullPath(TaskEnvironment.GetAbsolutePath(Files[i].ItemSpec));
+
+                                if (String.Compare(fullRootPathString, 0, itemSpecFullFileNamePath, 0, fullRootPathString.Length, StringComparison.CurrentCultureIgnoreCase) == 0)
+                                {
+                                    // The item spec file is in the "cone" of the RootFolder. Return the relative path from the cone root.
+                                    targetPath = itemSpecFullFileNamePath.Substring(fullRootPathString.Length);
+                                }
+                                else
+                                {
+                                    // The item spec file is not in the "cone" of the RootFolder. Return the filename only.
+                                    targetPath = Path.GetFileName(Files[i].ItemSpec);
+                                }
                             }
                         }
                     }

src/Tasks/ListOperators/FindUnderPath.cs

@@ -15,8 +15,14 @@ namespace Microsoft.Build.Tasks
     /// <summary>
     /// Given a list of items, determine which are in the cone of the folder passed in and which aren't.
     /// </summary>
-    public class FindUnderPath : TaskExtension
+    [MSBuildMultiThreadableTask]
+    public class FindUnderPath : TaskExtension, IMultiThreadableTask
     {
+        /// <summary>
+        /// Gets or sets the task execution environment for thread-safe path resolution.
+        /// </summary>
+        public TaskEnvironment TaskEnvironment { get; set; }
+
         /// <summary>
         /// Filter based on whether items fall under this path or not.
         /// </summary>
@@ -57,9 +63,19 @@ public override bool Execute()
 
             try
             {
-                conePath =
-                    Strings.WeakIntern(
-                        System.IO.Path.GetFullPath(FrameworkFileUtilities.FixFilePath(Path.ItemSpec)));
+                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5))
+                {
+                    conePath =
+                        Strings.WeakIntern(
+                            TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.FixFilePath(Path.ItemSpec)).GetCanonicalForm());
+                }
+                else
+                {
+                    conePath =
+                        Strings.WeakIntern(
+                            System.IO.Path.GetFullPath(TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.FixFilePath(Path.ItemSpec))));
+                }
+
                 conePath = FrameworkFileUtilities.EnsureTrailingSlash(conePath);
             }
             catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e))
@@ -78,9 +94,18 @@ public override bool Execute()
                 string fullPath;
                 try
                 {
-                    fullPath =
-                        Strings.WeakIntern(
-                            System.IO.Path.GetFullPath(FrameworkFileUtilities.FixFilePath(item.ItemSpec)));
+                    if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5))
+                    {
+                        fullPath =
+                            Strings.WeakIntern(
+                                TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.FixFilePath(item.ItemSpec)).GetCanonicalForm());
+                    }
+                    else
+                    {
+                        fullPath =
+                            Strings.WeakIntern(
+                                System.IO.Path.GetFullPath(TaskEnvironment.GetAbsolutePath(FrameworkFileUtilities.FixFilePath(item.ItemSpec))));
+                    }
                 }
                 catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e))
                 {