From b9858fb27ea4aec025343f01d5ce1e52b00cc9d9 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 12 Feb 2021 11:01:54 +0100 Subject: [PATCH 01/13] Code cleanup in FileMatcher.cs and MSBuildGlob.cs --- src/Build/Globbing/MSBuildGlob.cs | 39 +++++++----------- src/Build/Utilities/FileSpecMatchTester.cs | 3 +- src/Shared/FileMatcher.cs | 47 +++++++--------------- 3 files changed, 30 insertions(+), 59 deletions(-) diff --git a/src/Build/Globbing/MSBuildGlob.cs b/src/Build/Globbing/MSBuildGlob.cs index 4b03541b3df..79c6abb3cb2 100644 --- a/src/Build/Globbing/MSBuildGlob.cs +++ b/src/Build/Globbing/MSBuildGlob.cs @@ -117,23 +117,20 @@ public MatchInfoResult MatchInfo(string stringToMatch) { ErrorUtilities.VerifyThrowArgumentNull(stringToMatch, nameof(stringToMatch)); - if (FileUtilities.PathIsInvalid(stringToMatch) || - !IsLegal) + if (FileUtilities.PathIsInvalid(stringToMatch) || !IsLegal) { return MatchInfoResult.Empty; } - var normalizedInput = NormalizeMatchInput(stringToMatch); + string normalizedInput = NormalizeMatchInput(stringToMatch); - bool isMatch; - string fixedDirectoryPart, wildcardDirectoryPart, filenamePart; FileMatcher.GetRegexMatchInfo( normalizedInput, _state.Value.Regex, - out isMatch, - out fixedDirectoryPart, - out wildcardDirectoryPart, - out filenamePart); + out bool isMatch, + out string fixedDirectoryPart, + out string wildcardDirectoryPart, + out string filenamePart); return new MatchInfoResult(isMatch, fixedDirectoryPart, wildcardDirectoryPart, filenamePart); } @@ -145,7 +142,7 @@ private string NormalizeMatchInput(string stringToMatch) // Degenerate case when the string to match is empty. // Ensure trailing slash because the fixed directory part has a trailing slash. - if (stringToMatch == string.Empty) + if (string.IsNullOrEmpty(stringToMatch)) { normalizedInput += Path.DirectorySeparatorChar; } @@ -172,7 +169,7 @@ public static MSBuildGlob Parse(string globRoot, string fileSpec) ErrorUtilities.VerifyThrowArgumentNull(fileSpec, nameof(fileSpec)); ErrorUtilities.VerifyThrowArgumentInvalidPath(globRoot, nameof(globRoot)); - if (globRoot == string.Empty) + if (string.IsNullOrEmpty(globRoot)) { globRoot = Directory.GetCurrentDirectory(); } @@ -181,22 +178,14 @@ public static MSBuildGlob Parse(string globRoot, string fileSpec) var lazyState = new Lazy(() => { - string fixedDirectoryPart = null; - string wildcardDirectoryPart = null; - string filenamePart = null; - - string matchFileExpression; - bool needsRecursion; - bool isLegalFileSpec; - FileMatcher.Default.GetFileSpecInfo( fileSpec, - out fixedDirectoryPart, - out wildcardDirectoryPart, - out filenamePart, - out matchFileExpression, - out needsRecursion, - out isLegalFileSpec, + out string fixedDirectoryPart, + out string wildcardDirectoryPart, + out string filenamePart, + out string matchFileExpression, + out bool needsRecursion, + out bool isLegalFileSpec, (fixedDirPart, wildcardDirPart, filePart) => { var normalizedFixedPart = NormalizeTheFixedDirectoryPartAgainstTheGlobRoot(fixedDirPart, globRoot); diff --git a/src/Build/Utilities/FileSpecMatchTester.cs b/src/Build/Utilities/FileSpecMatchTester.cs index e48fca39e77..740c98b5b9b 100644 --- a/src/Build/Utilities/FileSpecMatchTester.cs +++ b/src/Build/Utilities/FileSpecMatchTester.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using Microsoft.Build.Shared; +using System; using System.Diagnostics; using System.IO; using System.Text.RegularExpressions; @@ -38,7 +39,7 @@ public bool IsMatch(string fileToMatch) // check if there is a regex matching the file if (_regex != null) { - var normalizedFileToMatch = FileUtilities.GetFullPathNoThrow(Path.Combine(_currentDirectory, fileToMatch)); + string normalizedFileToMatch = FileUtilities.GetFullPathNoThrow(Path.Combine(_currentDirectory, fileToMatch)); return _regex.IsMatch(normalizedFileToMatch); } diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index ba1fbfd01d3..02c0e2aadcf 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -34,8 +34,7 @@ internal class FileMatcher private static readonly string[] s_propertyAndItemReferences = { "$(", "@(" }; // on OSX both System.IO.Path separators are '/', so we have to use the literals - internal static readonly char[] directorySeparatorCharacters = { '/', '\\' }; - internal static readonly string[] directorySeparatorStrings = directorySeparatorCharacters.Select(c => c.ToString()).ToArray(); + internal static readonly char[] directorySeparatorCharacters = FileUtilities.Slashes; // until Cloudbuild switches to EvaluationContext, we need to keep their dependence on global glob caching via an environment variable private static readonly Lazy>> s_cachedGlobExpansions = new Lazy>>(() => new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase)); @@ -1460,15 +1459,9 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start out bool needsRecursion, out bool isLegalFileSpec) { - string fixedDirectoryPart; - string wildcardDirectoryPart; - string filenamePart; - string matchFileExpression; - GetFileSpecInfo(filespec, - out fixedDirectoryPart, out wildcardDirectoryPart, out filenamePart, - out matchFileExpression, out needsRecursion, out isLegalFileSpec); - + out _, out _, out _, + out string matchFileExpression, out needsRecursion, out isLegalFileSpec); regexFileMatch = isLegalFileSpec ? new Regex(matchFileExpression, DefaultRegexOptions) @@ -2012,21 +2005,15 @@ enum SearchAction stripProjectDirectory = false; result = new RecursionState(); - string fixedDirectoryPart; - string wildcardDirectoryPart; - string filenamePart; - string matchFileExpression; - bool needsRecursion; - bool isLegalFileSpec; GetFileSpecInfo ( filespecUnescaped, - out fixedDirectoryPart, - out wildcardDirectoryPart, - out filenamePart, - out matchFileExpression, - out needsRecursion, - out isLegalFileSpec + out string fixedDirectoryPart, + out string wildcardDirectoryPart, + out string filenamePart, + out string matchFileExpression, + out bool needsRecursion, + out bool isLegalFileSpec ); /* @@ -2110,7 +2097,7 @@ internal static string Normalize(string aString) var index = 0; // preserve meaningful roots and their slashes - if (aString.Length >= 2 && IsValidDriveChar(aString[0]) && aString[1] == ':') + if (aString.Length >= 2 && aString[1] == ':' && IsValidDriveChar(aString[0])) { sb.Append(aString[0]); sb.Append(aString[1]); @@ -2187,12 +2174,12 @@ private static int SkipCharacters(string aString, int startingIndex, Func /// Returns true if the given character is a valid drive letter /// - internal static bool IsValidDriveChar(char value) + private static bool IsValidDriveChar(char value) { return (value >= 'A' && value <= 'Z') || (value >= 'a' && value <= 'z'); } - static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, List excludeSpecsUnescaped) + private static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, List excludeSpecsUnescaped) { if (excludeSpecsUnescaped != null) { @@ -2235,10 +2222,8 @@ static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, /* * Analyze the file spec and get the information we need to do the matching. */ - bool stripProjectDirectory; - RecursionState state; var action = GetFileSearchData(projectDirectoryUnescaped, filespecUnescaped, - out stripProjectDirectory, out state); + out bool stripProjectDirectory, out RecursionState state); if (action == SearchAction.ReturnEmptyList) { @@ -2267,11 +2252,8 @@ static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, foreach (string excludeSpec in excludeSpecsUnescaped) { // This is ignored, we always use the include pattern's value for stripProjectDirectory - bool excludeStripProjectDirectory; - - RecursionState excludeState; var excludeAction = GetFileSearchData(projectDirectoryUnescaped, excludeSpec, - out excludeStripProjectDirectory, out excludeState); + out _, out RecursionState excludeState); if (excludeAction == SearchAction.ReturnFileSpec) { @@ -2455,7 +2437,6 @@ private static bool IsSubdirectoryOf(string possibleChild, string possibleParent } bool prefixMatch = possibleChild.StartsWith(possibleParent, StringComparison.OrdinalIgnoreCase); - if (!prefixMatch) { return false; From 28c9b7d31f81b94493e1df57e0516d83318a9fee Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 12 Feb 2021 11:03:50 +0100 Subject: [PATCH 02/13] Simplify Normalize and SkipCharacters --- src/Shared/FileMatcher.cs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 02c0e2aadcf..1dc6209430a 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -2102,7 +2102,7 @@ internal static string Normalize(string aString) sb.Append(aString[0]); sb.Append(aString[1]); - var i = SkipCharacters(aString, 2, c => FileUtilities.IsAnySlash(c)); + var i = SkipSlashes(aString, 2); if (index != i) { @@ -2114,22 +2114,22 @@ internal static string Normalize(string aString) else if (aString.StartsWith("/", StringComparison.Ordinal)) { sb.Append('/'); - index = SkipCharacters(aString, 1, c => FileUtilities.IsAnySlash(c)); + index = SkipSlashes(aString, 1); } else if (aString.StartsWith(@"\\", StringComparison.Ordinal)) { sb.Append(@"\\"); - index = SkipCharacters(aString, 2, c => FileUtilities.IsAnySlash(c)); + index = SkipSlashes(aString, 2); } else if (aString.StartsWith(@"\", StringComparison.Ordinal)) { sb.Append(@"\"); - index = SkipCharacters(aString, 1, c => FileUtilities.IsAnySlash(c)); + index = SkipSlashes(aString, 1); } while (index < aString.Length) { - var afterSlashesIndex = SkipCharacters(aString, index, c => FileUtilities.IsAnySlash(c)); + var afterSlashesIndex = SkipSlashes(aString, index); // do not append separator at the end of the string if (afterSlashesIndex >= aString.Length) @@ -2142,7 +2142,9 @@ internal static string Normalize(string aString) sb.Append(s_directorySeparator); } - var afterNonSlashIndex = SkipCharacters(aString, afterSlashesIndex, c => !FileUtilities.IsAnySlash(c)); + // skip non-slashes + var indexOfAnySlash = aString.IndexOfAny(directorySeparatorCharacters, afterSlashesIndex); + var afterNonSlashIndex = indexOfAnySlash == -1 ? aString.Length : indexOfAnySlash; sb.Append(aString, afterSlashesIndex, afterNonSlashIndex - afterSlashesIndex); @@ -2153,16 +2155,16 @@ internal static string Normalize(string aString) } /// - /// Skips characters that satisfy the condition + /// Skips slash characters in a string. /// /// The working string /// Offset in string to start the search in - /// First index that does not satisfy the condition. Returns the string's length if end of string is reached - private static int SkipCharacters(string aString, int startingIndex, Func jumpOverCharacter) + /// First index that is not a slash. Returns the string's length if end of string is reached + private static int SkipSlashes(string aString, int startingIndex) { var index = startingIndex; - while (index < aString.Length && jumpOverCharacter(aString[index])) + while (index < aString.Length && FileUtilities.IsAnySlash(aString[index])) { index++; } From f5ebb26361a8b8c42fac76d60d1b71970062e762 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 12 Feb 2021 11:53:22 +0100 Subject: [PATCH 03/13] Optimize globbing by ignoring excludes that cannot match anything --- src/Shared/FileMatcher.cs | 20 +++++++++++- src/Shared/UnitTests/FileMatcher_Tests.cs | 38 +++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 1dc6209430a..eee91652d72 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -2363,7 +2363,25 @@ private static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUn } else { - searchesToExclude.Add(excludeState); + // Optimization: ignore excludes whose file names can never match our filespec. For example, if we're looking + // for "**/*.cs", we don't have to worry about excluding "{anything}/*.sln" as the intersection of the two will + // always be empty. + string includeFilespec = state.SearchData.Filespec ?? string.Empty; + string excludeFilespec = excludeState.SearchData.Filespec ?? string.Empty; + int compareLength = Math.Min( + includeFilespec.Length - includeFilespec.LastIndexOfAny(s_wildcardCharacters) - 1, + excludeFilespec.Length - excludeFilespec.LastIndexOfAny(s_wildcardCharacters) - 1); + if (string.Compare( + includeFilespec, + includeFilespec.Length - compareLength, + excludeFilespec, + excludeFilespec.Length - compareLength, + compareLength, + StringComparison.OrdinalIgnoreCase) == 0) + { + // The suffix is the same so there is a possibility that the two will match the same files. + searchesToExclude.Add(excludeState); + } } } } diff --git a/src/Shared/UnitTests/FileMatcher_Tests.cs b/src/Shared/UnitTests/FileMatcher_Tests.cs index 418a51b91db..8c74787b754 100644 --- a/src/Shared/UnitTests/FileMatcher_Tests.cs +++ b/src/Shared/UnitTests/FileMatcher_Tests.cs @@ -409,6 +409,44 @@ public static IEnumerable GetTestData() ExpectNoMatches = NativeMethodsShared.IsLinux, } }; + + // Hits the early elimination of exclude file patterns that do not intersect with the include. + // The exclude is redundant and can be eliminated before starting the file system walk. + yield return new object[] + { + new GetFilesComplexGlobbingMatchingInfo + { + Include = @"src\foo\**\*.cs", + Excludes = new[] + { + @"src\foo\**\foo\**", + @"src\foo\**\*.vb" // redundant exclude + }, + ExpectedMatches = new[] + { + @"src\foo\foo.cs", + @"src\foo\inner\foo.cs", + @"src\foo\inner\bar\bar.cs" + }, + ExpectNoMatches = NativeMethodsShared.IsLinux, + } + }; + + // Hits the early elimination of exclude file patterns that do not intersect with the include. + // The exclude is not redundant and must not be eliminated. + yield return new object[] + { + new GetFilesComplexGlobbingMatchingInfo + { + Include = @"src\foo\**\*.cs", + Excludes = new[] + { + @"src\foo\**\*.*" // effective exclude + }, + ExpectedMatches = Array.Empty(), + ExpectNoMatches = NativeMethodsShared.IsLinux, + } + }; } } From e93ccdfa1695c2ddf364bda67d8961b6d587027c Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Thu, 11 Feb 2021 10:07:23 +0100 Subject: [PATCH 04/13] Remove unused method --- src/Shared/FileMatcher.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index eee91652d72..17528d2b90b 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -178,14 +178,6 @@ internal static bool HasWildcards(string filespec) return -1 != filespec.LastIndexOfAny(s_wildcardCharacters); } - /// - /// Determines whether the given path has any wild card characters or semicolons. - /// - internal static bool HasWildcardsOrSemicolon(string filespec) - { - return -1 != filespec.LastIndexOfAny(s_wildcardAndSemicolonCharacters); - } - /// /// Determines whether the given path has any wild card characters, any semicolons or any property references. /// From b841cdd09f4431ab5a5ea9d07ab1c652b9fff2af Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Thu, 11 Feb 2021 10:09:42 +0100 Subject: [PATCH 05/13] Remove unused parameter from IsMatch --- src/Shared/FileMatcher.cs | 26 ++++++++--------------- src/Shared/UnitTests/FileMatcher_Tests.cs | 7 +++--- src/Tasks/Unzip.cs | 4 ++-- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 17528d2b90b..df9087bd528 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -239,7 +239,7 @@ private static ImmutableArray GetAccessibleFilesAndDirectories(IFileSyst { return (ShouldEnforceMatching(pattern) ? fileSystem.EnumerateFileSystemEntries(path, pattern) - .Where(o => IsMatch(Path.GetFileName(o), pattern, true)) + .Where(o => IsMatch(Path.GetFileName(o), pattern)) : fileSystem.EnumerateFileSystemEntries(path, pattern) ).ToImmutableArray(); } @@ -322,7 +322,7 @@ bool stripProjectDirectory files = fileSystem.EnumerateFiles(dir, filespec); if (ShouldEnforceMatching(filespec)) { - files = files.Where(o => IsMatch(Path.GetFileName(o), filespec, true)); + files = files.Where(o => IsMatch(Path.GetFileName(o), filespec)); } } // If the Item is based on a relative path we need to strip @@ -385,7 +385,7 @@ string pattern directories = fileSystem.EnumerateDirectories((path.Length == 0) ? s_thisDirectory : path, pattern); if (ShouldEnforceMatching(pattern)) { - directories = directories.Where(o => IsMatch(Path.GetFileName(o), pattern, true)); + directories = directories.Where(o => IsMatch(Path.GetFileName(o), pattern)); } } @@ -901,7 +901,7 @@ struct RecursionState for (int i = 0; i < excludeNextSteps.Length; i++) { if (excludeNextSteps[i].NeedsDirectoryRecursion && - (excludeNextSteps[i].DirectoryPattern == null || IsMatch(Path.GetFileName(subdir), excludeNextSteps[i].DirectoryPattern, true))) + (excludeNextSteps[i].DirectoryPattern == null || IsMatch(Path.GetFileName(subdir), excludeNextSteps[i].DirectoryPattern))) { RecursionState thisExcludeStep = searchesToExclude[i]; thisExcludeStep.BaseDirectory = subdir; @@ -1017,7 +1017,7 @@ private static bool MatchFileRecursionStep(RecursionState recursionState, string { if (recursionState.SearchData.Filespec != null) { - return IsMatch(Path.GetFileName(file), recursionState.SearchData.Filespec, true); + return IsMatch(Path.GetFileName(file), recursionState.SearchData.Filespec); } // if no file-spec provided, match the file to the regular expression @@ -1596,8 +1596,7 @@ internal Result() /// /// String which is matched against the pattern. /// Pattern against which string is matched. - /// Determines whether ignoring case when comparing two characters - internal static bool IsMatch(string input, string pattern, bool ignoreCase) + internal static bool IsMatch(string input, string pattern) { if (input == null) { @@ -1695,10 +1694,7 @@ bool CompareIgnoreCase(char inputChar, char patternChar, int iIndex, int pIndex) break; } // If the tail doesn't match, we can safely return e.g. ("aaa", "*b") - if (( - (!ignoreCase && input[inputTailIndex] != pattern[patternTailIndex]) || - (ignoreCase && !CompareIgnoreCase(input[inputTailIndex], pattern[patternTailIndex], patternTailIndex, inputTailIndex)) - ) && + if (!CompareIgnoreCase(input[inputTailIndex], pattern[patternTailIndex], patternTailIndex, inputTailIndex) && pattern[patternTailIndex] != '?') { return false; @@ -1718,9 +1714,7 @@ bool CompareIgnoreCase(char inputChar, char patternChar, int iIndex, int pIndex) // The ? wildcard cannot be skipped as we will have a wrong result for e.g. ("aab" "*?b") if (pattern[patternIndex] != '?') { - while ( - (!ignoreCase && input[inputIndex] != pattern[patternIndex]) || - (ignoreCase && !CompareIgnoreCase(input[inputIndex], pattern[patternIndex], inputIndex, patternIndex))) + while (!CompareIgnoreCase(input[inputIndex], pattern[patternIndex], inputIndex, patternIndex)) { // Return if there is no character that match e.g. ("aa", "*b") if (++inputIndex >= inputLength) @@ -1735,9 +1729,7 @@ bool CompareIgnoreCase(char inputChar, char patternChar, int iIndex, int pIndex) } // If we have a match, step to the next character - if ( - (!ignoreCase && input[inputIndex] == pattern[patternIndex]) || - (ignoreCase && CompareIgnoreCase(input[inputIndex], pattern[patternIndex], inputIndex, patternIndex)) || + if (CompareIgnoreCase(input[inputIndex], pattern[patternIndex], inputIndex, patternIndex) || pattern[patternIndex] == '?') { patternIndex++; diff --git a/src/Shared/UnitTests/FileMatcher_Tests.cs b/src/Shared/UnitTests/FileMatcher_Tests.cs index 8c74787b754..e58f08d6616 100644 --- a/src/Shared/UnitTests/FileMatcher_Tests.cs +++ b/src/Shared/UnitTests/FileMatcher_Tests.cs @@ -524,10 +524,9 @@ public void WildcardMatching() { try { - Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1, input.Item2, false)); - Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1, input.Item2, true)); - Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1.ToUpperInvariant(), input.Item2, true)); - Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1, input.Item2.ToUpperInvariant(), true)); + Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1, input.Item2)); + Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1.ToUpperInvariant(), input.Item2)); + Assert.Equal(input.Item3, FileMatcher.IsMatch(input.Item1, input.Item2.ToUpperInvariant())); } catch (Exception) { diff --git a/src/Tasks/Unzip.cs b/src/Tasks/Unzip.cs index 4e00a677831..01544039194 100644 --- a/src/Tasks/Unzip.cs +++ b/src/Tasks/Unzip.cs @@ -243,12 +243,12 @@ private bool ShouldSkipEntry(ZipArchiveEntry zipArchiveEntry) if (_includePatterns.Length > 0) { - result = _includePatterns.All(pattern => !FileMatcher.IsMatch(FileMatcher.Normalize(zipArchiveEntry.FullName), pattern, true)); + result = _includePatterns.All(pattern => !FileMatcher.IsMatch(FileMatcher.Normalize(zipArchiveEntry.FullName), pattern)); } if (_excludePatterns.Length > 0) { - result |= _excludePatterns.Any(pattern => FileMatcher.IsMatch(FileMatcher.Normalize(zipArchiveEntry.FullName), pattern, true)); + result |= _excludePatterns.Any(pattern => FileMatcher.IsMatch(FileMatcher.Normalize(zipArchiveEntry.FullName), pattern)); } return result; From 601d097f5b3bf065585789071ac5be2cbf7a97d8 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Thu, 11 Feb 2021 17:11:19 +0100 Subject: [PATCH 06/13] Don't use regex to match patterns similar to **/.*/** --- src/Shared/FileMatcher.cs | 83 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 3 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index df9087bd528..d4d45a924c0 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -750,11 +750,13 @@ class FilesSearchData { public FilesSearchData( string filespec, // can be null + string directoryPattern, // can be null Regex regexFileMatch, // can be null bool needsRecursion ) { Filespec = filespec; + DirectoryPattern = directoryPattern; RegexFileMatch = regexFileMatch; NeedsRecursion = needsRecursion; } @@ -764,6 +766,13 @@ bool needsRecursion /// public string Filespec { get; } /// + /// Holds the directory pattern for globs like **/{pattern}/**, i.e. when we're looking for a matching directory name + /// regardless of where on the path it is. This field is used only if the wildcard directory part has this shape. In + /// other cases such as **/{pattern1}/**/{pattern2}/**, we don't use this optimization and instead rely on + /// to test if a file path matches the glob or not. + /// + public string DirectoryPattern { get; } + /// /// Wild-card matching. /// public Regex RegexFileMatch { get; } @@ -784,9 +793,18 @@ struct RecursionState /// public string RemainingWildcardDirectory; /// + /// True if SearchData.DirectoryPattern is non-null and we have descended into a directory that matches the pattern. + /// + public bool IsInsideMatchingDirectory; + /// /// Data about a search that does not change as the search recursively traverses directories /// public FilesSearchData SearchData; + + /// + /// True if a SearchData.DirectoryPattern is specified but we have not descended into a matching directory. + /// + public bool IsLookingForMatchingDirectory => (SearchData.DirectoryPattern != null && !IsInsideMatchingDirectory); } /// @@ -832,6 +850,8 @@ struct RecursionState // We can exclude all results in this folder if: if ( + // We are not looking for a directory matching the pattern given in SearchData.DirectoryPattern + !searchToExclude.IsLookingForMatchingDirectory && // We are matching files based on a filespec and not a regular expression searchToExclude.SearchData.Filespec != null && // The wildcard path portion of the excluded search matches the include search @@ -892,6 +912,12 @@ struct RecursionState newRecursionState.BaseDirectory = subdir; newRecursionState.RemainingWildcardDirectory = nextStep.RemainingWildcardDirectory; + if (newRecursionState.IsLookingForMatchingDirectory && + DirectoryEndsWithPattern(subdir, recursionState.SearchData.DirectoryPattern)) + { + newRecursionState.IsInsideMatchingDirectory = true; + } + List newSearchesToExclude = null; if (excludeNextSteps != null) @@ -906,6 +932,11 @@ struct RecursionState RecursionState thisExcludeStep = searchesToExclude[i]; thisExcludeStep.BaseDirectory = subdir; thisExcludeStep.RemainingWildcardDirectory = excludeNextSteps[i].RemainingWildcardDirectory; + if (thisExcludeStep.IsLookingForMatchingDirectory && + DirectoryEndsWithPattern(subdir, thisExcludeStep.SearchData.DirectoryPattern)) + { + thisExcludeStep.IsInsideMatchingDirectory = true; + } newSearchesToExclude.Add(thisExcludeStep); } } @@ -1038,9 +1069,14 @@ RecursionState recursionState bool considerFiles = false; // Only consider files if... - if (recursionState.RemainingWildcardDirectory.Length == 0) + if (recursionState.SearchData.DirectoryPattern != null) { - // We've reached the end of the wildcard directory elements. + // We are looking for a directory pattern and have descended into a matching directory, + considerFiles = recursionState.IsInsideMatchingDirectory; + } + else if (recursionState.RemainingWildcardDirectory.Length == 0) + { + // or we've reached the end of the wildcard directory elements, considerFiles = true; } else if (recursionState.RemainingWildcardDirectory.IndexOf(recursiveDirectoryMatch, StringComparison.Ordinal) == 0) @@ -2042,11 +2078,37 @@ enum SearchAction return SearchAction.ReturnEmptyList; } + string directoryPattern = null; + if (wildcardDirectoryPart.Length > 0) + { + // If the wildcard directory part looks like "**/{pattern}/**", we are essentially looking for files that have + // a matching directory anywhere on their path. This is commonly used when excluding hidden directories using + // "**/.*/**" for example, and is worth special-casing so it doesn't fall into the slow regex logic. + string wildcard = wildcardDirectoryPart.TrimTrailingSlashes(); + int wildcardLength = wildcard.Length; + if (wildcardLength > 6 && + wildcard[0] == '*' && + wildcard[1] == '*' && + FileUtilities.IsAnySlash(wildcard[2]) && + FileUtilities.IsAnySlash(wildcard[wildcardLength - 3]) && + wildcard[wildcardLength - 2] == '*' && + wildcard[wildcardLength - 1] == '*') + { + // Check that there are no other slashes in the wildcard. + if (wildcard.IndexOfAny(FileUtilities.Slashes, 3, wildcardLength - 6) == -1) + { + directoryPattern = wildcard.Substring(3, wildcardLength - 6); + } + } + } + // determine if we need to use the regular expression to match the files // PERF NOTE: Constructing a Regex object is expensive, so we avoid it whenever possible bool matchWithRegex = // if we have a directory specification that uses wildcards, and (wildcardDirectoryPart.Length > 0) && + // the directory pattern is not a simple "**/{pattern}/**", and + directoryPattern == null && // the specification is not a simple "**" !IsRecursiveDirectoryMatch(wildcardDirectoryPart); // then we need to use the regular expression @@ -2054,6 +2116,7 @@ enum SearchAction var searchData = new FilesSearchData( // if using the regular expression, ignore the file pattern matchWithRegex ? null : filenamePart, + directoryPattern, // if using the file pattern, ignore the regular expression matchWithRegex ? new Regex(matchFileExpression, RegexOptions.IgnoreCase) : null, needsRecursion); @@ -2338,7 +2401,8 @@ private static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUn // BaseDirectory to be the same as the exclude BaseDirectory, and change the wildcard part to be "**\" // because we don't know where the different parts of the exclude wildcard part would be matched. // Example: include="c:\git\msbuild\src\Framework\**\*.*" exclude="c:\git\msbuild\**\bin\**\*.*" - Debug.Assert(excludeState.SearchData.RegexFileMatch != null, "Expected Regex to be used for exclude file matching"); + Debug.Assert(excludeState.SearchData.RegexFileMatch != null || excludeState.SearchData.DirectoryPattern != null, + "Expected Regex or directory pattern to be used for exclude file matching"); excludeState.BaseDirectory = state.BaseDirectory; excludeState.RemainingWildcardDirectory = recursiveDirectoryMatch + s_directorySeparator; searchesToExclude.Add(excludeState); @@ -2458,6 +2522,19 @@ private static bool IsSubdirectoryOf(string possibleChild, string possibleParent } } + /// + /// Returns true if the last component of the given directory path (assumed to not have any trailing slashes) + /// matches the given pattern. + /// + /// The path to test. + /// The pattern to test against. + /// True in case of a match (e.g. directoryPath = "dir/subdir" and pattern = "s*"), false otherwise. + private static bool DirectoryEndsWithPattern(string directoryPath, string pattern) + { + int index = directoryPath.LastIndexOfAny(FileUtilities.Slashes); + return (index != -1 && IsMatch(directoryPath.Substring(index + 1), pattern)); + } + private static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch; } } From f700993d2f878d21886dde021b29b3777bb3585d Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Thu, 11 Feb 2021 23:17:26 +0100 Subject: [PATCH 07/13] Micro-optimize IsMatch::CompareIgnoreCase --- src/Shared/FileMatcher.cs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index d4d45a924c0..a821269d33c 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -1671,26 +1671,20 @@ internal static bool IsMatch(string input, string pattern) bool CompareIgnoreCase(char inputChar, char patternChar, int iIndex, int pIndex) #endif { - // We will mostly be comparing ASCII characters, check this first - if (inputChar < 128 && patternChar < 128) + // We will mostly be comparing ASCII characters, check English letters first. + char inputCharLower = (char)(inputChar | 0x20); + if (inputCharLower >= 'a' && inputCharLower <= 'z') { - if (inputChar >= 'A' && inputChar <= 'Z' && patternChar >= 'a' && patternChar <= 'z') - { - return inputChar + 32 == patternChar; - } - if (inputChar >= 'a' && inputChar <= 'z' && patternChar >= 'A' && patternChar <= 'Z') - { - return inputChar == patternChar + 32; - } - return inputChar == patternChar; + // This test covers all combinations of lower/upper as both sides are converted to lower case. + return inputCharLower == (patternChar | 0x20); } - if (inputChar > 128 && patternChar > 128) + if (inputChar < 128 || patternChar < 128) { - return string.Compare(input, iIndex, pattern, pIndex, 1, StringComparison.OrdinalIgnoreCase) == 0; + // We don't need to compare, an ASCII character cannot have its lowercase/uppercase outside the ASCII table + // and a non ASCII character cannot have its lowercase/uppercase inside the ASCII table + return inputChar == patternChar; } - // We don't need to compare, an ASCII character cannot have its lowercase/uppercase outside the ASCII table - // and a non ASCII character cannot have its lowercase/uppercase inside the ASCII table - return false; + return string.Compare(input, iIndex, pattern, pIndex, 1, StringComparison.OrdinalIgnoreCase) == 0; } #if MONO ; // The end of the CompareIgnoreCase anonymous function From f9c5c08e1b788c37abc97b2b77855d4dc36429c7 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 12 Feb 2021 10:46:50 +0100 Subject: [PATCH 08/13] Optimize FileSpecMatcherTester to match common patterns without regex'es --- src/Build/Utilities/FileSpecMatchTester.cs | 57 +++++++++++++++++----- src/Shared/FileMatcher.cs | 2 +- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/Build/Utilities/FileSpecMatchTester.cs b/src/Build/Utilities/FileSpecMatchTester.cs index 740c98b5b9b..41aaea15e97 100644 --- a/src/Build/Utilities/FileSpecMatchTester.cs +++ b/src/Build/Utilities/FileSpecMatchTester.cs @@ -13,36 +13,58 @@ namespace Microsoft.Build.Internal { private readonly string _currentDirectory; private readonly string _unescapedFileSpec; + private readonly string _filenamePattern; private readonly Regex _regex; - private FileSpecMatcherTester(string currentDirectory, string unescapedFileSpec, Regex regex) + private FileSpecMatcherTester(string currentDirectory, string unescapedFileSpec, string filenamePattern, Regex regex) { Debug.Assert(!string.IsNullOrEmpty(unescapedFileSpec)); + Debug.Assert(currentDirectory != null); _currentDirectory = currentDirectory; _unescapedFileSpec = unescapedFileSpec; + _filenamePattern = filenamePattern; _regex = regex; } public static FileSpecMatcherTester Parse(string currentDirectory, string fileSpec) { string unescapedFileSpec = EscapingUtilities.UnescapeAll(fileSpec); - Regex regex = EngineFileUtilities.FilespecHasWildcards(fileSpec) ? CreateRegex(unescapedFileSpec, currentDirectory) : null; + string filenamePattern = null; + Regex regex = null; - return new FileSpecMatcherTester(currentDirectory, unescapedFileSpec, regex); + if (EngineFileUtilities.FilespecHasWildcards(fileSpec)) + { + CreateRegexOrFilenamePattern(unescapedFileSpec, currentDirectory, out filenamePattern, out regex); + } + + return new FileSpecMatcherTester(currentDirectory, unescapedFileSpec, filenamePattern, regex); } public bool IsMatch(string fileToMatch) { Debug.Assert(!string.IsNullOrEmpty(fileToMatch)); - // check if there is a regex matching the file + // We do the matching using one of three code paths, depending on the value of _filenamePattern and _regex. if (_regex != null) { string normalizedFileToMatch = FileUtilities.GetFullPathNoThrow(Path.Combine(_currentDirectory, fileToMatch)); return _regex.IsMatch(normalizedFileToMatch); } + if (_filenamePattern != null) + { + // Check file name first as it's more likely to not match. + string filename = Path.GetFileName(fileToMatch); + if (!FileMatcher.IsMatch(filename, _filenamePattern)) + { + return false; + } + + var normalizedFileToMatch = FileUtilities.GetFullPathNoThrow(Path.Combine(_currentDirectory, fileToMatch)); + return normalizedFileToMatch.StartsWith(_currentDirectory, StringComparison.OrdinalIgnoreCase); + } + return FileUtilities.ComparePathsNoThrow(_unescapedFileSpec, fileToMatch, _currentDirectory, alwaysIgnoreCase: true); } @@ -50,17 +72,27 @@ public bool IsMatch(string fileToMatch) // without this normalization step, strings pointing outside the globbing cone would still match when they shouldn't // for example, we dont want "**/*.cs" to match "../Shared/Foo.cs" // todo: glob rooting knowledge partially duplicated with MSBuildGlob.Parse and FileMatcher.ComputeFileEnumerationCacheKey - private static Regex CreateRegex(string unescapedFileSpec, string currentDirectory) + private static void CreateRegexOrFilenamePattern(string unescapedFileSpec, string currentDirectory, out string filenamePattern, out Regex regex) { FileMatcher.Default.SplitFileSpec( - unescapedFileSpec, - out string fixedDirPart, - out string wildcardDirectoryPart, - out string filenamePart); + unescapedFileSpec, + out string fixedDirPart, + out string wildcardDirectoryPart, + out string filenamePart); if (FileUtilities.PathIsInvalid(fixedDirPart)) { - return null; + filenamePattern = null; + regex = null; + return; + } + + // Most file specs have "**" as their directory specification so we special case these and make matching faster. + if (string.IsNullOrEmpty(fixedDirPart) && FileMatcher.IsRecursiveDirectoryMatch(wildcardDirectoryPart)) + { + filenamePattern = filenamePart; + regex = null; + return; } var absoluteFixedDirPart = Path.Combine(currentDirectory, fixedDirPart); @@ -75,11 +107,12 @@ private static Regex CreateRegex(string unescapedFileSpec, string currentDirecto FileMatcher.Default.GetFileSpecInfoWithRegexObject( recombinedFileSpec, - out Regex regex, + out Regex regexObject, out bool _, out bool isLegal); - return isLegal ? regex : null; + filenamePattern = null; + regex = isLegal ? regexObject : null; } } } diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index a821269d33c..1ea5ee25531 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -2529,6 +2529,6 @@ private static bool DirectoryEndsWithPattern(string directoryPath, string patter return (index != -1 && IsMatch(directoryPath.Substring(index + 1), pattern)); } - private static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch; + internal static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch; } } From 52b84e9887736bfeb11e5a7afbefda9ed76df6c6 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 15 Feb 2021 09:45:56 +0100 Subject: [PATCH 09/13] Don't build regex string if regex is not needed --- src/Build/Globbing/MSBuildGlob.cs | 9 +++-- src/Shared/FileMatcher.cs | 43 +++++++++-------------- src/Shared/UnitTests/FileMatcher_Tests.cs | 9 +++-- 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/src/Build/Globbing/MSBuildGlob.cs b/src/Build/Globbing/MSBuildGlob.cs index 79c6abb3cb2..0420aa9edd3 100644 --- a/src/Build/Globbing/MSBuildGlob.cs +++ b/src/Build/Globbing/MSBuildGlob.cs @@ -28,11 +28,10 @@ public class MSBuildGlob : IMSBuildGlob public string FixedDirectoryPart { get; } public string WildcardDirectoryPart { get; } public string FilenamePart { get; } - public string MatchFileExpression { get; } public bool NeedsRecursion { get; } public Regex Regex { get; } - public GlobState(string globRoot, string fileSpec, bool isLegal, string fixedDirectoryPart, string wildcardDirectoryPart, string filenamePart, string matchFileExpression, bool needsRecursion, Regex regex) + public GlobState(string globRoot, string fileSpec, bool isLegal, string fixedDirectoryPart, string wildcardDirectoryPart, string filenamePart, bool needsRecursion, Regex regex) { GlobRoot = globRoot; FileSpec = fileSpec; @@ -40,7 +39,6 @@ public GlobState(string globRoot, string fileSpec, bool isLegal, string fixedDir FixedDirectoryPart = fixedDirectoryPart; WildcardDirectoryPart = wildcardDirectoryPart; FilenamePart = filenamePart; - MatchFileExpression = matchFileExpression; NeedsRecursion = needsRecursion; Regex = regex; } @@ -183,7 +181,6 @@ public static MSBuildGlob Parse(string globRoot, string fileSpec) out string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, - out string matchFileExpression, out bool needsRecursion, out bool isLegalFileSpec, (fixedDirPart, wildcardDirPart, filePart) => @@ -196,6 +193,8 @@ public static MSBuildGlob Parse(string globRoot, string fileSpec) Regex regex = null; if (isLegalFileSpec) { + string matchFileExpression = FileMatcher.RegularExpressionFromFileSpec(fixedDirectoryPart, wildcardDirectoryPart, filenamePart); + lock (s_regexCache) { s_regexCache.TryGetValue(matchFileExpression, out regex); @@ -215,7 +214,7 @@ public static MSBuildGlob Parse(string globRoot, string fileSpec) regex ??= newRegex; } } - return new GlobState(globRoot, fileSpec, isLegalFileSpec, fixedDirectoryPart, wildcardDirectoryPart, filenamePart, matchFileExpression, needsRecursion, regex); + return new GlobState(globRoot, fileSpec, isLegalFileSpec, fixedDirectoryPart, wildcardDirectoryPart, filenamePart, needsRecursion, regex); }, true); diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 1ea5ee25531..4329ca3c23b 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -1148,21 +1148,14 @@ RecursionState recursionState /// The fixed directory part. /// The wildcard directory part. /// The filename part. - /// Receives whether this pattern is legal or not. /// The regular expression string. - private static string RegularExpressionFromFileSpec + internal static string RegularExpressionFromFileSpec ( string fixedDirectoryPart, string wildcardDirectoryPart, - string filenamePart, - out bool isLegalFileSpec + string filenamePart ) { - isLegalFileSpec = IsLegalFileSpec(wildcardDirectoryPart, filenamePart); - if (!isLegalFileSpec) - { - return string.Empty; - } #if DEBUG ErrorUtilities.VerifyThrow( FileSpecRegexMinLength == FileSpecRegexParts.FixedDirGroupStart.Length @@ -1488,12 +1481,18 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start out bool isLegalFileSpec) { GetFileSpecInfo(filespec, - out _, out _, out _, - out string matchFileExpression, out needsRecursion, out isLegalFileSpec); - - regexFileMatch = isLegalFileSpec - ? new Regex(matchFileExpression, DefaultRegexOptions) - : null; + out string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, + out needsRecursion, out isLegalFileSpec); + + if (isLegalFileSpec) + { + string matchFileExpression = RegularExpressionFromFileSpec(fixedDirectoryPart, wildcardDirectoryPart, filenamePart); + regexFileMatch = new Regex(matchFileExpression, DefaultRegexOptions); + } + else + { + regexFileMatch = null; + } } internal delegate (string fixedDirectoryPart, string recursiveDirectoryPart, string fileNamePart) FixupParts( @@ -1508,7 +1507,6 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start /// Receives the fixed directory part. /// Receives the wildcard directory part. /// Receives the filename part. - /// Receives the regular expression. /// Receives the flag that is true if recursion is required. /// Receives the flag that is true if the filespec is legal. /// hook method to further change the parts @@ -1517,7 +1515,6 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start out string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, - out string matchFileExpression, out bool needsRecursion, out bool isLegalFileSpec, FixupParts fixupParts = null) @@ -1526,7 +1523,6 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start fixedDirectoryPart = String.Empty; wildcardDirectoryPart = String.Empty; filenamePart = String.Empty; - matchFileExpression = null; if (!RawFileSpecIsValid(filespec)) { @@ -1548,14 +1544,10 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start filenamePart = newParts.fileNamePart; } - /* - * Get a regular expression for matching files that will be found. - */ - matchFileExpression = RegularExpressionFromFileSpec(fixedDirectoryPart, wildcardDirectoryPart, filenamePart, out isLegalFileSpec); - /* * Was the filespec valid? If not, then just return now. */ + isLegalFileSpec = IsLegalFileSpec(wildcardDirectoryPart, filenamePart); if (!isLegalFileSpec) { return; @@ -2025,7 +2017,6 @@ enum SearchAction out string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, - out string matchFileExpression, out bool needsRecursion, out bool isLegalFileSpec ); @@ -2040,11 +2031,11 @@ enum SearchAction // The projectDirectory is not null only if we are running the evaluation from // inside the engine (i.e. not from a task) + string oldFixedDirectoryPart = fixedDirectoryPart; if (projectDirectoryUnescaped != null) { if (fixedDirectoryPart != null) { - string oldFixedDirectoryPart = fixedDirectoryPart; try { fixedDirectoryPart = Path.Combine(projectDirectoryUnescaped, fixedDirectoryPart); @@ -2112,7 +2103,7 @@ enum SearchAction matchWithRegex ? null : filenamePart, directoryPattern, // if using the file pattern, ignore the regular expression - matchWithRegex ? new Regex(matchFileExpression, RegexOptions.IgnoreCase) : null, + matchWithRegex ? new Regex(RegularExpressionFromFileSpec(oldFixedDirectoryPart, wildcardDirectoryPart, filenamePart), RegexOptions.IgnoreCase) : null, needsRecursion); result.SearchData = searchData; diff --git a/src/Shared/UnitTests/FileMatcher_Tests.cs b/src/Shared/UnitTests/FileMatcher_Tests.cs index e58f08d6616..6e77112f21c 100644 --- a/src/Shared/UnitTests/FileMatcher_Tests.cs +++ b/src/Shared/UnitTests/FileMatcher_Tests.cs @@ -1625,7 +1625,7 @@ public void ExcludeComplexPattern(string include, string[] exclude, string[] mat "", "", "", - null, + "", false, false )] @@ -1635,7 +1635,7 @@ public void ExcludeComplexPattern(string include, string[] exclude, string[] mat "", "", "", - null, + "", false, false )] @@ -1876,10 +1876,13 @@ bool expectedIsLegalFileSpec out string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, - out string matchFileExpression, out bool needsRecursion, out bool isLegalFileSpec ); + string matchFileExpression = isLegalFileSpec + ? FileMatcher.RegularExpressionFromFileSpec(fixedDirectoryPart, wildcardDirectoryPart, filenamePart) + : string.Empty; + fixedDirectoryPart.ShouldBe(expectedFixedDirectoryPart); wildcardDirectoryPart.ShouldBe(expectedWildcardDirectoryPart); filenamePart.ShouldBe(expectedFilenamePart); From 6a0ccba9b6f3193634907116959c5149fd2bb649 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 15 Feb 2021 22:26:47 +0100 Subject: [PATCH 10/13] Don't create temporary and redundant ImmutableArray --- .../Evaluation/Context/EvaluationContext.cs | 6 +- src/Shared/FileMatcher.cs | 63 +++++++++---------- src/Shared/UnitTests/FileMatcher_Tests.cs | 23 ++++--- 3 files changed, 45 insertions(+), 47 deletions(-) diff --git a/src/Build/Evaluation/Context/EvaluationContext.cs b/src/Build/Evaluation/Context/EvaluationContext.cs index 633dd5404da..470b4f0cb1e 100644 --- a/src/Build/Evaluation/Context/EvaluationContext.cs +++ b/src/Build/Evaluation/Context/EvaluationContext.cs @@ -3,7 +3,7 @@ using System; using System.Collections.Concurrent; -using System.Collections.Immutable; +using System.Collections.Generic; using System.Threading; using Microsoft.Build.BackEnd.SdkResolution; using Microsoft.Build.FileSystem; @@ -48,7 +48,7 @@ public enum SharingPolicy /// /// Key to file entry list. Example usages: cache glob expansion and intermediary directory expansions during glob expansion. /// - private ConcurrentDictionary> FileEntryExpansionCache { get; } + private ConcurrentDictionary> FileEntryExpansionCache { get; } private EvaluationContext(SharingPolicy policy, IFileSystem fileSystem) { @@ -61,7 +61,7 @@ private EvaluationContext(SharingPolicy policy, IFileSystem fileSystem) Policy = policy; SdkResolverService = new CachingSdkResolverService(); - FileEntryExpansionCache = new ConcurrentDictionary>(); + FileEntryExpansionCache = new ConcurrentDictionary>(); FileSystem = fileSystem ?? new CachingFileSystemWrapper(FileSystems.Default); EngineFileUtilities = new EngineFileUtilities(new FileMatcher(FileSystem, FileEntryExpansionCache)); } diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 4329ca3c23b..b02f651529d 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -9,7 +9,6 @@ using System.Linq; using System.Text.RegularExpressions; using System.Collections.Generic; -using System.Collections.Immutable; using System.Threading.Tasks; using Microsoft.Build.Shared.FileSystem; using Microsoft.Build.Utilities; @@ -37,10 +36,10 @@ internal class FileMatcher internal static readonly char[] directorySeparatorCharacters = FileUtilities.Slashes; // until Cloudbuild switches to EvaluationContext, we need to keep their dependence on global glob caching via an environment variable - private static readonly Lazy>> s_cachedGlobExpansions = new Lazy>>(() => new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase)); + private static readonly Lazy>> s_cachedGlobExpansions = new Lazy>>(() => new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase)); private static readonly Lazy> s_cachedGlobExpansionsLock = new Lazy>(() => new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase)); - private readonly ConcurrentDictionary> _cachedGlobExpansions; + private readonly ConcurrentDictionary> _cachedGlobExpansions; private readonly Lazy> _cachedGlobExpansionsLock = new Lazy>(() => new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase)); /// @@ -82,7 +81,7 @@ private static class FileSpecRegexParts /// public static FileMatcher Default = new FileMatcher(FileSystems.Default, null); - public FileMatcher(IFileSystem fileSystem, ConcurrentDictionary> fileEntryExpansionCache = null) : this( + public FileMatcher(IFileSystem fileSystem, ConcurrentDictionary> fileEntryExpansionCache = null) : this( fileSystem, (entityType, path, pattern, projectDirectory, stripProjectDirectory) => GetAccessibleFileSystemEntries( fileSystem, @@ -90,12 +89,12 @@ private static class FileSpecRegexParts path, pattern, projectDirectory, - stripProjectDirectory), + stripProjectDirectory).ToArray(), fileEntryExpansionCache) { } - public FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEntries, ConcurrentDictionary> getFileSystemDirectoryEntriesCache = null) + internal FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEntries, ConcurrentDictionary> getFileSystemDirectoryEntriesCache = null) { if (Traits.Instance.MSBuildCacheFileEnumerations) { @@ -123,7 +122,7 @@ public FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEnt path, pattern, directory, - projectDirectory)); + projectDirectory).ToArray()); } return getFileSystemEntries(type, path, pattern, directory, projectDirectory); }; @@ -148,8 +147,8 @@ internal enum FileSystemEntity /// The file pattern. /// /// - /// An immutable array of filesystem entries. - internal delegate ImmutableArray GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory); + /// An enumerable of filesystem entries. + internal delegate IEnumerable GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory); internal static void ClearFileEnumerationsCache() { @@ -208,7 +207,7 @@ internal static bool HasPropertyOrItemReferences(string filespec) /// If true the project directory should be stripped /// The file system abstraction to use that implements file system operations /// - private static ImmutableArray GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) + private static IEnumerable GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) { path = FileUtilities.FixFilePath(path); switch (entityType) @@ -220,18 +219,18 @@ private static ImmutableArray GetAccessibleFileSystemEntries(IFileSystem ErrorUtilities.VerifyThrow(false, "Unexpected filesystem entity type."); break; } - return ImmutableArray.Empty; + return Array.Empty(); } /// - /// Returns an immutable array of file system entries matching the specified search criteria. Inaccessible or non-existent file + /// Returns an enumerable of file system entries matching the specified search criteria. Inaccessible or non-existent file /// system entries are skipped. /// /// /// /// The file system abstraction to use that implements file system operations - /// An immutable array of matching file system entries (can be empty). - private static ImmutableArray GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern) + /// An enumerable of matching file system entries (can be empty). + private static IEnumerable GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern) { if (fileSystem.DirectoryExists(path)) { @@ -241,7 +240,7 @@ private static ImmutableArray GetAccessibleFilesAndDirectories(IFileSyst ? fileSystem.EnumerateFileSystemEntries(path, pattern) .Where(o => IsMatch(Path.GetFileName(o), pattern)) : fileSystem.EnumerateFileSystemEntries(path, pattern) - ).ToImmutableArray(); + ); } // for OS security catch (UnauthorizedAccessException) @@ -255,7 +254,7 @@ private static ImmutableArray GetAccessibleFilesAndDirectories(IFileSyst } } - return ImmutableArray.Empty; + return Array.Empty(); } /// @@ -297,7 +296,7 @@ private static bool ShouldEnforceMatching(string searchPattern) /// /// The file system abstraction to use that implements file system operations /// Files that can be accessed. - private static ImmutableArray GetAccessibleFiles + private static IEnumerable GetAccessibleFiles ( IFileSystem fileSystem, string path, @@ -341,17 +340,17 @@ bool stripProjectDirectory files = RemoveInitialDotSlash(files); } - return files.ToImmutableArray(); + return files; } catch (System.Security.SecurityException) { // For code access security. - return ImmutableArray.Empty; + return Array.Empty(); } catch (System.UnauthorizedAccessException) { // For OS security. - return ImmutableArray.Empty; + return Array.Empty(); } } @@ -365,7 +364,7 @@ bool stripProjectDirectory /// Pattern to match /// The file system abstraction to use that implements file system operations /// Accessible directories. - private static ImmutableArray GetAccessibleDirectories + private static IEnumerable GetAccessibleDirectories ( IFileSystem fileSystem, string path, @@ -399,17 +398,17 @@ string pattern directories = RemoveInitialDotSlash(directories); } - return directories.ToImmutableArray(); + return directories; } catch (System.Security.SecurityException) { // For code access security. - return ImmutableArray.Empty; + return Array.Empty(); } catch (System.UnauthorizedAccessException) { // For OS security. - return ImmutableArray.Empty; + return Array.Empty(); } } @@ -500,9 +499,10 @@ GetFileSystemEntries getFileSystemEntries else { // getFileSystemEntries(...) returns an empty array if longPath doesn't exist. - ImmutableArray entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false); + IEnumerable entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false); - if (0 == entries.Length) + int entriesCount = entries.Count(); + if (0 == entriesCount) { // The next part doesn't exist. Therefore, no more of the path will exist. // Just return the rest. @@ -514,12 +514,12 @@ GetFileSystemEntries getFileSystemEntries } // Since we know there are no wild cards, this should be length one. - ErrorUtilities.VerifyThrow(entries.Length == 1, + ErrorUtilities.VerifyThrow(entriesCount == 1, "Unexpected number of entries ({3}) found when enumerating '{0}' under '{1}'. Original path was '{2}'", - parts[i], longPath, path, entries.Length); + parts[i], longPath, path, entriesCount); // Entries[0] contains the full path. - longPath = entries[0]; + longPath = entries.First(); // We just want the trailing node. longParts[i - startingElement] = Path.GetFileName(longPath); @@ -1892,7 +1892,7 @@ internal string[] GetFiles var enumerationKey = ComputeFileEnumerationCacheKey(projectDirectoryUnescaped, filespecUnescaped, excludeSpecsUnescaped); - ImmutableArray files; + IReadOnlyList files; if (!_cachedGlobExpansions.TryGetValue(enumerationKey, out files)) { // avoid parallel evaluations of the same wildcard by using a unique lock for each wildcard @@ -1908,8 +1908,7 @@ internal string[] GetFiles GetFilesImplementation( projectDirectoryUnescaped, filespecUnescaped, - excludeSpecsUnescaped) - .ToImmutableArray()); + excludeSpecsUnescaped)); } } } diff --git a/src/Shared/UnitTests/FileMatcher_Tests.cs b/src/Shared/UnitTests/FileMatcher_Tests.cs index 6e77112f21c..e44680aac5b 100644 --- a/src/Shared/UnitTests/FileMatcher_Tests.cs +++ b/src/Shared/UnitTests/FileMatcher_Tests.cs @@ -5,7 +5,6 @@ using Shouldly; using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Text.RegularExpressions; @@ -542,7 +541,7 @@ public void WildcardMatching() * Simulate Directories.GetFileSystemEntries where file names are short. * */ - private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) + private static IReadOnlyList GetFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) { if ( @@ -550,7 +549,7 @@ private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSyste && (@"D:\" == path || @"\\server\share\" == path || path.Length == 0) ) { - return ImmutableArray.Create(Path.Combine(path, "LongDirectoryName")); + return new string[] { Path.Combine(path, "LongDirectoryName") }; } else if ( @@ -558,7 +557,7 @@ private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSyste && (@"D:\LongDirectoryName" == path || @"\\server\share\LongDirectoryName" == path || @"LongDirectoryName" == path) ) { - return ImmutableArray.Create(Path.Combine(path, "LongSubDirectory")); + return new string[] { Path.Combine(path, "LongSubDirectory") }; } else if ( @@ -566,7 +565,7 @@ private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSyste && (@"D:\LongDirectoryName\LongSubDirectory" == path || @"\\server\share\LongDirectoryName\LongSubDirectory" == path || @"LongDirectoryName\LongSubDirectory" == path) ) { - return ImmutableArray.Create(Path.Combine(path, "LongFileName.txt")); + return new string[] { Path.Combine(path, "LongFileName.txt") }; } else if ( @@ -574,7 +573,7 @@ private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSyste && @"c:\apple\banana\tomato" == path ) { - return ImmutableArray.Create(Path.Combine(path, "pomegranate")); + return new string[] { Path.Combine(path, "pomegranate") }; } else if ( @@ -582,14 +581,14 @@ private static ImmutableArray GetFileSystemEntries(FileMatcher.FileSyste ) { // No files exist here. This is an empty directory. - return ImmutableArray.Empty; + return Array.Empty(); } else { Console.WriteLine("GetFileSystemEntries('{0}', '{1}')", path, pattern); Assert.True(false, "Unexpected input into GetFileSystemEntries"); } - return ImmutableArray.Create(""); + return new string[] { "" }; } private static readonly char S = Path.DirectorySeparatorChar; @@ -2121,7 +2120,7 @@ private void GetMatchingDirectories(string[] candidates, string path, string pat /// The path to search. /// The pattern to search (may be null) /// The matched files or folders. - internal ImmutableArray GetAccessibleFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) + internal IReadOnlyList GetAccessibleFileSystemEntries(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) { string normalizedPath = Normalize(path); @@ -2140,7 +2139,7 @@ internal ImmutableArray GetAccessibleFileSystemEntries(FileMatcher.FileS GetMatchingDirectories(_fileSet3, normalizedPath, pattern, files); } - return files.ToImmutableArray(); + return files.ToList(); } /// @@ -2393,9 +2392,9 @@ private static void MatchDriver(string filespec, string[] excludeFilespecs, stri /// /// /// Array of matching file system entries (can be empty). - private static ImmutableArray GetFileSystemEntriesLoopBack(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) + private static IReadOnlyList GetFileSystemEntriesLoopBack(FileMatcher.FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) { - return ImmutableArray.Create(Path.Combine(path, pattern)); + return new string[] { Path.Combine(path, pattern) }; } /************************************************************************************* From 6a51a75fb2ec3c3dc72d648f283381aedd910dec Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Tue, 16 Feb 2021 09:41:05 +0100 Subject: [PATCH 11/13] Cache all files and directories --- .../ProjectEvaluationContext_Tests.cs | 8 +++ src/Shared/FileMatcher.cs | 54 ++++++++++++++----- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/Build.UnitTests/Definition/ProjectEvaluationContext_Tests.cs b/src/Build.UnitTests/Definition/ProjectEvaluationContext_Tests.cs index b9329dad3ba..00dc1bb6f61 100644 --- a/src/Build.UnitTests/Definition/ProjectEvaluationContext_Tests.cs +++ b/src/Build.UnitTests/Definition/ProjectEvaluationContext_Tests.cs @@ -471,6 +471,14 @@ public void ContextDisambiguatesSameRelativeGlobsPointingOutsideDifferentProject [MemberData(nameof(ContextDisambiguatesRelativeGlobsData))] public void ContextDisambiguatesAFullyQualifiedGlobPointingInAnotherRelativeGlobsCone(EvaluationContext.SharingPolicy policy, string[][] expectedGlobExpansions) { + if (policy == EvaluationContext.SharingPolicy.Shared) + { + // This test case has a dependency on our glob expansion caching policy. If the evaluation context is reused + // between evaluations and files are added to the filesystem between evaluations, the cache may be returning + // stale results. Run only the Isolated variant. + return; + } + var project1Directory = _env.DefaultTestDirectory.CreateDirectory("Project1"); var project1GlobDirectory = project1Directory.CreateDirectory("Glob").CreateDirectory("1").Path; diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index b02f651529d..f444acd8c3f 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -110,21 +110,51 @@ internal FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemE _getFileSystemEntries = getFileSystemDirectoryEntriesCache == null ? getFileSystemEntries - : (type, path, pattern, directory, projectDirectory) => + : (type, path, pattern, directory, stripProjectDirectory) => { - // Cache only directories, for files we won't hit the cache because the file name patterns tend to be unique - if (type == FileSystemEntity.Directories) + if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10)) { - return getFileSystemDirectoryEntriesCache.GetOrAdd( - $"{path};{pattern ?? "*"}", - s => getFileSystemEntries( - type, - path, - pattern, - directory, - projectDirectory).ToArray()); + // New behavior: + // Always hit the filesystem with "*" pattern, cache the results, and do the filtering here. + string cacheKey = type switch + { + FileSystemEntity.Files => "F", + FileSystemEntity.Directories => "D", + FileSystemEntity.FilesAndDirectories => "A", + _ => throw new NotImplementedException() + } + ";" + path; + IReadOnlyList allEntriesForPath = getFileSystemDirectoryEntriesCache.GetOrAdd( + cacheKey, + s => getFileSystemEntries( + type, + path, + "*", + directory, + false).ToArray()); + IEnumerable filteredEntriesForPath = (pattern != null && pattern != "*") + ? allEntriesForPath.Where(o => IsMatch(Path.GetFileName(o), pattern)) + : allEntriesForPath; + return stripProjectDirectory + ? RemoveProjectDirectory(filteredEntriesForPath, directory) + : filteredEntriesForPath; + } + else + { + // Legacy behavior: + // Cache only directories, for files we won't hit the cache because the file name patterns tend to be unique + if (type == FileSystemEntity.Directories) + { + return getFileSystemDirectoryEntriesCache.GetOrAdd( + $"D;{path};{pattern ?? "*"}", + s => getFileSystemEntries( + type, + path, + pattern, + directory, + stripProjectDirectory).ToArray()); + } } - return getFileSystemEntries(type, path, pattern, directory, projectDirectory); + return getFileSystemEntries(type, path, pattern, directory, stripProjectDirectory); }; } From da0124592459d5193981038268c7c1a6facfb277 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 22 Feb 2021 13:31:37 +0100 Subject: [PATCH 12/13] Materialize enumerable early (except where guaranteed only one enumeration) --- src/Shared/FileMatcher.cs | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index f444acd8c3f..6aed15166a4 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -130,13 +130,13 @@ internal FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemE path, "*", directory, - false).ToArray()); + false)); IEnumerable filteredEntriesForPath = (pattern != null && pattern != "*") ? allEntriesForPath.Where(o => IsMatch(Path.GetFileName(o), pattern)) : allEntriesForPath; return stripProjectDirectory - ? RemoveProjectDirectory(filteredEntriesForPath, directory) - : filteredEntriesForPath; + ? RemoveProjectDirectory(filteredEntriesForPath, directory).ToArray() + : filteredEntriesForPath.ToArray(); } else { @@ -178,7 +178,7 @@ internal enum FileSystemEntity /// /// /// An enumerable of filesystem entries. - internal delegate IEnumerable GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory); + internal delegate IReadOnlyList GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory); internal static void ClearFileEnumerationsCache() { @@ -237,7 +237,7 @@ internal static bool HasPropertyOrItemReferences(string filespec) /// If true the project directory should be stripped /// The file system abstraction to use that implements file system operations /// - private static IEnumerable GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) + private static IReadOnlyList GetAccessibleFileSystemEntries(IFileSystem fileSystem, FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory) { path = FileUtilities.FixFilePath(path); switch (entityType) @@ -260,7 +260,7 @@ private static IEnumerable GetAccessibleFileSystemEntries(IFileSystem fi /// /// The file system abstraction to use that implements file system operations /// An enumerable of matching file system entries (can be empty). - private static IEnumerable GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern) + private static IReadOnlyList GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern) { if (fileSystem.DirectoryExists(path)) { @@ -270,7 +270,7 @@ private static IEnumerable GetAccessibleFilesAndDirectories(IFileSystem ? fileSystem.EnumerateFileSystemEntries(path, pattern) .Where(o => IsMatch(Path.GetFileName(o), pattern)) : fileSystem.EnumerateFileSystemEntries(path, pattern) - ); + ).ToArray(); } // for OS security catch (UnauthorizedAccessException) @@ -326,7 +326,7 @@ private static bool ShouldEnforceMatching(string searchPattern) /// /// The file system abstraction to use that implements file system operations /// Files that can be accessed. - private static IEnumerable GetAccessibleFiles + private static IReadOnlyList GetAccessibleFiles ( IFileSystem fileSystem, string path, @@ -370,7 +370,7 @@ bool stripProjectDirectory files = RemoveInitialDotSlash(files); } - return files; + return files.ToArray(); } catch (System.Security.SecurityException) { @@ -394,7 +394,7 @@ bool stripProjectDirectory /// Pattern to match /// The file system abstraction to use that implements file system operations /// Accessible directories. - private static IEnumerable GetAccessibleDirectories + private static IReadOnlyList GetAccessibleDirectories ( IFileSystem fileSystem, string path, @@ -428,7 +428,7 @@ string pattern directories = RemoveInitialDotSlash(directories); } - return directories; + return directories.ToArray(); } catch (System.Security.SecurityException) { @@ -528,11 +528,10 @@ GetFileSystemEntries getFileSystemEntries } else { - // getFileSystemEntries(...) returns an empty array if longPath doesn't exist. - IEnumerable entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false); + // getFileSystemEntries(...) returns an empty enumerable if longPath doesn't exist. + IReadOnlyList entries = getFileSystemEntries(FileSystemEntity.FilesAndDirectories, longPath, parts[i], null, false); - int entriesCount = entries.Count(); - if (0 == entriesCount) + if (0 == entries.Count) { // The next part doesn't exist. Therefore, no more of the path will exist. // Just return the rest. @@ -543,13 +542,13 @@ GetFileSystemEntries getFileSystemEntries break; } - // Since we know there are no wild cards, this should be length one. - ErrorUtilities.VerifyThrow(entriesCount == 1, + // Since we know there are no wild cards, this should be length one, i.e. MoveNext should return false. + ErrorUtilities.VerifyThrow(entries.Count == 1, "Unexpected number of entries ({3}) found when enumerating '{0}' under '{1}'. Original path was '{2}'", - parts[i], longPath, path, entriesCount); + parts[i], longPath, path, entries.Count); // Entries[0] contains the full path. - longPath = entries.First(); + longPath = entries[0]; // We just want the trailing node. longParts[i - startingElement] = Path.GetFileName(longPath); From c5afa7c8d54f16596eb849e4affca7d2ff28654a Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 12 Feb 2021 13:14:01 +0100 Subject: [PATCH 13/13] Back-compat workaround --- src/Shared/FileMatcher.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Shared/FileMatcher.cs b/src/Shared/FileMatcher.cs index 6aed15166a4..0784baba45a 100644 --- a/src/Shared/FileMatcher.cs +++ b/src/Shared/FileMatcher.cs @@ -1063,8 +1063,24 @@ struct RecursionState { return Enumerable.Empty(); } + + // Back-compat hack: We don't use case-insensitive file enumeration I/O on Linux so the behavior is different depending + // on the NeedsToProcessEachFile flag. If the flag is false and matching is done within the _getFileSystemEntries call, + // it is case sensitive. If the flag is true and matching is handled with MatchFileRecursionStep, it is case-insensitive. + // TODO: Can we fix this by using case-insensitive file I/O on Linux? + string filespec; + if (NativeMethodsShared.IsLinux && recursionState.SearchData.DirectoryPattern != null) + { + filespec = "*.*"; + stepResult.NeedsToProcessEachFile = true; + } + else + { + filespec = recursionState.SearchData.Filespec; + } + IEnumerable files = _getFileSystemEntries(FileSystemEntity.Files, recursionState.BaseDirectory, - recursionState.SearchData.Filespec, projectDirectory, stripProjectDirectory); + filespec, projectDirectory, stripProjectDirectory); if (!stepResult.NeedsToProcessEachFile) {