Skip to content
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

Still requires GMP #316

Closed
senky opened this issue Jan 11, 2021 · 6 comments · Fixed by #346
Closed

Still requires GMP #316

senky opened this issue Jan 11, 2021 · 6 comments · Fixed by #346

Comments

@senky
Copy link

senky commented Jan 11, 2021

README says:

gmp (optional but better for performance)

but the code still triggers warning:

'gmp' => '[WebPush] gmp extension is not loaded but is required for sending push notifications with payload or for VAPID authentication. You can fix this in your php.ini.',
'mbstring' => '[WebPush] mbstring extension is not loaded but is required for sending push notifications with payload or for VAPID authentication. You can fix this in your php.ini.',
'openssl' => '[WebPush] openssl extension is not loaded but is required for sending push notifications with payload or for VAPID authentication. You can fix this in your php.ini.',
];
foreach ($extensions as $extension => $message) {
if (!extension_loaded($extension)) {
trigger_error($message, E_USER_WARNING);
}
}

What's the real status with GMP?

@8Ozymandias

This comment has been minimized.

@askvortsov1
Copy link

Would also appreciate some clarity on this: not requiring GMP would be huge for adoption in my use case.

@marc1706
Copy link
Contributor

Based on my testing, web-push-php does work fine without GMP on PHP 7.3+. This is mostly due to PHP 7.3+ using openssl_pkey_derive instead of the pure PHP implementation (which does not work, see e.g. this test run: https://github.com/marc1706/web-push-php/runs/2156729472).

@askvortsov1
Copy link

Ah great, thanks for the testing! PHP 7.2 is EOL anyways, so this is excellent news.

@marc1706
Copy link
Contributor

Since I showed you that 7.2 does not work, here actually an example that unit tests start working with PHP 7.3 (only phpstan issues left): https://github.com/marc1706/web-push-php/runs/2607203285?check_suite_focus=true

@Azuxul
Copy link

Azuxul commented Jul 9, 2021

I it is possible to add a check for the php version and only require gmp in case of php<7.3 ?

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 a pull request may close this issue.

5 participants