improve imagick, bcmath and gmp extension warnings

[szaimen]

Fix part of #13099
Fix #22849

Signed-off-by: szaimen szaimen@e.mail.de

[MichaIng]

Not a real solution, but most appliance and NC installer developers will like it, including myself, muting one of the warnings we do not want to address intentionally for our users.

[skjnldsv]

I'm really not ok with that. Yet again a random hidden config just to tell the admin everything is fine?
The admin should know how to run a nc instance and learn what this warning means and if they can ignore it or not. Adding a config switch for a cosmetic reason is not the way to go imho

[MichaIng]

As much as I would like to mute this for our users, I have to agree with @skjnldsv, but the way the warning shows up currently does not serve the stated intention:

The admin should know how to run a nc instance and learn what this warning means

Then we should rephrase the text from a generic "highly recommended for performance and compatibility" to one that DOES explain what it is really used for. AFAIK it speeds up thumbnail, avatar and possibly other UI image resizing, compared to GD. As this is a one time step until new images are added, IMHO it is not worth it to bloat the PHP instance, considering the open security questions discussed in #13099.

Similar with bcmath and gmp, required for WebAuthn authentication but otherwise unnecessary, so the warnings are currently misleading, IMHO: #22849

[szaimen]

Then we should rephrase the text from a generic "highly recommended for performance and compatibility" to one that DOES explain what it is really used for. AFAIK it speeds up thumbnail, avatar and possibly other UI image resizing, compared to GD. As this is a one time step until new images are added, IMHO it is not worth it to bloat the PHP instance, considering the open security questions discussed in #13099.

Similar with bcmath and gmp, required for WebAuthn authentication but otherwise unnecessary, so the warnings are currently misleading, IMHO: #22849

Sounds good to me. Any pointers how and where to implement this?

[MichaIng]

At best a dedicated function like hasImagickPHPModule(), passing a dedicated DataResponse array entry (bottom of this script), which triggers a dedicated warning in the frontend: https://github.com/nextcloud/server/blob/master/core/js/setupchecks.js

There are a few tests which would need to be adjusted as well, see the changes for OPcache setup checks: https://github.com/nextcloud/server/pull/27403/files

[szaimen]

@MichaIng does 70bd008 look good to you? I will fix the tests afterwards.

[MichaIng]

LGTM, aside of some comments I left for message consistency and to make clearer that the two WebAuthn modules are required only when WebAuthn is actually used. IMHO having dedicated messages or each of these optional modules is much better than the previous generic message 👍. That way admins have a better basis to decide whether to ignore the warning or install the modules.

[MichaIng]

Approving, given the tests will be fixed 👍.

[szaimen]

Tests are fixed :)

[skjnldsv]

Nice!

[MichaIng]

/backport to stable23

Probably? 🙂

[szaimen]

/backport to stable23

Probably? 🙂

Not sure. @skjnldsv?