search

YunoHost / issues

General issue tracker for the YunoHost project
72 stars 8 forks source link

Fix php madness !!! #1503

Closed maniackcrudelis closed 4 years ago

maniackcrudelis commented 4 years ago

Again, the sury shit storm is back !

Now with php7.4, which is strange since no apps use that version. The error happened recently with nextcloud

https://github.com/YunoHost-Apps/nextcloud_ynh/issues/250 https://forum.yunohost.org/t/php7-4-a-casse-nextcloud/9955 https://forum.yunohost.org/t/nextcloud-mise-a-jour-impossible/9956/7

Smells like it's only a beginning... php7.4 seems to be new, late November on https://packages.sury.org/php/pool/main/p/php7.4/ If I do understand that PR, https://github.com/YunoHost/yunohost/pull/809, regarding the mess in https://github.com/YunoHost/issues/issues/1407, I believe adding back sury repo without any security may have raised that new issue.

Tests have to be made, and probably working more on the experimental helper and made it official as soon as possible.

I think (but not checked yet) that pinning at least that sury repo would help.

I will hardly have time during January, but I will try to my best to go forward on that issue. Everyone willing to help is highly welcome !

maniackcrudelis commented 4 years ago

Did some tests on a fresh VM.

Step by step:

So far, I do reproduce the unwanted installation of php7.4 But my current version is still 7.0

PS: Without pixelfed installed first, nextcloud does not have any error with php-zip. Probably using php-zip would get apt to use sury repo and end up with a php7.4 version of php-zip... EDIT: It appears that php-zip is simply not installed...

maniackcrudelis commented 4 years ago

Not considering that issue with php-zip, installing manually the dependencies of Nextcloud end up with only php7.4 version of all packages needed. Even though my default version is still php7.0, it applies only to /usr/bin/php which is only php-cli.

I'm quite sure nextcloud would refuse to install, considering all its dependencies are from php7.4 now.

maniackcrudelis commented 4 years ago

... New test today

I expect that now that we have php7.4, nextcloud won't upgrade. But things went fine... I can see that nextcloud is still using php7.0.33...

What bothers me is that anyway, nginx use a php7.0 socket. So a socket handled by a php7.0 process.

Well, bad news if I can't reproduce that error...

maniackcrudelis commented 4 years ago

Actually found out why php-zip was missing, not easy though...

When using dpkg on the deb file for nextcloud (equivs with the fake dependency do that), nothing is said about php-zip missing as dependency, but it is missing ! apt-cache policy php-zip says "Installed: (none)" But actually, if you did check apt-cache policy php7.3-zip, you get "Installed: 7.3.13-1+0\~20191218.50+debian9~1.gbp23c2da"

Indeed, when installing php7.3, we're not asking for php-zip, but php7.3-zip, so we don't have the common package with it. There's currently no way to fix that properly, php-zip is an alias, there is a package corresponding to that one.

But, with sury currently added, apt install php-zip will end up with php7.4-zip not php7.0-zip Without, php-zip can't be installed...

But, with a correct pin on sury though, apt install php-zip will install php7.0-zip and apt install php7.3-zip the correct one as well.

maniackcrudelis commented 4 years ago

About the issue on its own regarding Nextcloud and php7.4

It appears, https://forum.yunohost.org/t/nextcloud-mise-a-jour-impossible/9956/10, that php-cli was indeed set 7.4 I know that some old apps were doing that, none are doing it anymore, but without an new install from the helper, php7.0 won't be set as default.

If we probably know how to fix the current error with nextcloud, we shouldn't let sury installed whatever version of php it wants. This simple pin configuration is enough to jail sury and prevent any further shit storm.

Package: *
Pin: origin "packages.sury.org"
Pin-Priority: 200

Package: php7.0*
Pin: origin "packages.sury.org"
Pin-Priority: 600

An other thing (that I'm about to try) is, should we specify php7.0 instead of php in all apps ? Is an app will work with php7.0-zip instead of php-zip ? Surely we'll have to specify as well to use php7.0 instead of php when using php-cli. Of course we may think it would be better to loose php7.0 which is not supported anymore, but all nginx configurations are using php7.0 as well. So anyway...

maniackcrudelis commented 4 years ago

So, finally, specifiyng php7.0 for every package in nextcloud (as well as php-cli when used) is very useful. First it does work perfectly without any trouble. Second, even with pixelfed installed first, I don't any issue with php-zip. as we ask for php7.0-zip specifically.

Well now... What should we do ?

That's some questions I would really like to not answer only on my own -_- @YunoHost/apps @Psycojoker @alexAubin @WhoElseAsSomethingToSay...

Josue-T commented 4 years ago

Look like good for me.

Ask to packagers to modify their dependencies to specify php7.0 instead of php ? We should remember that we did that on purpose because of php5 and 7.

As I see for me it's a good idea to specify evewhere the php version. I'm thinking also about buster support (I know that it's not directly related). The question is that when we will be on buster we will replace (as we know actually) php5 by php7.3. And does we also need to replace php7.0 by php7.3 ?

Maybe we can think about something like an environment variable or something else which give the exact php version officially provided by debian. By example on debian stretch we will have something like YNH_PHP_VERSION=7.0 and on debian buster we will have YNH_PHP_VERSION=7.3.

maniackcrudelis commented 4 years ago

I don't think the migration will be as painful from stretch to buster as it was from jessie. With stretch debian has added a better handle of multiple php version. So I really think running php7.0 on buster won't be a real pain.

Anyway, moving to an upper version is clearly a good idea, as soon as we're on buster.

However, even if having a variable to set globally the default php version looks like a good idea, that means also the second solution

Create a new Epic ugly hack to do that automatically ?

Indeed, that would mean modifying all dependencies to add the php version. Or, we would consider having something like 

php_version=${YNH_PHP_VERSION:-7.0}

and then rely to that variable for each dependency and php command.

maniackcrudelis commented 4 years ago

Started to work on that https://github.com/YunoHost/yunohost/pull/879 I'll modify right away the example app to ease the migration. My idea is to not have to change the php version anymore and let the helper do the job.

maniackcrudelis commented 4 years ago

https://github.com/YunoHost/example_ynh/pull/110