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

Allow using vcpkg on any Windows target, and use find_package #509

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

micolous
Copy link

@micolous micolous commented Jun 27, 2023

This change addresses two issues building for Windows:

  • Allow using vcpkg on any Windows build, not just MSVC.

    vcpkg-rs includes curl-sys as part of its integration tests, but curl-sys includes extra guards which block its use with non-MSVC targets. While it doesn't support -pc-windows-gnu (mingw) yet, its integration tests will try to build curl-sys, so these guards are a problem.

    That part of the change should have no effect for *-pc-windows-gnu, and should still fall back to existing options.

    Using vcpkg with the GNU toolchain is particularly useful for cross-compiling Windows binaries on non-Windows platforms where MSYS' version of pacman is not available. Technically you could also use vcpkg to provide non-Windows platforms' libraries, but I haven't gone that far as yet.

  • Try to use vcpkg::find_package for libcurl and its dependencies, rather than trying to guess which features it was built with.

    Those guesses are currently wrong on -pc-windows-gnu, as they assume MSVC-style library naming (so zlib is broken); but I've left these alone for now to reduce risk.

    Those guesses also result in spurious additional linkages on -pc-windows-msvc: for example, if curl was not built with libssh or openssl support, but libssh or openssl was installed on the build machine, it would result in a useless extra linkage.

While here, I've disabled fail_fast in the CI configuration, as a single-platform flake cancels tests on slower platforms (eg: MSVC).

@micolous micolous changed the title Allow running vcpkg on any Windows target Allow running vcpkg on any Windows target Jun 27, 2023
@micolous micolous changed the title Allow running vcpkg on any Windows target Allow running vcpkg on any Windows target, and use find_package Jun 28, 2023
@micolous micolous changed the title Allow running vcpkg on any Windows target, and use find_package Allow using vcpkg on any Windows target, and use find_package Jun 28, 2023
@micolous
Copy link
Author

@alexcrichton Any feedback on this? This is currently the last PR blocking mcgoo/vcpkg-rs#52, because curl-rust is in its CI configuration.

@alexcrichton
Copy link
Owner

Sorry for the delay, but I'm not confident myself signing off on this just yet. I would need to investigate the relevant packages and pieces involved here to ensure that everything works as I'd expect and I don't see any issues with this integration. I don't have the time to do this right now, though. @sagebind do you have thoughts on this though?

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.

2 participants