YunoHost-Apps / garradin_ynh

Logiciel libre de gestion associative pour YunoHost
https://garradin.eu/
GNU General Public License v3.0
7 stars 8 forks source link

Important: Upgrade and restoration fails #61

Closed fflorent closed 2 years ago

fflorent commented 3 years ago

Describe the bug

If I upgrade Garradin to version 1.1, the upgrade fails and so does the restoration.

Context

Steps to reproduce

  1. Install Garradin 1.0.7~ynh2 (sudo yunohost app install -f https://github.com/YunoHost-Apps/garradin_ynh/tree/3b0d2ada57933bf325612446cbe338eb8cbcf115 and validate the default values for the questions of the manifest)
  2. Upgrade Garradin without upgrading php packages with apt (sudo yunohost app fetchlist; sudo yunohost app upgrade garradin)

Expected behavior

  1. The upgrade should work
  2. If not, the restoration should work

Logs

https://paste.yunohost.org/raw/igebetahij

Workaround

Connect to the server and run:

$ sudo apt remove garradin-ynh-deps
$ sudo yunohost backup restore garradin-pre-upgrade1 --apps
$ sudo apt update && sudo apt full-upgrade
$ sudo yunohost app upgrade garradin
rodinux commented 3 years ago

I see on the log problems of dependencies with php7.4 modules also used on nextcloud

2021-09-28 13:48:49,251: DEBUG - The following packages will be REMOVED:
2021-09-28 13:48:49,252: DEBUG -   nextcloud-ynh-deps

perhaps with php-imagick also...

fflorent commented 3 years ago

Yep, there are nextcloud dependencies issues… But I also tested it in another VM without it and same result: https://paste.yunohost.org/raw/runuhukeba

Here there are issues with Dolibarr.

rodinux commented 3 years ago

Huu ! I need help to understand what's going wrong. It is strange because I have updated before an instance garradin in a server with Nextcloud without these issue, but it was a time ago with a less version of Yunohost... I think I can precise on the manifest.json the service is php7.4-fpm for this app. It is strange, because for example in this server I have php7.4-imagick and php7.3-imagick installed:

$ apt-cache policy php7.4-imagick
php7.4-imagick:
  Installed: 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
  Candidate: 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
  Version table:
 *** 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2 500
        500 https://packages.sury.org/php buster/main amd64 Packages
        100 /var/lib/dpkg/status
~$ apt-cache policy php7.3-imagick
php7.3-imagick:
  Installed: 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
  Candidate: 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
  Version table:
 *** 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2 500
        500 https://packages.sury.org/php buster/main amd64 Packages
        100 /var/lib/dpkg/status

here's the file pinning the extra_php_version

$ cat /etc/apt/preferences.d/extra_php_version 

Package: php
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-fpm
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-mysql
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-xml
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-zip
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-mbstring
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-ldap
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-gd
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-curl
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-bz2
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-json
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-sqlite3
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: php-intl
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: openssl
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: libssl1.1
Pin: origin "packages.sury.org" 
Pin-Priority: -1

Package: libssl-dev
Pin: origin "packages.sury.org" 
Pin-Priority: -1
rodinux commented 3 years ago

can you try this please: sudo yunohost upgrade garradin -u https://github.com//YunoHost-Apps/garradin_ynh/tree/1.1

I have add some lines you can see here: https://github.com/YunoHost-Apps/garradin_ynh/commit/cbeec8ec1ea5d56b61435b74e997b20c222cf6cb#diff-8c5967fd8486f34493710cc39b240aad46536cf4ee421ffd0479e6542db03e36

fflorent commented 3 years ago

Not more luck, unfortunately: https://paste.yunohost.org/raw/icoxuwetuj

alexAubin commented 3 years ago

I've seen this kind of issue with php-imagick a few days ago (NB we're talking about php-imagick, not php7.4-imagick)

Can you share the output of apt policy php-imagick

fflorent commented 3 years ago

Indeed:

php-imagick:
  Installé : 3.4.3-4.1
  Candidat : 3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
 Table de version :
     3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2 500
        500 https://packages.sury.org/php buster/main amd64 Packages
 *** 3.4.3-4.1 500
        500 http://ftp.debian.org/debian buster/main amd64 Packages
        100 /var/lib/dpkg/status
alexAubin commented 3 years ago

Then let's try to : apt install php-imagick=3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2

fflorent commented 3 years ago

I managed to update the dependency through the UI (otherwise apt would have removed dolibarr-ynh-deps).

And it worked!

Do we have to assume the user updates first the system dependencies (deb packages) before the yunohost application packages?

alexAubin commented 3 years ago

I managed to update the dependency through the UI (otherwise apt would have removed dolibarr-ynh-deps).

Uh can you elaborate on the "otherwise apt would have removed dolibarr-ynh-deps", and are you sure that the upgrade through the UI did not remove those ...?

fflorent commented 3 years ago

Uh can you elaborate on the "otherwise apt would have removed dolibarr-ynh-deps"?

Yes. I got this:

sudo LANGUAGE=en apt install php-imagick=3.5.1-1+0~20210825.29+debian10~1.gbp4a47e2
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following additional packages will be installed:
  php8.0-common php8.0-imagick
The following packages will be REMOVED:
  dolibarr-ynh-deps
The following NEW packages will be installed:
  php8.0-common php8.0-imagick
The following packages will be upgraded:
  php-imagick
1 upgraded, 2 newly installed, 1 to remove and 14 not upgraded.
Need to get 773 kB of archives.
After this operation, 8 262 kB of additional disk space will be used.
Do you want to continue? [O/n] n
Abort.

(I aborted, of course)

and are you sure that the upgrade through the UI did not remove those ...?

It didn't remove dolibarr-ynh-deps, as you can see (this command was run after the upgrade of the php deb packages):

:~$ sudo LANGUAGE=en apt policy dolibarr-ynh-deps
dolibarr-ynh-deps:
  Installed: 13.0.4~ynh1
  Candidate: 13.0.4~ynh1
  Version table:
 *** 13.0.4~ynh1 100
        100 /var/lib/dpkg/status
alexAubin commented 3 years ago

Uuuuuh okay not sure to understand why it wanted to remove dolibarr but didn't in the end ... but apt/dpkg dependency conflicts/edgecases take at least twelve PhD to understand so x_x

rodinux commented 3 years ago

Hi @fflorent, I have merge your patch to resolve the issue, but I am afraid with the line php7.4-fpm on the manifest.json here: https://github.com/YunoHost-Apps/garradin_ynh/blob/e8c8b2d5ef1fdc587b5b4dd5d11c797f918e5a17/manifest.json#L29 what happens if there's not php7.4 on the system before the install ? However the CLI seems happy, so perhaps is not a problem...

alexAubin commented 3 years ago

what happens if there's not php7.4 on the system before the install ?

Rereading the code (this is the only place that info is actually used iirc) : having php7.4-fpm listed or not actually has no effect ... this key is only used to check if nginx / php7.3-fpm / mysql / postfix (depending on what the list contains) are up before running the app install and upgrade. But it doesn't support other services (including php-fpm versions different from the default one).

This services key is mainly legacy, it was never super clear what was the intention behind it or what to do with it (both from the packager perspective and the core development).

fflorent commented 3 years ago

I see… Sorry for the mistake then :/

rodinux commented 3 years ago

I see… Sorry for the mistake then :/

So, I am not sure understand all, but is it better keep the line on the manifest.json with "php7.3-fpm"??

fflorent commented 3 years ago

@rodinux I asked @alexAubin for more information for your question, it answered me this: image