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

[vcpkg] Fix vcpkg_acquire_msys failing on path with spaces #13104

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

SupSuper
Copy link
Contributor

Describe the pull request

  • What does your PR fix? Fixes #

If vcpkg is installed on a path with spaces, packages that need to install MSYS2 fail due to unquoted paths in vcpkg_acquire_msys.

This adds the missing quotes where appropriate.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

N/A

Yes

Make sure to quote the KEYRING_PATH
@Neumann-A
Copy link
Contributor

Just saying: If your path has spaces not only msys install will fail but probably also configure and make. That is why it will throw an hard error in vcpkg_configure_make if it detects a space in the vcpkg root path.

@PhoebeHui PhoebeHui added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Aug 24, 2020
@@ -107,15 +107,15 @@ function(vcpkg_acquire_msys PATH_TO_ROOT_OUT)
)
# install the new keyring
_execute_process(
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;pacman-key --verify ${KEYRING_SIG_PATH}"
COMMAND ${PATH_TO_ROOT}/usr/bin/bash.exe --noprofile --norc -c "PATH=/usr/bin;pacman-key --verify \"${KEYRING_SIG_PATH}\""
WORKING_DIRECTORY ${TOOLPATH}
Copy link
Member

Choose a reason for hiding this comment

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

Stupid question: Does this WORKING_DIRECTORY also need quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my testing only the KEYRING_SIG_PATH needed quotes since it's a command string passed to bash. The rest are handled by CMake.

@SupSuper
Copy link
Contributor Author

@Neumann-A I tested this with fribidi which uses vcpkg_acquire_msys for meson. I didn't get any other errors so I assume it doesn't depend on configure/make.

I realize that vcpkg on a path with spaces is a constant struggle, but I figure any fix is better than none. :)

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

LGTM

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Aug 25, 2020
@BillyONeal BillyONeal merged commit 42a90d3 into microsoft:master Aug 25, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

remz1337 pushed a commit to remz1337/vcpkg that referenced this pull request Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants