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 CoreText support for darwin #39502
Conversation
Success on x86_64-darwin (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: harfbuzz Partial log (click to expand)
|
I dislike adding meta attribute names to all-packages.nix. Could we just have Harfbuzz default to use CoreText on Darwin (with a config option to disable it)? Users of Harfbuzz shouldn't have to know what it's been using on the backend IIRC. If it's really needed we need at least one package using each new attribute. |
I'll remove it for now, don't want to enable it by default since it depends on frameworks. |
I wonder if we can get rid of harfbuzz-icu as well? It looks like we didn't want to enable it by default but a few packages might actually need it (IMO the override should go in the callPackage of where it's needed). |
What about harfbuzz and harfbuzzFull, that's pretty common for packages with feature flags. It avoids extra dependencies by default while still making it available. (eg. nix-index will be able to find the |
Success on x86_64-darwin (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
@@ -1,5 +1,7 @@ | |||
{ stdenv, fetchurl, pkgconfig, glib, freetype, cairo, libintl | |||
, icu, graphite2, harfbuzz # The icu variant uses and propagates the non-icu one. | |||
, ApplicationServices, CoreText | |||
, withCoreText ? false |
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.
Also see `withApple' in #39580.
Success on x86_64-darwin (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
This looks ready but there's a merge conflict now. |
Success on x86_64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: harfbuzz Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: harfbuzz Partial log (click to expand)
|
Motivation for this change
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)