Skip to content

Fix crash in integrate-install powershell#965

Merged
BillyONeal merged 1 commit intomicrosoft:mainfrom
autoantwort:feature/fix-crash-2
Mar 20, 2023
Merged

Fix crash in integrate-install powershell#965
BillyONeal merged 1 commit intomicrosoft:mainfrom
autoantwort:feature/fix-crash-2

Conversation

@autoantwort
Copy link
Copy Markdown
Contributor

.string_arg("Bypass")
.string_arg("-Command")
.string_arg(fmt::format("& {& '{}' }", script_path));
.string_arg(fmt::format("& {{& '{}' }}", script_path));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for improving the identification of the fmt library. :)

For the modified content: I think it is to enable the fnt library to correctly identify a situation:

If script_path contains regular '{}', these braces will not be incorrectly recognized as placeholders in the formatted string.
eg: script_path: "/path/to/my{}script"
Before: "& /path/to/myscript"
After: "/path/to/my{}script"

Of course, using double curly braces also conforms to the escape rules of the fmt library.
But you mentioned that microsoft/vcpkg#30283 can be fixed, but I don't see such a problem in the path provided.
Can you explain it? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous code was Strings::format("& {& '%s' }", script_path), so the %s should be replaced. But in the fmt lib {} gets replaced and if you want to use a { or } without replacement, you have to double them. That was done here.

@BillyONeal BillyONeal merged commit 5b259c7 into microsoft:main Mar 20, 2023
@BillyONeal
Copy link
Copy Markdown
Member

Thanks!

@autoantwort autoantwort deleted the feature/fix-crash-2 branch March 20, 2023 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcpkg crash when integrate powershell

3 participants