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

Add "build-native-harfbuzz" and "build-native-freetype" features #129

Merged
merged 1 commit into from Mar 6, 2019
Merged

Add "build-native-harfbuzz" and "build-native-freetype" features #129

merged 1 commit into from Mar 6, 2019
+24 −7

Conversation

@kpozin
Copy link
Contributor

kpozin commented Mar 5, 2019 •
edited by larsbergstrom

  • Declare default features "build-native-harfbuzz" and "build-native-freetype".
  • Make the pkg-config and cmake build-dependencies optional.
  • Make build.rs a no-op if "build-native-harfbuzz" is disabled.

These changes allow the library to be used in projects where the native harfbuzz and freetype librares are built separately, and where cmake and pkg-config are not supported.

Depends on servo/rust-freetype#57.

This change is Reviewable
@jdm
jdm requested changes Mar 5, 2019
View changes
harfbuzz-sys/Cargo.toml Outdated Show resolved Hide resolved
harfbuzz/Cargo.toml Outdated Show resolved Hide resolved
@jdm jdm mentioned this pull request Mar 5, 2019
Closed
@kpozin
Add "build-native-harfbuzz" and "build-native-freetype" features
Loading status checks…
68e2727 
- Declare default features "build-native-harfbuzz" and
"build-native-freetype".
- Make the pkg-config and cmake build-dependencies optional.
- Make build.rs a no-op if "build-native-harfbuzz" is disabled.

These changes allow the library to be used in projects where the native
harfbuzz and freetype librares are built separately, and where cmake and
pkg-config are not supported.
@kpozin
Copy link
Contributor Author

kpozin commented Mar 6, 2019

I think harfbuzz-sys has to remain a required dependency because everything in servo/rust-harfbuzz/harfbuzz/src/ depends on it.

Please see the latest commit (68e2727), which reexports freetype's feature servo-freetype-sys as build-native-freetype (for consistency with build-native-harfbuzz).
@kpozin kpozin marked this pull request as ready for review Mar 6, 2019
@jdm
Copy link
Member

jdm commented Mar 6, 2019

@bors-servo r+
Looks reasonable!
@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2019

📌 Commit 68e2727 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2019

⌛ Testing commit 68e2727 with merge 8488629...
bors-servo added a commit that referenced this pull request Mar 6, 2019
@bors-servo
Auto merge of #129 - kpozin:build-native-feature, 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…
8488629 
Add "build-native-harfbuzz" and "build-native-freetype" features

- Declare default features "build-native-harfbuzz" and "build-native-freetype".
- Make the pkg-config and cmake build-dependencies optional.
- Make build.rs a no-op if "build-native-harfbuzz" is disabled.

These changes allow the library to be used in projects where the native harfbuzz and freetype librares are built separately, and where cmake and pkg-config are not supported.

Depends on servo/rust-freetype#57.

<!-- 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/129)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 6, 2019

☀️ Test successful - checks-travis
Approved by: jdm
Pushing 8488629 to master...
@bors-servo bors-servo merged commit 68e2727 into servo:master Mar 6, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@kpozin
Copy link
Contributor Author

kpozin commented Mar 6, 2019

Thanks for accepting the PR! Do you know when the crate's next scheduled release is?
@jdm
Copy link
Member

jdm commented Mar 6, 2019

It gets published whenever someone needs a new version published. I'll do that now.
@jdm jdm mentioned this pull request Mar 6, 2019
Merged
bors-servo added a commit that referenced this pull request Mar 8, 2019
@bors-servo
Auto merge of #130 - servo:jdm-patch-1, r=mbrubeck
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…
a1bcd10 
Release 0.3.1.

This includes #126 and #127 which only add new APIs, and #129 which adds new features but uses them by default.

<!-- 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/130)
<!-- Reviewable:end -->
@spl
Copy link
Contributor

spl commented Mar 29, 2019

I was wondering what the purpose of build-native-harfbuzz was. Now having read this, I understand it a bit better.

(Aside: Not to be too critical, but I think the feature name is a misnomer since no building is involved if pkg-config is used.)
@spl spl mentioned this pull request Mar 29, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm

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

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