-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
harfbuzz: add with_coretext option #20870
harfbuzz: add with_coretext option #20870
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed explanation!
This comment has been minimized.
This comment has been minimized.
@mayeut Thank you for bringing this topic in this PR. It seems be a blocker indeed, but have tried to discuss with the upstream first? Maybe |
I detected other pull requests that are modifying harfbuzz/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
@uilianries, "Maybe coretext could be enabled by default in Harfbuzz project again.", it never has been enabled by default officially, it was only enabled by default in the contrib cmake build that was used before. |
@uilianries Pinging for a review, as the issue fixed by this PR is a blocker for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Checked https://github.com/harfbuzz/harfbuzz/blob/main/CMakeLists.txt and this change makes sense.
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 2 (
Conan v2 pipeline ✔️
All green in build 1 (
|
And shouldn't you update |
Specify library name and version: harfbuzz/all
Before #13942 (switch from cmake to meson), CoreText was always enabled on macOS.
Meson is the official way to build and has CoreText disabled by default. The CMake build had a default of enabling CoreText on macOS.
This PR adds the option to enable/disable CoreText with the default being to enable it (which differs from the official default).
Why enabled by default: the pango recipe being ported to v2 in multiple PRs requires harfbuzz to be built with CoreText enabled (c.f. the v2 build logs in #20795 (comment)).
I couldn't get pango to build on macOS by disabling CoreText (by patching pango). While this might be doable, I won't have the time to investigate this option and I'm thus proposing to re-enable CoreText as a default on macOS.