-
Notifications
You must be signed in to change notification settings - Fork 27
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
Fixes init validation for Termux #260
base: master
Are you sure you want to change the base?
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`.
throw new Exception("zend.max_allowed_stack_size not set to '-1'"); | ||
} | ||
|
||
$vips_libname = self::libraryName("libvips", 42); | ||
// Use "libvips.so" on Termux, otherwise use the standard versioned library | ||
$vips_libname = getenv('PREFIX') === '/data/data/com.termux/files/usr' |
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.
PR #261 handles the directive INI check in a different way. So I revert the lines in that part.
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 returnsfalse
.