-
Notifications
You must be signed in to change notification settings - Fork 28
Fixes init validation for Termux #260
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
Conversation
Termux does not store versioned `libvips`, so a unique Termux condition is used to specify the file to locate. The INI directive `zend.max_allowed_stack_size` also does not exist, so retrieving it returns `false`.
src/FFI.php
Outdated
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.
Very specific hacks like this feel bad. Perhaps we should try to open libvips.so.42 first, then fall back to libvips.so if that fails?
Off-topic, but do you know why termux doesn't include API numbers in library names? I'm curious.
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 think it's a good idea to check if .42 exists first before the other one. However, I'm not sure how to write it without breaking the code.
To be honest, in this commit, I just added conditions that worked for me to make this package function properly. It's a dependency of a package I use, and it was only failing to validate these conditions in Termux. It hasn't caused me any issues since.
Termux always updates libraries to the latest compatible version. I haven't seen any ways to downgrade a package. I guess that's the reason why it omits version numbers in its files.
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.
It looks like Termux needs to compile libvips with the TERMUX_MESON_ENABLE_SOVERSION=1 env set.
termux_step_pre_configure() { export TERMUX_MESON_ENABLE_SOVERSION=1 }
See for example:
https://github.com/termux/termux-packages/blob/38379b121bff426393df599b7b08554dcaea1c5f/packages/libmpdclient/build.sh#L15-L17
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 just opened PR termux/termux-packages#24153 for this.
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.
PR #261 handles the directive INI check in a different way. So I revert the lines in that part.
I think this is now resolved from the php-vips side. We just need the soname change in termux.
I suppose we could test for libvips.so, but I'd like to avoid this if possible.
Most FFI-based libvips bindings expect version-suffixed SONAMEs. See: libvips/php-vips#260 Co-authored-by: TomIO <tomjo00@web.de>
I have just merged this change over on the Termux side.
SO versioning will be enabled for the package starting with libvips 8.16.1-1 in Termux
That's great! Thank you @TomJo2000
I'll close here. @pizcadesaber, please open a new issue if this isn't resolved.
Most FFI-based libvips bindings expect version-suffixed SONAMEs. See: libvips/php-vips#260 Co-authored-by: TomIO <tomjo00@web.de>
Termux does not store versioned
libvips, so a unique Termux condition is used to specify the file to locate.The INI directive
zend.max_allowed_stack_sizealso does not exist, so retrieving it returnsfalse.