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

Add the ability to use pkg_config on windows build #486

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lepapareil
Copy link

@lepapareil lepapareil commented Feb 10, 2023

Closes #484

@lepapareil
Copy link
Author

Hi @alexcrichton and @sagebind can you review this change please ? Many Thks 😀.

@alexcrichton
Copy link
Owner

Sorry this is one I don't personally have the time to invest in at this time. Changing build behavior of longstanding platforms often comes with breakage perhaps months down the line that I'm not in a position to manage at this time personally.

@lepapareil
Copy link
Author

I understand.

Copy link

@micolous micolous left a comment

Choose a reason for hiding this comment

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

I made a similar change to rust-openssl recently, and this change looks reasonable to me with two suggestions:

  1. I'd only attempt using pkg-config with non-MSVC targets.

  2. Add a CI target which cross-compiles from Linux on a mingw toolchain, and running the tests in Wine.

    Unfortunately Ubuntu doesn't have a pre-packaged mingw libcurl, so you'd need to build curl from source outside of this project, disable the auto-vendoring in build.rs (to ensure you're definitely using your "system libcurl"), and setup the pkg-config paths correctly for it.

With those changes, I think this PR should be good:

  • when targeting Windows, curl-rust will continue to try using vcpkg first (so should have no effect if you use vcpkg); so this will only activate if vcpkg-rs can't find a matching package for the target.

  • pkg-config-rs by default won't allow cross-compiles (and potentially mixing host and target .pc files) unless you set up certain pkg-config environment variables for cross-compilation, or you set PKG_CONFIG_ALLOW_CROSS=1 -- so it's effectively opt-in there.

  • mingw has separate environments and prefixes for each build toolchain (eg: mingw64, clangw64, etc.) in an attempt to avoid cross-contamination.

The risk is if static-curl is disabled and try_pkgconfig somehow returned an invalid configuration (because of a misconfigured build environment), now it won't fall back to building a vendored curl from the git submodule. Setting the static-curl feature would work around this.

@@ -39,6 +39,8 @@ fn main() {
if windows {
if try_vcpkg() {
return;
} else if try_pkg_config() {
Copy link

Choose a reason for hiding this comment

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

I'd only run try_pkg_config when targeting non-MSVC targets.

Choose a reason for hiding this comment

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

Actually, on second thought, this is fine; will explain in a comment on this PR.

@micolous
Copy link

micolous commented Jul 25, 2023

It looks like pkg-config-rs with MSVC had actually been fixed up: rust-lang/pkg-config-rs#72, so it can be used on MSVC just fine (eg: rust-lang/libz-sys#39).

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.

Add the ability to use pkg_config instead of vcpkg when building with mingw64 on Windows
3 participants