Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd harfbuzz-sys-test using ctest #140
Merged
+108
−37
Conversation
This currently only works with HARFBUZZ_SYS_NO_PKG_CONFIG=1.
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 29, 2019
Add harfbuzz-sys-test using ctest This adds a crate to the workspace for testing `harfbuzz-sys` automatically using `ctest`. I changed a few types in `harfbuzz-sys/src/lib.rs` to make the testing automatic (i.e. to avoid creating special cases). However, I suppose `lib.rs` is generated? If so, this might be problematic if it is regenerated later. The changes seem quite sensible to me, so maybe there's an alternative to tweak the generation to get something similar? `harfbuzz-sys-test` currently only works with `HARFBUZZ_SYS_NO_PKG_CONFIG=1` because it doesn't get the `DEP_HARFBUZZ_INCLUDE` environment variable otherwise. The `.travis.yml` change reflects that. If #139 or something similar is merged, this special case can be removed, and the test should be run for both `HARFBUZZ_SYS_NO_PKG_CONFIG=1` and without `HARFBUZZ_SYS_NO_PKG_CONFIG`. There's a `FIXME` in `harfbuzz-sys-test/build.rs` because I don't understand the errors when these functions are not skipped. I would appreciate it if someone else would take a look at that. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-harfbuzz/140) <!-- Reviewable:end -->
|
I'm assuming the errors occur because they are per-platform APIs? What sort of errors were you encountering? |
|
|
Correct.
Things about the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
spl commentedMar 29, 2019
•
edited by larsbergstrom
This adds a crate to the workspace for testing
harfbuzz-sysautomatically usingctest.I changed a few types in
harfbuzz-sys/src/lib.rsto make the testing automatic (i.e. to avoid creating special cases). However, I supposelib.rsis generated? If so, this might be problematic if it is regenerated later. The changes seem quite sensible to me, so maybe there's an alternative to tweak the generation to get something similar?harfbuzz-sys-testcurrently only works withHARFBUZZ_SYS_NO_PKG_CONFIG=1because it doesn't get theDEP_HARFBUZZ_INCLUDEenvironment variable otherwise. The.travis.ymlchange reflects that. If #139 or something similar is merged, this special case can be removed, and the test should be run for bothHARFBUZZ_SYS_NO_PKG_CONFIG=1and withoutHARFBUZZ_SYS_NO_PKG_CONFIG.There's a
FIXMEinharfbuzz-sys-test/build.rsbecause I don't understand the errors when these functions are not skipped. I would appreciate it if someone else would take a look at that.This change is