harfbuzz: add mingw32 support #273663
harfbuzz: add mingw32 support #273663
Conversation
seanybaggins
changed the title
|
@edolstra Any chance I can snag a review :)? |
seanybaggins
force-pushed
the
add-mingw32-support-harfbuzz
branch
from
ab64bd1 to
0899577
Compare
seanybaggins
force-pushed
the
add-mingw32-support-harfbuzz
branch
2 times, most recently
from
3dbbdac to
bdb9552
Compare
seanybaggins
force-pushed
the
add-mingw32-support-harfbuzz
branch
from
bdb9552 to
85e203e
Compare
| ++ lib.optionals withCoreText [ ApplicationServices CoreText ]; | ||
|
|
||
| propagatedBuildInputs = lib.optional withGraphite2 graphite2 | ||
| ++ lib.optionals withIcu [ icu harfbuzz ]; |
There was a problem hiding this comment.
This was also causing a circular dependency when trying to build mingw. Perhaps I should do...
propagateHarfbuzz ? if stdenv.hostplatform.isMinGW then false else true
...
++ lib.optionals withIcu [ icu ]
++ lib.optionals propagateHarfbuzz [ harfbuzz ]
There was a problem hiding this comment.
It's clear from withIcu ? stdenv.hostPlatform.isMinGW that ICU support is enforced for MinGW (why?), so just pass (withIcu && !stdenv.hostPlatform.isMinGW) here.
There was a problem hiding this comment.
I had trouble getting this to compile without icu.
There was a problem hiding this comment.
Added recommended changes
| ++ lib.optionals withCoreText [ ApplicationServices CoreText ]; | ||
|
|
||
| propagatedBuildInputs = lib.optional withGraphite2 graphite2 | ||
| ++ lib.optionals withIcu [ icu harfbuzz ]; | ||
| ++ lib.optionals withIcu [ icu ]; |
There was a problem hiding this comment.
| ++ lib.optionals withIcu [ icu ]; | |
| ++ lib.optionals withIcu [ icu ] | |
| ++ lib.optionals (withIcu && !stdenv.hostPlatform.isMinGW) [ harfbuzz ] |
There was a problem hiding this comment.
Used suggestion
| @@ -18,13 +18,14 @@ | |||
| , ApplicationServices | |||
| , CoreText | |||
| , withCoreText ? false | |||
| , withIcu ? false # recommended by upstream as default, but most don't needed and it's big | |||
| , withGraphite2 ? true # it is small and major distros do include it | |||
| , withIcu ? if stdenv.hostPlatform.isMinGW then true else false | |||
There was a problem hiding this comment.
| , withIcu ? if stdenv.hostPlatform.isMinGW then true else false | |
| , withIcu ? stdenv.hostPlatform.isMinGW # recommended by upstream as default, but most don't needed and it's big |
There was a problem hiding this comment.
used suggestion
| , withGraphite2 ? true # it is small and major distros do include it | ||
| , withIcu ? if stdenv.hostPlatform.isMinGW then true else false | ||
| , withGlib ? lib.meta.availableOn stdenv.hostPlatform glib | ||
| , withGraphite2 ? lib.meta.availableOn stdenv.hostPlatform graphite2 |
There was a problem hiding this comment.
| , withGraphite2 ? lib.meta.availableOn stdenv.hostPlatform graphite2 | |
| , withGraphite2 ? lib.meta.availableOn stdenv.hostPlatform graphite2 # it is small and major distros do include it |
There was a problem hiding this comment.
Used suggestion
|
Wait, |
seanybaggins
force-pushed
the
add-mingw32-support-harfbuzz
branch
from
85e203e to
67024cb
Compare
Oof. I am not sure. My goal is to get this dependency working for qt6.qtbase. I have a hello world application working with the changes here. I may publish my overlays so the community can upstream/improve the packages |
|
Closing PR since upstream changes seemed to have implemented this better than I have when I started. Here is the repo to my overlays that I mentioned in my previous comment. |
Description of changes
Adding mingw32 host platform build support for harfbuzz. Depency update needed to add mingw32 build support for qt6. See #272538
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.