-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Fix includes of thirdparty libs which can be unbundled on Linux #73441
Fix includes of thirdparty libs which can be unbundled on Linux #73441
Conversation
Changes `builtin_icu` and `builtin_recast` to match the folder names in `thirdparty`.
if (not all(ft_linked_deps)) and any(ft_linked_deps): # All or nothing. | ||
print( | ||
"These libraries should be either all builtin, or all system provided:\n" | ||
"freetype, libpng, zlib, graphite, harfbuzz.\n" | ||
"Please specify `builtin_<name>=no` for all of them, or none." | ||
) | ||
sys.exit() |
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.
This is a bit brittle, and I think the list is actually longer but with some of the components optional?
Namely fontconfig and msdfgen.
if not env["builtin_icu"]: | ||
env.ParseConfig("pkg-config icu-uc --cflags --libs") | ||
if not env["builtin_icu4c"]: | ||
env.ParseConfig("pkg-config icu-i18n icu-uc --cflags --libs") |
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.
uspoof_*
is in icu-i18n.
@@ -28,7 +28,7 @@ | |||
#endif | |||
|
|||
#if BASISD_SUPPORT_KTX2_ZSTD | |||
#include "../zstd/zstd.h" | |||
#include <zstd.h> |
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.
I'll make a patch file for this once I confirm the changes work, and see what upstream thinks about it.
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.
There's already an upstream PR for it: BinomialLLC/basis_universal#228
Changes
builtin_icu
andbuiltin_recast
to match the folder names inthirdparty
.Testing on Mageia together with #73359 for an upcoming update to my official
godot
RPM packages for Mageia and Fedora, with the following script:Not 100% done compiling to make sure everything works.Edit: Built a Godot 4 package for Mageia successfully.
For the record, this shaves off around 10% of the binary size, which isn't as huge as some might expect ;) (but it's not the primary reason to unbundle libraries, shared the burden on bugfixes - especially security fixes - among all packages is the goal).