Skip to content

Fix for libargon2 not being used when compiled in #7538

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

Closed
wants to merge 3 commits into from

Conversation

mallardduck
Copy link
Contributor

@mallardduck mallardduck commented Sep 30, 2021

This adds a missing line to the ext/standard/config.m4 file. Currently the block to load libargon2 does not define HAVE_ARGON2LIB to true. The result of this is that none of the libargon2 specific features work at all.

This leads to users hitting the Unexpected failure hashing password error if the build does not include libsodium. Or if libsodium is included it this causes the "threads greater than 1 not valid error".

EDIT: PR message updated to reflect actual root issue and fix.

@mallardduck
Copy link
Contributor Author

I see that a similar change was attempted for PHP 7.4 but ultimately reverted due to the state of pkgconfig/pkg-config being hit or miss. I'm wondering if that should have improved and we should push for fixing the systems with issues - or if this should be reverted once again. 🤔

I personally think not accepting this PR and finding an alternate solution would probably be better overall. However I'm just not experienced enough to know how to improve that (other than by this revert).

@mallardduck mallardduck changed the title Revert "Use pkg-config for libargon2" Fix for libargon2 not being used when compiled in Sep 30, 2021
mallardduck added a commit to mallardduck/homebrew-php that referenced this pull request Oct 1, 2021
In PHP 8.1 argon2 lib is found using pkg-config/pkgconfig - because of this the flag no longer needs (or uses) any input values.
Having it included shouldn't hurt anything, but it also doesn't provide any affect when building PHP.

---

Additionally, the current HEAD for PHP 8.1 has a bug, but the fix for that bug is here: php/php-src#7538
Once that merges in libargon2 should work fine again - tho as this is technically a related but not-dependent change that's why I'm proposing this before that is merged in.
@nikic nikic closed this in 6d3ef57 Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants