Print proper cargo:include values

[spl]

In order to build a Rust crate with C/C++ that uses the HarfBuzz library referenced by the harfbuzz crate, I need to know where the header files are. I believe the standard way of doing that is the links key/value environment variables. In particular, I'm expecting DEP_HARFBUZZ_INCLUDE to tell me the include path for hb.h and other header files. Unfortunately, this doesn't seem to be working correctly now. This PR is an attempt to resolve that.

Summary of changes:

• Prints proper values for cargo:include in the build script for both the pkg-config and vendored cases.
• Changes a use of to_str().unwrap() to PathBuf::display(). I believe this is better, but I'm happy to be corrected.

As far as adding a useful feature goes, I think this PR is complete. However, there are a couple of potential improvements that would be nice, but I'm not sure how to go about them.

not(feature = "build-native-harfbuzz")

I'm not exactly sure what disabling the feature build-native-harfbuzz does. It doesn't use pkg-config and doesn't build the vendored library? Consequently I'm not sure what to print for cargo:include here.

Testing

It would be nice to test for the presence and correctness of DEP_HARFBUZZ_INCLUDE. I used the following files (in the repository's top-level directory for convenience) both with HARFBUZZ_SYS_NO_PKG_CONFIG=1 and without:

Cargo.toml:

[project]
name = "foo"
version = "0.5.0"
authors = []
build = "build.rs"

[dependencies]
harfbuzz-sys = { path = "harfbuzz-sys" }

[build-dependencies]
cc = "1.0"

build.rs:

use std::env;
fn main() {
    let mut cfg = cc::Build::new();
    for path in env::split_paths(&env::var_os("DEP_HARFBUZZ_INCLUDE").unwrap()) {
        cfg.include(path);
    }
    cfg.file("src/test.c").warnings(false).compile("test");
}

src/test.c:

#include <hb.h>
int test() {
    hb_font_t *hb_font;
    return 0;
}

src/lib.rs: empty

I'm not sure how to add such a test without simply writing a script and running it in CI. I did a quick search on GitHub and couldn't find any other projects that actually tested cargo:include in this way.

---

This change is 

[spl]

Oh, I forgot to mention one thing.

Currently, the cargo:include for a pkg-config build includes all of the harfbuzz dependencies, the vendored cargo:include includes only the vendored include directory. It seems like one would want them to be consistent. I saw other repositories with a similar story. I'm not sure what to do about it, if anything.

[spl]

Well, I've now discovered ctest, which looks like it might be useful for testing harfbuzz-sys. I'm looking at it now. Would that be something appreciated by the maintainers?

[jdm]

That seems valuable.

[spl]

I think #140 provides a satisfactory answer to the testing issue (for me, at least). Any thoughts on the other two issues (build-native-harfbuzz and consistent cargo:include values)?

BTW, I think this PR is a self-contained improvement and useful to me. If you wanted to postpone dealing with the other issues, merge this, and make a release, I wouldn't complain. 😉

[spl]

Okay, my ignorance is gradually dissipating. For the not(feature = "build-native-harfbuzz") case (#129), I cannot think of any value that can reasonably be given to cargo:include. Without a vendored build or pkg-config, we do not know where the header files are located. So, it should stay undefined as it is.

[jdm]

@bors-servo r+

[bors-servo]

📌 Commit 692048e has been approved by jdm

[bors-servo]

⌛ Testing commit 692048e with merge 62a0050...

[bors-servo]

☀️ Test successful - checks-travis
Approved by: jdm
Pushing 62a0050 to master...

[spl]

Thanks!