New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Include/Exclude filtering capability to Unzip Task (#5169) #6018
Changes from 5 commits
9987940
08cb0d1
86c25c2
fbc0a4f
e71e37f
152aadc
941d7eb
658c23a
8164bfd
1f1aa6d
5e489e2
9fedb34
62d56a9
76a94f1
e9e0f5f
8608320
e4d0286
164e8e7
081c179
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,5 +214,97 @@ public void LogsErrorIfSourceFileDoesNotExist() | |
_mockEngine.Log.ShouldContain("MSB3932", () => _mockEngine.Log); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void CanUnzip_WithIncludeFilter() | ||
{ | ||
using (TestEnvironment testEnvironment = TestEnvironment.Create()) | ||
{ | ||
TransientTestFolder source = testEnvironment.CreateFolder(createFolder: true); | ||
TransientTestFolder destination = testEnvironment.CreateFolder(createFolder: false); | ||
testEnvironment.CreateFile(source, "BE78A17D30144B549D21F71D5C633F7D.txt", "file1"); | ||
testEnvironment.CreateFile(source, "A04FF4B88DF14860B7C73A8E75A4FB76.txt", "file2"); | ||
|
||
TransientZipArchive zipArchive = TransientZipArchive.Create(source, testEnvironment.CreateFolder(createFolder: true)); | ||
|
||
Unzip unzip = new Unzip | ||
{ | ||
BuildEngine = _mockEngine, | ||
DestinationFolder = new TaskItem(destination.Path), | ||
OverwriteReadOnlyFiles = true, | ||
SkipUnchangedFiles = false, | ||
SourceFiles = new ITaskItem[] { new TaskItem(zipArchive.Path) }, | ||
Include = "BE78A17D30144B549D21F71D5C633F7D" | ||
}; | ||
|
||
unzip.Execute().ShouldBeTrue(() => _mockEngine.Log); | ||
|
||
_mockEngine.Log.ShouldContain(Path.Combine(destination.Path, "BE78A17D30144B549D21F71D5C633F7D.txt"), () => _mockEngine.Log); | ||
_mockEngine.Log.ShouldNotContain(Path.Combine(destination.Path, "A04FF4B88DF14860B7C73A8E75A4FB76.txt"), () => _mockEngine.Log); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void CanUnzip_WithExcludeFilter() | ||
{ | ||
using (TestEnvironment testEnvironment = TestEnvironment.Create()) | ||
{ | ||
TransientTestFolder source = testEnvironment.CreateFolder(createFolder: true); | ||
TransientTestFolder destination = testEnvironment.CreateFolder(createFolder: false); | ||
testEnvironment.CreateFile(source, "BE78A17D30144B549D21F71D5C633F7D.txt", "file1"); | ||
testEnvironment.CreateFile(source, "A04FF4B88DF14860B7C73A8E75A4FB76.txt", "file2"); | ||
|
||
TransientZipArchive zipArchive = TransientZipArchive.Create(source, testEnvironment.CreateFolder(createFolder: true)); | ||
|
||
Unzip unzip = new Unzip | ||
{ | ||
BuildEngine = _mockEngine, | ||
DestinationFolder = new TaskItem(destination.Path), | ||
OverwriteReadOnlyFiles = true, | ||
SkipUnchangedFiles = false, | ||
SourceFiles = new ITaskItem[] { new TaskItem(zipArchive.Path) }, | ||
Exclude = "BE78A17D30144B549D21F71D5C633F7D" | ||
}; | ||
|
||
unzip.Execute().ShouldBeTrue(() => _mockEngine.Log); | ||
|
||
_mockEngine.Log.ShouldNotContain(Path.Combine(destination.Path, "BE78A17D30144B549D21F71D5C633F7D.txt"), () => _mockEngine.Log); | ||
_mockEngine.Log.ShouldContain(Path.Combine(destination.Path, "A04FF4B88DF14860B7C73A8E75A4FB76.txt"), () => _mockEngine.Log); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void CanUnzip_WithIncludeAndExcludeFilter() | ||
{ | ||
using (TestEnvironment testEnvironment = TestEnvironment.Create()) | ||
{ | ||
TransientTestFolder source = testEnvironment.CreateFolder(createFolder: true); | ||
TransientTestFolder destination = testEnvironment.CreateFolder(createFolder: false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind modifying this test to use wildcards such that two are included, of which one is also excluded; a third is also excluded; and a fourth isn't excluded or included? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not at all, I hope this new version is more what you were looking for? |
||
testEnvironment.CreateFile(source, "file1.js", "file1"); | ||
testEnvironment.CreateFile(source, "file1.js.map", "file2"); | ||
testEnvironment.CreateFile(source, "file2.js", "file3"); | ||
testEnvironment.CreateFile(source, "readme.txt", "file4"); | ||
|
||
TransientZipArchive zipArchive = TransientZipArchive.Create(source, testEnvironment.CreateFolder(createFolder: true)); | ||
|
||
Unzip unzip = new Unzip | ||
{ | ||
BuildEngine = _mockEngine, | ||
DestinationFolder = new TaskItem(destination.Path), | ||
OverwriteReadOnlyFiles = true, | ||
SkipUnchangedFiles = false, | ||
SourceFiles = new ITaskItem[] { new TaskItem(zipArchive.Path) }, | ||
Include = ".*?\\.js", | ||
Exclude = ".*?\\.js\\.map" | ||
}; | ||
|
||
unzip.Execute().ShouldBeTrue(() => _mockEngine.Log); | ||
|
||
_mockEngine.Log.ShouldContain(Path.Combine(destination.Path, "file1.js"), () => _mockEngine.Log); | ||
_mockEngine.Log.ShouldNotContain(Path.Combine(destination.Path, "file1.js.map"), () => _mockEngine.Log); | ||
_mockEngine.Log.ShouldContain(Path.Combine(destination.Path, "file2.js"), () => _mockEngine.Log); | ||
_mockEngine.Log.ShouldNotContain(Path.Combine(destination.Path, "readme.txt"), () => _mockEngine.Log); | ||
} | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we stick with regular expressions for this, we should change the names for
Exclude
andInclude
as they are "default" msbuild names. ie. items are added via Include and the patterns differ between them and regular expressions. A suggested name during PR review wasIncludePattern
&ExcludePattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, the functionality differs indeed. I have pushed the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benvillalobos, I thought we'd agreed globs were more MSBuild-y than regex? Confused by your comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did, I posted this while we were talking about it, hence the "If we stick with regular expressions." The current implementation is still regex, is this how we process includes and excludes normally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No; we use globs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benvillalobos ok, thank you for the clearing up. Is there a good example in MSBuild of how you'd like this to function I can use as a lead for the implementation? Just want to make sure that it feels native without any quirks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out
Expander.cs
,ExpandIntoItemsLeaveEscaped
may have what you're looking for.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ItemSpec.cs
is also relevant here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @benvillalobos , I studied Expander and ItemSpec but they were outside of reach for the Tasks assembly and I did not want to introduce dependencies so I leveraged FileMatcher to handle the normalization and verification of the globs and paths. It does not support property references due to this. I hope this matches with your expected behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSBuildGlob would have been great here, but unfortunately it's not visible by tasks. :(