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

sys: introduce bundled building #1

Merged
merged 5 commits into from
May 15, 2020
Merged

Conversation

jmagnuson
Copy link
Owner

Adds bundled source builds via CMake and the bundled feature flag. Also adds configuration for OpenSSL (openssl/openssl_bundled) and pthreads (threading).

This will be used in bundled builds.
libevent-sys/build.rs Outdated Show resolved Hide resolved
libevent-sys/build.rs Outdated Show resolved Hide resolved
@nastevens
Copy link
Collaborator

In 7325963 it'd be nice to note that the bundled version corresponds to libevent release-2.1.11-stable


[build-dependencies]
bindgen = "^0.45"
cmake = { version = "0.1", optional = true }
pkg-config = "^0.3"
Copy link
Collaborator

@nastevens nastevens May 12, 2020

Choose a reason for hiding this comment

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

Should pkg-config be optional and only enabled if not using the bundled feature?

Copy link
Owner Author

@jmagnuson jmagnuson May 13, 2020

Choose a reason for hiding this comment

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

Hmm, I think that would mean either default-features = false wouldn't be supported by libevent-sys, or introducing the need for handling features pkg-config and bundled as non-mutually-exclusive.

Update: bundled is now required to build if --no-default-features is used.

};
// TODO: Probably combine all pc includes, just to be safe.

pkg.cargo_metadata(true).probe("libevent_core").unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Owner Author

Choose a reason for hiding this comment

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

So I lifted this pattern from curl-sys thinking that it had to do with operation ordering between emitting includes and libs. But looking again, I wonder if it has to do with preventing emission of libs in case curl_config_reports_http2 fails out a few lines up. But if that's the case, why not just do the check first-thing?

@@ -24,6 +24,11 @@ fn build_libevent(libevent_path: &str) -> PathBuf {
.define("CMAKE_POLICY_DEFAULT_CMP0066", "NEW")*/
}

// TODO: Can we tap into "-vv" cargo argument?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried playing around with the following build.rs to see if anything was passed in when doing -vv, but I didn't find anything.

use std::env;

pub fn main() {
    for value in env::args() {
        println!("cargo:warning={}", value);
    }
    for (name, value) in env::vars() {
        println!("cargo:warning={}={}", name, value);
    }
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

I also drilled into cargo's CLI code for a bit and saw that the flag eventually reaches the Shell struct which handles the printing, but don't think there's any way to access that.

@jmagnuson jmagnuson requested a review from elbe0046 May 13, 2020 23:49
Adds `cmake` as a build-dependency in order to do builds directly from
libevent source. This feature flag also serves as a toggle between
building and the current method of discovery using pkg-config. This is
done to try and simplify the logical complexity of the build script.
Enables verbose CMake builds, and dumps environment variables and cargo
arguments.
If `bundled`, will add the appropriate CMake flag to the build. If not,
attempts to link with the proper library that pkg-config found.
If `bundled`, will add the appropriate CMake flag to the build. If not,
attempts to link with the proper library that pkg-config found. Also
available is the `bundled_openssl` feature, to tell `openssl-sys` to
build from source and statically link.

One caveat is that I haven't quite figured out how to separate bundled
openssl forcing pthread support (at least from the `openssl-sys` layer),
so for now `bundled_openssl` implies `threading`.
@jmagnuson jmagnuson force-pushed the feature/map-cmake-variables branch from b19326e to 044df41 Compare May 14, 2020 20:57
@jmagnuson jmagnuson merged commit 64af128 into master May 15, 2020
@jmagnuson jmagnuson deleted the feature/map-cmake-variables branch May 15, 2020 15:38
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.

None yet

3 participants