Skip to content
This repository has been archived by the owner on Oct 1, 2020. It is now read-only.

use pkg-config by default, fall back to source build automatically #25

Merged
merged 7 commits into from
Sep 11, 2020

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Sep 11, 2020

@michaelkirk michaelkirk force-pushed the mkirk/pkg-config branch 2 times, most recently from 5edbdb3 to 7028b08 Compare September 11, 2020 00:47
build.rs Outdated
println!("building libproj from source");
if let Ok(val) = &env::var("_PROJ_SYS_TEST_EXPECT_BUILD_FROM_SRC") {
if val != "1" {
panic!("for testing purposes: package was building from source but should not have been");
Copy link
Member Author

@michaelkirk michaelkirk Sep 11, 2020

Choose a reason for hiding this comment

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

I wanted to test some things about the build and this is the best I could come up with.

A failure looks like this: https://travis-ci.org/github/georust/proj-sys/jobs/726133697#L2410

WDYT?

@michaelkirk michaelkirk force-pushed the mkirk/pkg-config branch 3 times, most recently from 03ec634 to 3a3deb8 Compare September 11, 2020 01:15
@michaelkirk michaelkirk force-pushed the mkirk/pkg-config branch 2 times, most recently from 889efb3 to e532b1f Compare September 11, 2020 03:19
@michaelkirk michaelkirk changed the title WIP: use pkg-config by default, fall back to source build automatically use pkg-config by default, fall back to source build automatically Sep 11, 2020
@michaelkirk michaelkirk marked this pull request as ready for review September 11, 2020 03:20
Copy link

@rmanoka rmanoka left a comment

Choose a reason for hiding this comment

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

Nice PR! Only had minor suggestions to consider.

README.md Outdated

A guide to the functions can be found here: https://proj.org/development/reference/functions.html. Run `cargo doc (optionally --open)` to generate the crate documentation.
**This is a
[`*-sys`](https://doc.rust-lang.org/cargo/reference/build-scripts.html#a-sys-packages)
Copy link

Choose a reason for hiding this comment

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

This link doesn't point correctly. Did you mean https://doc.rust-lang.org/cargo/reference/build-scripts.html#-sys-packages

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! Fixed.

src/lib.rs Outdated

//!
//! **This is a
//! [`*-sys`](https://doc.rust-lang.org/cargo/reference/build-scripts.html#a-sys-packages)
Copy link

Choose a reason for hiding this comment

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

This link also should be fixed #a- -> #-

build.rs Outdated
// Unwrap the Result and panic on failure.
.expect("Unable to generate bindings");
.and_then(|pk| {
println!("found acceptable libproj already installed at: {:?}", pk.link_paths[0]);
Copy link

Choose a reason for hiding this comment

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

Should this be eprintln! to not mess with stdout that cargo uses to pass options to rustc, etc.?

build.rs Outdated
#[cfg(not(feature = "nobuild"))]
fn main() -> Result<(), Box<dyn std::error::Error>> {
let include_path = if cfg!(feature = "bundled_proj") {
println!("feature flags specified source build");
Copy link

Choose a reason for hiding this comment

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

Should all the "info" messages print to stderr (eprintln!) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

cargo only looks at stdout lines that begin with "cargo:" - so this won't break anything, but I've switched it to eprintln. I'm fine with it either way.

@urschrei
Copy link
Member

Yep, great work. @rmanoka beat me to the prntln stuff – it won't show as output because the build script swallows it.

@michaelkirk
Copy link
Member Author

michaelkirk commented Sep 11, 2020

bors r=rmanoka

@michaelkirk michaelkirk merged commit e7a4b6a into master Sep 11, 2020
@michaelkirk michaelkirk deleted the mkirk/pkg-config branch September 30, 2020 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo features must be additive
3 participants