Skip to content

Commit

Permalink
Introduce IsAllFilesWildcard() and call it from MatchFileRecursionStep
Browse files Browse the repository at this point in the history
Fixes dotnet#6502

Summary

This change fixes a regression in glob matching where files without extension are erroneously not matched when taking a specific globbing code path.

Customer impact

Any customer who uses a glob pattern susceptible to the bug and has files without extensions in their source tree is affected. The bug was reported by external customers.

Regression?

Yes, caused by dotnet#6151 where glob matching was optimized which internally made it take a different code path.

Changes Made

Fixes the regression by properly handling `*.*` to mean all files, not just files with a dot in the name. This convention is used in .NET APIs on all platforms and matches the pre-regression behavior.

Testing

Added unit test coverage. Also verified locally with the repro provided by the original bug reporter.

Risk

Low. The star patterns are special-cased to mean all files, other patterns are unaffected.
  • Loading branch information
ladipro authored and rainersigwald committed Jun 8, 2021
1 parent 2fd48ab commit de3f28e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 4 deletions.
21 changes: 18 additions & 3 deletions src/Shared/FileMatcher.cs
Expand Up @@ -131,7 +131,7 @@ internal FileMatcher(IFileSystem fileSystem, GetFileSystemEntries getFileSystemE
"*",
directory,
false));
IEnumerable<string> filteredEntriesForPath = (pattern != null && pattern != "*" && pattern != "*.*")
IEnumerable<string> filteredEntriesForPath = (pattern != null && !IsAllFilesWildcard(pattern))
? allEntriesForPath.Where(o => IsMatch(Path.GetFileName(o), pattern))
: allEntriesForPath;
return stripProjectDirectory
Expand Down Expand Up @@ -886,7 +886,7 @@ struct RecursionState
// The wildcard path portion of the excluded search matches the include search
searchToExclude.RemainingWildcardDirectory == recursionState.RemainingWildcardDirectory &&
// The exclude search will match ALL filenames OR
(searchToExclude.SearchData.Filespec == "*" || searchToExclude.SearchData.Filespec == "*.*" ||
(IsAllFilesWildcard(searchToExclude.SearchData.Filespec) ||
// The exclude search filename pattern matches the include search's pattern
searchToExclude.SearchData.Filespec == recursionState.SearchData.Filespec))
{
Expand Down Expand Up @@ -1091,7 +1091,11 @@ struct RecursionState

private static bool MatchFileRecursionStep(RecursionState recursionState, string file)
{
if (recursionState.SearchData.Filespec != null)
if (IsAllFilesWildcard(recursionState.SearchData.Filespec))
{
return true;
}
else if (recursionState.SearchData.Filespec != null)
{
return IsMatch(Path.GetFileName(file), recursionState.SearchData.Filespec);
}
Expand Down Expand Up @@ -2564,6 +2568,17 @@ private static bool DirectoryEndsWithPattern(string directoryPath, string patter
return (index != -1 && IsMatch(directoryPath.Substring(index + 1), pattern));
}

/// <summary>
/// Returns true if <paramref name="pattern"/> is <code>*</code> or <code>*.*</code>.
/// </summary>
/// <param name="pattern">The filename pattern to check.</param>
private static bool IsAllFilesWildcard(string pattern) => pattern?.Length switch
{
1 => pattern[0] == '*',
3 => pattern[0] == '*' && pattern[1] == '.' && pattern[2] == '*',
_ => false
};

internal static bool IsRecursiveDirectoryMatch(string path) => path.TrimTrailingSlashes() == recursiveDirectoryMatch;
}
}
28 changes: 27 additions & 1 deletion src/Shared/UnitTests/FileMatcher_Tests.cs
Expand Up @@ -154,6 +154,7 @@ public class GetFilesComplexGlobbingMatchingInfo
@"src\bar.cs",
@"src\baz.cs",
@"src\foo\foo.cs",
@"src\foo\licence",
@"src\bar\bar.cs",
@"src\baz\baz.cs",
@"src\foo\inner\foo.cs",
Expand Down Expand Up @@ -368,7 +369,8 @@ public static IEnumerable<object[]> GetTestData()
ExpectedMatches = new[]
{
@"readme.txt",
@"licence"
@"licence",
@"src\foo\licence",
}
}
};
Expand Down Expand Up @@ -422,6 +424,30 @@ public static IEnumerable<object[]> GetTestData()
}
};

// Regression test for https://github.com/Microsoft/msbuild/issues/6502
yield return new object[]
{
new GetFilesComplexGlobbingMatchingInfo
{
Include = @"src\**",
Excludes = new[]
{
@"**\foo\**",
},
ExpectedMatches = new[]
{
@"src\foo.cs",
@"src\bar.cs",
@"src\baz.cs",
@"src\bar\bar.cs",
@"src\baz\baz.cs",
@"src\bar\inner\baz.cs",
@"src\bar\inner\baz\baz.cs",
},
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[]
Expand Down

0 comments on commit de3f28e

Please sign in to comment.