Change BrotliCompress response files to quote file paths #26077
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
baronfel
changed the title
baronfel
force-pushed
the
fix-file-paths-in-response-files
branch
3 times, most recently
from
76b2ee2 to
77fafd2
Compare
|
This is targeted for the next 6.0.3xx servicing release to fix a regression, so it will need servicing consideration, but I still need review from code owners. |
javiercn
reviewed
Jun 22, 2022
javiercn
approved these changes
Jun 22, 2022
|
The NU1505 changes have also been validated by @wtgodbe (we think the build environment for full framework got ahead of us). We won't take those changes in 6.0.4xx and 7.0.1xx, since the core SDK problem has been fixed in those branches. |
|
#26133 covered many of the NU1505 changes, may be worth rebasing |
|
Solid point, I'll do that now. |
baronfel
force-pushed
the
fix-file-paths-in-response-files
branch
from
77fafd2 to
908a64f
Compare
|
Closing this as we are going to revert the S.CL update now. Additional reports of the regression in 6.0.3xx have triggered this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes a regression in the parsing/handling of response files (raised in #26061) by ensuring that file paths are quoted in response files generated by the brotli compression tool task. This ensures that the response files can be read by System.CommandLine, which updated and brought along this change in handling.
Given that this utility is the only report of this problem, we decided this was an acceptable fix for the issue. If it proves to be more widespread we can reimplement the old tokenization inside of Parser.cs.
If/when the other razor CLI tools migrate to System.CommandLine we'll need to do the same quoting of file paths.
Customer Impact
Users using the brotli compression feature cannot compile their applications if the application is in a path containing spaces. The user must rename containing folders to work around this at the moment.
Regression
Yes. Introduced in 6.0.302.
Testing
New tests verifying the contents of the generated response files were added for the Brotli Compression task.
Risk
The risk of taking this is low - it only impacts response file generation for the Brotli compression command, which is also authored in the SDK and not distributed anywhere else.