diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index ea6af8d5611..68cae7eb843 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -29,6 +29,9 @@ Change wave checks around features will be removed in the release that accompani ## Current Rotation of Change Waves +### 18.6 +- [AbsolutePath.GetCanonicalForm optimization - avoid expensive Path.GetFullPath calls when paths don't need canonicalization](https://github.com/dotnet/msbuild/pull/13369) + ### 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) diff --git a/src/Framework.UnitTests/AbsolutePath_Tests.cs b/src/Framework.UnitTests/AbsolutePath_Tests.cs index 145a24bc797..a78f6c17b37 100644 --- a/src/Framework.UnitTests/AbsolutePath_Tests.cs +++ b/src/Framework.UnitTests/AbsolutePath_Tests.cs @@ -251,6 +251,8 @@ public void GetCanonicalForm_NullPath_ShouldReturnSameInstance() [InlineData("C:\\foo\\..\\bar")] // Parent directory reference [InlineData("C:\\foo/bar")] // Forward slash to backslash [InlineData("C:\\foo\\bar")] // Simple Windows path (no normalization needed) + [InlineData("C:\\foo\\\\bar")] // Consecutive backslashes + [InlineData("C:\\foo\\bar\\\\")] // Trailing consecutive backslashes public void GetCanonicalForm_WindowsPathNormalization_ShouldMatchPathGetFullPath(string inputPath) { ValidateGetCanonicalFormMatchesSystem(inputPath); @@ -261,6 +263,8 @@ public void GetCanonicalForm_WindowsPathNormalization_ShouldMatchPathGetFullPath [InlineData("/foo/../bar")] // Parent directory reference [InlineData("/foo/bar")] // Simple Unix path (no normalization needed) [InlineData("/foo/bar\\baz")] // Simple Unix path with backslash that is not a path separator (no normalization needed) + [InlineData("/foo//bar")] // Consecutive forward slashes + [InlineData("/foo/bar//")] // Trailing consecutive forward slashes public void GetCanonicalForm_UnixPathNormalization_ShouldMatchPathGetFullPath(string inputPath) { ValidateGetCanonicalFormMatchesSystem(inputPath); diff --git a/src/Framework/ChangeWaves.cs b/src/Framework/ChangeWaves.cs index d40cd86c8f3..e759dcafbcd 100644 --- a/src/Framework/ChangeWaves.cs +++ b/src/Framework/ChangeWaves.cs @@ -33,7 +33,8 @@ internal static class ChangeWaves internal static readonly Version Wave18_3 = new Version(18, 3); internal static readonly Version Wave18_4 = new Version(18, 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]; + internal static readonly Version Wave18_6 = new Version(18, 6); + internal static readonly Version[] AllWaves = [Wave17_10, Wave17_12, Wave17_14, Wave18_3, Wave18_4, Wave18_5, Wave18_6]; /// /// Special value indicating that all features behind all Change Waves should be enabled. diff --git a/src/Framework/PathHelpers/AbsolutePath.cs b/src/Framework/PathHelpers/AbsolutePath.cs index 3ad5080e8b0..24a3a926615 100644 --- a/src/Framework/PathHelpers/AbsolutePath.cs +++ b/src/Framework/PathHelpers/AbsolutePath.cs @@ -146,25 +146,43 @@ internal AbsolutePath GetCanonicalForm() return this; } - - // Note: this is a quick check to avoid calling Path.GetFullPath when it's not necessary, since it can be expensive. - // It should cover the most common cases and avoid the overhead of Path.GetFullPath in those cases. - - // Check for relative path segments "." and ".." - // In absolute path those segments can not appear in the beginning of the path, only after a path separator. - // This is not a precise full detection of relative segments. There is no false negatives as this might affect correctenes, but it may have false positives: - // like when there is a hidden file or directory starting with a dot, or on linux the backslash and dot can be part of the file name. - // In case of false positives we would call Path.GetFullPath and the result would still be correct. - - bool hasRelativeSegment = Value.Contains("/.") || Value.Contains("\\."); - - // Check if directory separator normalization is required (only on Windows: "/" to "\") - // On unix "\" is not a valid path separator, but is a part of the file/directory name, so no normalization is needed. - bool needsSeparatorNormalization = NativeMethods.IsWindows && Value.IndexOf(Path.AltDirectorySeparatorChar) >= 0; - - if (!hasRelativeSegment && !needsSeparatorNormalization) + if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_6)) { - return this; + // Note: this is a quick check to avoid calling Path.GetFullPath when it's not necessary, since it can be expensive. + // It should cover the most common cases and avoid the overhead of Path.GetFullPath in those cases. + + // Check for relative path segments "." and ".." + // In absolute path those segments can not appear in the beginning of the path, only after a path separator. + // This is not a precise full detection of relative segments. There is no false negatives as this might affect correctenes, but it may have false positives: + // like when there is a hidden file or directory starting with a dot, or on linux the backslash and dot can be part of the file name. + // In case of false positives we would call Path.GetFullPath and the result would still be correct. + + bool hasRelativeSegment = Value.Contains("/.") || Value.Contains("\\."); + + // Check if directory separator normalization is required (only on Windows: "/" to "\") + // On unix "\" is not a valid path separator, but is a part of the file/directory name, so no normalization is needed. + bool needsSeparatorNormalization = NativeMethods.IsWindows && Value.IndexOf(Path.AltDirectorySeparatorChar) >= 0; + + // Check for consecutive directory separators (e.g., "\\") which Path.GetFullPath would collapse. + // On Windows, consecutive backslashes in the middle of a path (not at the start for UNC) should be collapsed. + // On Unix, consecutive forward slashes should be collapsed. + bool hasConsecutiveSeparators; + if (NativeMethods.IsWindows) + { + // On Windows, search from offset 1 to skip position 0 where UNC paths legitimately start with "\\". + // This still catches cases like "C:\\foo" (positions 2-3) or "D:\foo\\bar". + // Length > 3 guard: searching for 2-char match from offset 1 needs at least 4 chars. + hasConsecutiveSeparators = Value.Length > 3 && Value.IndexOf("\\\\", 1, StringComparison.Ordinal) >= 0; + } + else + { + hasConsecutiveSeparators = Value.Contains("//"); + } + + if (!hasRelativeSegment && !needsSeparatorNormalization && !hasConsecutiveSeparators) + { + return this; + } } // Use Path.GetFullPath to resolve relative segments and normalize separators.