-
-
Notifications
You must be signed in to change notification settings - Fork 335
add extra libs for curl (psl, krb5, unistring, idn2) #968
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
This reverts commit a1b8d20.
|
okay, musl doesn't have python? other one just needs LD_LIBRARY_PATH. |
|
I remember that the reason I didn't add these dependencies to curl in the beginning was because it had a library dependency on Python. Dependent on a language with various system-level dependency issues can be quite a headache. |
|
Yeah I think I'll remove psl. Looks like python is no longer default in Ubuntu either. |
| $this->source_dir .= '/src'; | ||
| shell()->cd($this->source_dir)->exec('autoreconf -if'); |
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.
If there is no need to modify the root directory, it is best to pass the subpath directly instead of modifying 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.
How do we pass a subpath directly? I tried searching for it, but couldn't find 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.
I mean shell()->cd("{$this->source_dir}/src")->exec() instead of modifying 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.
Not working for UnixAutoconfExecutor, that's why I changed 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.
Ah that makes sense. But I think it would be more appropriate to add an arg for executor. I would make changes in 3.0 rather than this PR. I will test later to see if it affects other functions, or we can restore it at the end.
What does this PR do?
Checklist before merging
*.phpor*.json, run them locally to ensure your changes are valid:composer cs-fixcomposer analysecomposer testbin/spc dev:sort-configsrc/globals/test-extensions.php.extension testortest extensionsto trigger full test suite.