Error in the upgrade script of Digiwords (fails CI tests)

Melchisedech · February 24, 2022, 1:39pm

Hi,
I’m packaging the Digiwords app for Yunohost. I made a first version that reached level 6 but as far as I understand, the current upgrade script just overwrites every file.

However, the official documentation says that upgrading the app takes indeed the form of overwriting every file but the inc/digiwords.db file which stores in progress word clouds.

So, I’m trying to improve the upgrade script to ensure this file is not modified during upgrade.

Maybe I’m wrong, but I thought that the best way to handle it would be to restore this file from backup at the end of the upgrade.

So, I added these lines in the scripts/upgrade file of my package :

#=================================================
# TAKE BACK THE OLD digiwords.db file
#=================================================
ynh_script_progression --message="Restoring database..." --time --weight=1
ynh_restore_file -o "$final_path/inc/digiwords.db"

Full source.

But the CI test upgrade fails. Here is the full log.

This part is particularly interesting:

44713 INFO [##############++....] > Restoring database... [00h00m,06s]
44852 WARNING /usr/share/yunohost/helpers.d/backup: line 257: YNH_BACKUP_DIR: unbound variable
45417 WARNING [Error] Upgrade failed.
100946 WARNING The app was restored to the way it was before the failed upgrade.
101951 ERROR Could not upgrade digiwords: An error occurred inside the app upgrade script
101957 INFO The operation 'Upgrade the 'digiwords' app' could not be completed. Please share the full log of this operation using the command 'yunohost log share 20220224-040902-app_upgrade-digiwords' to get help
102030 WARNING Here's an extract of the logs before the crash. It might help debugging the error:
102032 INFO DEBUG - + action=reload
102034 INFO DEBUG - + line_match=
102035 INFO DEBUG - + length=20
102035 INFO DEBUG - + log_path=/var/log/php7.3-fpm/php7.3-fpm.log
102036 INFO DEBUG - + timeout=300
102037 INFO DEBUG - + '[' reload == stop ']'
102037 INFO DEBUG - + '[' reload == reload ']'
102039 INFO DEBUG - + action=reload-or-restart
102040 INFO DEBUG - + systemctl reload-or-restart php7.3-fpm
102041 INFO INFO - [##############++....] > Restoring database... [00h00m,06s]
102042 INFO DEBUG - + ynh_restore_file -o /var/www/digiwords/inc/digiwords.db
102045 INFO DEBUG - + origin_path=/var/www/digiwords/inc/digiwords.db
102046 INFO DEBUG - + dest_path=/var/www/digiwords/inc/digiwords.db
102056 INFO DEBUG - + not_mandatory=0
102057 INFO DEBUG - + local archive_path=/var/cache/yunohost/app_tmp_work_dirs/app_h7clzp95/app_folder/scripts/var/www/digiwords/inc/digiwords.db
102057 INFO DEBUG - + '[' '!' -d /var/cache/yunohost/app_tmp_work_dirs/app_h7clzp95/app_folder/scripts/var/www/digiwords/inc/digiwords.db ']'
102057 INFO DEBUG - + '[' '!' -f /var/cache/yunohost/app_tmp_work_dirs/app_h7clzp95/app_folder/scripts/var/www/digiwords/inc/digiwords.db ']'
102057 INFO DEBUG - + '[' '!' -L /var/cache/yunohost/app_tmp_work_dirs/app_h7clzp95/app_folder/scripts/var/www/digiwords/inc/digiwords.db ']'
102057 INFO DEBUG - + '[' 0 == 0 ']'
102058 INFO DEBUG - ++ ynh_exit_properly
103345 ERROR The operation 'Upgrade the 'digiwords' app' could not be completed. Please share the full log of this operation using the command 'yunohost log share 20220224-040902-app_upgrade-digiwords' to get help

I really don’t know what if wrong with my script, even if this is a conception problem, if I just forgot to set ynh_backup_dir variable (but to what?) or a syntax one…
Thanks a lot for your help.

Tagada · February 24, 2022, 8:25pm

Hi,
I think the ynh_restore_file is for the restore script only, and I think you can’t really use it for the upgrade script.
What’s happening when someone don’t backup before the upgrade ?

Lots of package that need to re-use configuration, db, media… uses temporary dir.
See Ghost and Mastodon.

Melchisedech · February 24, 2022, 9:41pm

Thank you very much.

I think the ynh_restore_file is for the restore script only, and I think you can’t really use it for the upgrade script.

I think you’re right. This is what I guessed by investigating src/backup.py and helpers/backup. But I also think this is sub-optimal since the upgrade begins by a backup… then the files I need have already been copied or archived somewhere.

What’s happening when someone don’t backup before the upgrade ?

Her inc/digiwords.db is overwritten with the one included by default in the app and she loses all her word clouds.

Lots of package that need to re-use configuration, db, media… uses temporary dir.
See Ghost and Mastodon.

Woh. I didn’t know it was possible to link directly to a line of a file. This is great. These examples help me a lot. I removed the lines involving ynh_restore_file and wrote those instead:

#=================================================
# DOWNLOAD, CHECK AND UNPACK SOURCE
#=================================================

if [ "$upgrade_type" == "UPGRADE_APP" ]
then
	ynh_script_progression --message="Upgrading source files..." --time --weight=1
	
	# Create a temporary directory
	tmpdir="$(mktemp -d)"
	
	# Backup the inc/digiwords.db file to the temp dir
	cp -ar "$final_path/inc/digiwords.db" "$tmpdir/digiwords.db"
	
	# Remove the app directory securely
	ynh_secure_remove --file=$final_path

	# Download, check integrity, uncompress and patch the source from app.src
	ynh_setup_source --dest_dir="$final_path"
	
	# Copy digiwords.db back to the final_path
	cp -ar "$tmpdir/digiwords.db" "$final_path/inc/digiwords.db"

	# Remove the tmp directory securely
	ynh_secure_remove --file="$tmpdir"
fi

I think it will make it. I’ll tell you. Thanks again.

Melchisedech · February 26, 2022, 10:44am

Thanks a lot, it works!
However, @yalh76 talked to me about the --keep= argument from ynh_setup_source and I think I’ll use it in future versions of my script. Maybe I’m wrong but I’m inclined to think that using a built-in function is very often more efficient.

Tagada · February 26, 2022, 11:36pm

One behavior that is not clear to me about ynh_setup_source during upgrades. If upstream deletes a file, or rename/move a file, my understanding is that ynh_setup_source will not remove the old files, and this can cause bugs/crashes in some apps. To prevent this, I use ynh_secure_remove before ynh_setup_source, therefore I can’t use --keep in that case (because all files are removed before I do ynh_setup_source) and I have to backup them without the helper.

Melchisedech · February 27, 2022, 4:10pm

Well, I’ll keep that in mind. In the doubt, I think I’ll stick to the solution you proposed and investigate later to see how ynh_setup_source behave to see if it is safe to use the --keep parameter.