Skip to content

/ rust-harfbuzz

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

Build harfbuzz crate in CI. #138

Closed
wants to merge 6 commits into from
Closed

Build harfbuzz crate in CI. #138

wants to merge 6 commits into from
+10 −1

Conversation

@jdm
Copy link
Member

jdm commented Mar 27, 2019 •
edited by larsbergstrom

This change is Reviewable
@jdm
Build harfbuzz crate in CI.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
0926bab
@jdm
Copy link
Member Author

jdm commented Mar 27, 2019

This does not actually help with #137, but I noticed that we aren't building the harfbuzz crate in CI at all.
@jdm
Add CI check for building package.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9241f00
@jdm
Copy link
Member Author

jdm commented Mar 27, 2019

The latest commit attempts to address #137 by verifying the package creation works with HARFBUZZ_SYS_NO_PKG_CONFIG=1.
@spl
Copy link
Contributor

spl commented Mar 27, 2019

This seems to be along the right lines. Can you unpack the tarball created by cargo package and build it?
@jdm
Copy link
Member Author

jdm commented Mar 27, 2019

I believe that is already what happens as part of cargo package. I'm waiting for CI to confirm this theory.
@spl
Copy link
Contributor

spl commented Mar 27, 2019 •
edited

Nice. I had to read the package documentation quite thoroughly to find where that was mentioned.
@jdm
Copy link
Member Author

jdm commented Mar 27, 2019

Excellent, cargo package breaks in CI, so we can reproduce the problem from #137.
jdm added 4 commits Mar 27, 2019
@jdm
Typo.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4b6133b
@jdm
Include all makefiles in package.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4ed8475
@jdm
Try packaging without the manifest path
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
fd1e9b7
@jdm
Exclude only test font data.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9c346aa
@jdm
Copy link
Member Author

jdm commented Mar 28, 2019

Wooo, green build.
@jdm
Copy link
Member Author

jdm commented Mar 28, 2019 

59.22s$ cargo package
   Packaging harfbuzz-sys v0.3.1 (/home/travis/build/servo/rust-harfbuzz/harfbuzz-sys)
   Verifying harfbuzz-sys v0.3.1 (/home/travis/build/servo/rust-harfbuzz/harfbuzz-sys)
    Updating crates.io index
   Compiling cc v1.0.32
   Compiling pkg-config v0.3.14
   Compiling libc v0.2.50
   Compiling cmake v0.1.36
   Compiling servo-freetype-sys v4.0.5
   Compiling harfbuzz-sys v0.3.1 (/home/travis/build/servo/rust-harfbuzz/target/package/harfbuzz-sys-0.3.1)
   Compiling freetype v0.4.1
    Finished dev [unoptimized + debuginfo] target(s) in 59.19s
error: failed to verify package tarball
Caused by:
  Source directory was modified by build.rs during cargo publish. Build scripts should not modify anything outside of OUT_DIR. Modified file: /home/travis/build/servo/rust-harfbuzz/target/package/harfbuzz-sys-0.3.1/harfbuzz/src/hb-ot-shape-complex-myanmar-machine.hh
To proceed despite this, pass the `--no-verify` flag.
The command "cargo package" exited with 101.
@spl
Copy link
Contributor

spl commented Mar 28, 2019 

Source directory was modified by build.rs during cargo publish.
Build scripts should not modify anything outside of OUT_DIR.
Modified file: .../harfbuzz/src/hb-ot-shape-complex-myanmar-machine.hh

Huh. That's strange. I just cargo packaged your branch locally: no error and no change in the modification time of hb-ot-shape-complex-myanmar-machine.hh.
@spl
Copy link
Contributor

spl commented Mar 28, 2019

I noticed that you have the following in the .travis.yml change:

env:
 - HARFBUZZ_SYS_NO_PKG_CONFIG: 1

I believe this creates a single run with HARFBUZZ_SYS_NO_PKG_CONFIG=1. Should you also have an env option without HARFBUZZ_SYS_NO_PKG_CONFIG=1 to test that aspect as well?
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2019

☔ The latest upstream changes (presumably #140) made this pull request unmergeable. Please resolve the merge conflicts.
This was referenced Apr 2, 2019
Closed
Merged
bors-servo added a commit that referenced this pull request Apr 8, 2019
@bors-servo
Auto merge of #144 - spl:fix-test-excludes, r=jdm
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
68a5ac1 
Exclude only test font data

This is a key part taken from #138 and #142. I'm creating a new PR here for just this since those others have been combined with other, somewhat orthogonal issues. I think it's best to go ahead and merge this and make a release, since this issue is affecting others. I'll continue working on the other issues.

<!-- 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/144)
<!-- Reviewable:end -->
@jdm
Copy link
Member Author

jdm commented Apr 16, 2019

Continued in #142.
@jdm jdm closed this Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@jdm @spl @bors-servo
You can’t perform that action at this time.