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/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/Build/Globbing/MSBuildGlob.cs b/src/Build/Globbing/MSBuildGlob.cs index 4b03541b3df..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; } @@ -117,23 +115,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 +140,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 +167,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 +176,13 @@ 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 bool needsRecursion, + out bool isLegalFileSpec, (fixedDirPart, wildcardDirPart, filePart) => { var normalizedFixedPart = NormalizeTheFixedDirectoryPartAgainstTheGlobRoot(fixedDirPart, globRoot); @@ -207,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); @@ -226,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/Build/Utilities/FileSpecMatchTester.cs b/src/Build/Utilities/FileSpecMatchTester.cs index e48fca39e77..41aaea15e97 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; @@ -12,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) { - var normalizedFileToMatch = FileUtilities.GetFullPathNoThrow(Path.Combine(_currentDirectory, fileToMatch)); + 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); } @@ -49,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); @@ -74,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 ba1fbfd01d3..0784baba45a 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; @@ -34,14 +33,13 @@ 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)); + 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)); /// @@ -83,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, @@ -91,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) { @@ -112,21 +110,51 @@ public FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemEnt _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)); + // 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)); + IEnumerable filteredEntriesForPath = (pattern != null && pattern != "*") + ? allEntriesForPath.Where(o => IsMatch(Path.GetFileName(o), pattern)) + : allEntriesForPath; + return stripProjectDirectory + ? RemoveProjectDirectory(filteredEntriesForPath, directory).ToArray() + : filteredEntriesForPath.ToArray(); + } + 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); }; } @@ -149,8 +177,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 IReadOnlyList GetFileSystemEntries(FileSystemEntity entityType, string path, string pattern, string projectDirectory, bool stripProjectDirectory); internal static void ClearFileEnumerationsCache() { @@ -179,14 +207,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. /// @@ -217,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 ImmutableArray 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) @@ -229,18 +249,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 IReadOnlyList GetAccessibleFilesAndDirectories(IFileSystem fileSystem, string path, string pattern) { if (fileSystem.DirectoryExists(path)) { @@ -248,9 +268,9 @@ 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(); + ).ToArray(); } // for OS security catch (UnauthorizedAccessException) @@ -264,7 +284,7 @@ private static ImmutableArray GetAccessibleFilesAndDirectories(IFileSyst } } - return ImmutableArray.Empty; + return Array.Empty(); } /// @@ -306,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 ImmutableArray GetAccessibleFiles + private static IReadOnlyList GetAccessibleFiles ( IFileSystem fileSystem, string path, @@ -331,7 +351,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 @@ -350,17 +370,17 @@ bool stripProjectDirectory files = RemoveInitialDotSlash(files); } - return files.ToImmutableArray(); + return files.ToArray(); } 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(); } } @@ -374,7 +394,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 IReadOnlyList GetAccessibleDirectories ( IFileSystem fileSystem, string path, @@ -394,7 +414,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)); } } @@ -408,17 +428,17 @@ string pattern directories = RemoveInitialDotSlash(directories); } - return directories.ToImmutableArray(); + return directories.ToArray(); } 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(); } } @@ -508,10 +528,10 @@ GetFileSystemEntries getFileSystemEntries } else { - // getFileSystemEntries(...) returns an empty array if longPath doesn't exist. - ImmutableArray 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); - if (0 == entries.Length) + if (0 == entries.Count) { // The next part doesn't exist. Therefore, no more of the path will exist. // Just return the rest. @@ -522,10 +542,10 @@ GetFileSystemEntries getFileSystemEntries break; } - // Since we know there are no wild cards, this should be length one. - ErrorUtilities.VerifyThrow(entries.Length == 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, entries.Length); + parts[i], longPath, path, entries.Count); // Entries[0] contains the full path. longPath = entries[0]; @@ -759,11 +779,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; } @@ -773,6 +795,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; } @@ -793,9 +822,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); } /// @@ -841,6 +879,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 @@ -901,6 +941,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) @@ -910,11 +956,16 @@ 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; thisExcludeStep.RemainingWildcardDirectory = excludeNextSteps[i].RemainingWildcardDirectory; + if (thisExcludeStep.IsLookingForMatchingDirectory && + DirectoryEndsWithPattern(subdir, thisExcludeStep.SearchData.DirectoryPattern)) + { + thisExcludeStep.IsInsideMatchingDirectory = true; + } newSearchesToExclude.Add(thisExcludeStep); } } @@ -1012,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) { @@ -1026,7 +1093,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 @@ -1047,9 +1114,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) @@ -1121,21 +1193,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 @@ -1460,19 +1525,19 @@ 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 string fixedDirectoryPart, out string wildcardDirectoryPart, out string filenamePart, + out needsRecursion, out isLegalFileSpec); - - regexFileMatch = isLegalFileSpec - ? new Regex(matchFileExpression, DefaultRegexOptions) - : null; + 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( @@ -1487,7 +1552,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 @@ -1496,7 +1560,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) @@ -1505,7 +1568,6 @@ private static int LastIndexOfDirectoryOrRecursiveSequence(string str, int start fixedDirectoryPart = String.Empty; wildcardDirectoryPart = String.Empty; filenamePart = String.Empty; - matchFileExpression = null; if (!RawFileSpecIsValid(filespec)) { @@ -1527,14 +1589,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; @@ -1611,8 +1669,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) { @@ -1651,26 +1708,20 @@ internal static bool IsMatch(string input, string pattern, bool ignoreCase) 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 @@ -1710,10 +1761,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; @@ -1733,9 +1781,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) @@ -1750,9 +1796,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++; @@ -1893,7 +1937,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 @@ -1909,8 +1953,7 @@ internal string[] GetFiles GetFilesImplementation( projectDirectoryUnescaped, filespecUnescaped, - excludeSpecsUnescaped) - .ToImmutableArray()); + excludeSpecsUnescaped)); } } } @@ -2012,21 +2055,14 @@ 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 bool needsRecursion, + out bool isLegalFileSpec ); /* @@ -2039,11 +2075,11 @@ out isLegalFileSpec // 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); @@ -2071,11 +2107,37 @@ out isLegalFileSpec 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 @@ -2083,8 +2145,9 @@ out isLegalFileSpec 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, + matchWithRegex ? new Regex(RegularExpressionFromFileSpec(oldFixedDirectoryPart, wildcardDirectoryPart, filenamePart), RegexOptions.IgnoreCase) : null, needsRecursion); result.SearchData = searchData; @@ -2110,12 +2173,12 @@ 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]); - var i = SkipCharacters(aString, 2, c => FileUtilities.IsAnySlash(c)); + var i = SkipSlashes(aString, 2); if (index != i) { @@ -2127,22 +2190,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) @@ -2155,7 +2218,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); @@ -2166,16 +2231,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++; } @@ -2187,12 +2252,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 +2300,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 +2330,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) { @@ -2370,7 +2430,8 @@ static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, // 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); @@ -2379,7 +2440,25 @@ static string[] CreateArrayWithSingleItemIfNotExcluded(string filespecUnescaped, } 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); + } } } } @@ -2455,7 +2534,6 @@ private static bool IsSubdirectoryOf(string possibleChild, string possibleParent } bool prefixMatch = possibleChild.StartsWith(possibleParent, StringComparison.OrdinalIgnoreCase); - if (!prefixMatch) { return false; @@ -2473,6 +2551,19 @@ private static bool IsSubdirectoryOf(string possibleChild, string possibleParent } } - private static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch; + /// + /// 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)); + } + + internal static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch; } } diff --git a/src/Shared/UnitTests/FileMatcher_Tests.cs b/src/Shared/UnitTests/FileMatcher_Tests.cs index 418a51b91db..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; @@ -409,6 +408,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, + } + }; } } @@ -486,10 +523,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) { @@ -505,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 ( @@ -513,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 ( @@ -521,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 ( @@ -529,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 ( @@ -537,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 ( @@ -545,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; @@ -1588,7 +1624,7 @@ public void ExcludeComplexPattern(string include, string[] exclude, string[] mat "", "", "", - null, + "", false, false )] @@ -1598,7 +1634,7 @@ public void ExcludeComplexPattern(string include, string[] exclude, string[] mat "", "", "", - null, + "", false, false )] @@ -1839,10 +1875,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); @@ -2081,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); @@ -2100,7 +2139,7 @@ internal ImmutableArray GetAccessibleFileSystemEntries(FileMatcher.FileS GetMatchingDirectories(_fileSet3, normalizedPath, pattern, files); } - return files.ToImmutableArray(); + return files.ToList(); } /// @@ -2353,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) }; } /************************************************************************************* 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;