Skip to content
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

Fix MSBuild Bundling When Path Contains Spaces #7677

Merged
2 commits merged into from
Apr 28, 2021

Conversation

NickGerleman
Copy link
Collaborator

@NickGerleman NickGerleman commented Apr 28, 2021

Fixes #7597

We're passing raw strings into the exec string for running commands from MSBuild. Paths with spaces will be interpreted as multiple CLI args. Wrap paths in quotes to fix paths with spaces.

Microsoft Reviewers: Open in CodeFlow

Fixes microsoft#7597

We're passing raw strings into the exec string for running commands from MSBuild. Paths with spaces will be interpreted as multiple CLI args. Wrap paths in quotes to fix paths with spaces.
@asklar
Copy link
Member

asklar commented Apr 28, 2021

main thing to watch out for is whether any of the arguments that you're adding quotes to, might themselves have quotes in them.
Ideally we don't insert the quotes ourselves and instead use something like NormalizePath or one of the Path functions to ensure the path is correctly quoted

@NickGerleman
Copy link
Collaborator Author

main thing to watch out for is whether any of the arguments that you're adding quotes to, might themselves have quotes in them.
Ideally we don't insert the quotes ourselves and instead use something like NormalizePath or one of the Path functions to ensure the path is correctly quoted

I did a (quick) scan looking for the pattern. Mostly just followed the chain of MSBuild props to where we wrap quotes. I didn't see any quote literals, and we were using MSBuild path functions.

@NickGerleman NickGerleman added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label Apr 28, 2021
@ghost
Copy link

ghost commented Apr 28, 2021

Hello @NickGerleman!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 29 minutes. No worries though, I will be back when the time is right! 😉

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit a4f65de into microsoft:master Apr 28, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Bundle AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bundle targets fail when path contains spaces
3 participants