Nextcloud-snap: Update PHP to 7.2.x

[pachulo]

It seems that Nextcloud 13.x supports PHP 7.2.x:

• https://docs.nextcloud.com/server/13/admin_manual/installation/system_requirements.html

And as per this comparison, it seems that the overall performance should be a little bit better:

• https://www.phoronix.com/scan.php?page=news_item&px=PHP-7.2-Benchmarks-RC6

[JamesBream]

FYI, PHP 7.2 removed mcrypt so us moving to it would break any apps which depend on it. Not sure how many there are out there, but I do know that the SSO & SAML app hasn't yet been updated but is listed in the NC 13 docs as being a supported app.

Seen as these snap updates get pushed out automatically, pushing this update as is could cause some headaches for anyone already using an app which relies on mcrypt.

In favour of shipping 7.2 for the performance gains, but should we perhaps consider bundling mcrypt from PECL too? I'm assuming that the current PHP configflag --with-mcrypt isn't doing this.

[kyrofa]

Excellent catch @JamesBream, wonder why --with-mcrypt isn't busting, then. We'll look into it.

[pachulo]

Thanks for the information @JamesBream ! The relevant bug is this one, I think: https://github.com/nextcloud/user_saml/issues/168
But there are more: https://github.com/nextcloud/user_saml/issues/216

[kyrofa]

Quick update (longer conversation in #618): we're holding off on this until apps (user_saml in particular) migrate away from mcrypt. Once we update to 7.2, we will not include mcrypt. Hat tip to @JamesBream who brought this to our attention in the first place.

[Singman33]

I dont want to hurry anyone but... Ubuntu 18.04.1 LTS is out now and default to PhP 7.2. Problems are incomming.

[JamesBream]

@Singman33 This isn’t an issue as by design, the snap doesn’t rely on pulling from default repos on installation. We bundle the php version we want to use into the snap meaning in our case, all users of the snap are running php 7.1 regardless of what php version is available in their default software repositories.

However, the fact that 7.2 is now the default in the latest LTS should hurry along this issue as you’re right, users installing NC manually on LTS will only have access to 7.2 so they’d run into issues with any apps which haven’t been updated - something we’re keen to avoid here.

I suspect it’ll probably be sensible to move to 7.2 as soon as the incoming NC 14 release lands.

[kyrofa]

[T]he fact that 7.2 is now the default in the latest LTS should hurry along this issue as you’re right, users installing NC manually on LTS will only have access to 7.2 so they’d run into issues with any apps which haven’t been updated - something we’re keen to avoid.

Man, you beat me to the punch! This exactly. I have nothing to add.

[Singman33]

My server was upgraded today to 18.04.1 but I dont use any apps depending on mbstring. To be safe (some kind of), I pushed NC to 14 beta 3 and it's working fine.

[pachulo]

OK, version 14.0 is here: https://nextcloud.com/blog/nextcloud-14-now-available-with-video-verification-signaltelegram-2fa-support-improved-collaboration-and-gdpr-compliance/
But user_saml is still not ready: https://github.com/nextcloud/user_saml/issues/168

[kyrofa]

How sad. So we continue marching along with 7.1, that's okay.

[pachulo]

Well, it seems that user_saml already released a version without mcrypt dependency: https://github.com/nextcloud/user_saml/issues/168#issuecomment-426353019
But that will come in Nextcloud 15 https://github.com/nextcloud/user_saml/issues/168#event-1923194826 if they don't backport to 14.

[pachulo]

Checking again I see that the user_saml version that enables (2.x.x) this is already out for nextcloud 14:
https://apps.nextcloud.com/apps/user_saml

[bn4t]

Does this mean we can upgrade to 7.2.x? @pachulo

[pachulo]

Once we jump into nextcloud 14 we should be able to @bn4t