curl-rust icon indicating copy to clipboard operation
curl-rust copied to clipboard

Allow using vcpkg on any Windows target, and use find_package

Open micolous opened this issue 1 year ago • 2 comments

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 avatar Jun 27 '23 08:06 micolous

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

micolous avatar Jul 28 '23 05:07 micolous

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?

alexcrichton avatar Aug 02 '23 10:08 alexcrichton