apps: fix inconsistent app removal during remove-after-failed-upgrade and remove-after-failed-backup contexts

src/app.py

@@ -743,7 +743,7 @@ def app_upgrade(app=[], url=None, file=None, force=False, no_safety_backup=False
                     "Upgrade failed ... attempting to restore the satefy backup (Yunohost first need to remove the app for this) ..."
                 )
 
-                app_remove(app_instance_name)
+                app_remove(app_instance_name, force_workdir=extracted_app_folder)
                 backup_restore(
                     name=safety_backup_name, apps=[app_instance_name], force=True
                 )
@@ -1270,14 +1270,14 @@ def app_install(
 
 
 @is_unit_operation()
-def app_remove(operation_logger, app, purge=False):
+def app_remove(operation_logger, app, purge=False, force_workdir=None):
     """
     Remove app
 
     Keyword arguments:
         app -- App(s) to delete
         purge -- Remove with all app data
-
+        force_workdir -- Special var to force the working directoy to use, in context such as remove-after-failed-upgrade or remove-after-failed-restore
     """
     from yunohost.utils.legacy import _patch_legacy_php_versions, _patch_legacy_helpers
     from yunohost.hook import hook_exec, hook_remove, hook_callback
@@ -1296,7 +1296,6 @@ def app_remove(operation_logger, app, purge=False):
     operation_logger.start()
 
     logger.info(m18n.n("app_start_remove", app=app))
-
     app_setting_path = os.path.join(APPS_SETTING_PATH, app)
 
     # Attempt to patch legacy helpers ...
@@ -1306,8 +1305,20 @@ def app_remove(operation_logger, app, purge=False):
     # script might date back from jessie install)
     _patch_legacy_php_versions(app_setting_path)
 
-    manifest = _get_manifest_of_app(app_setting_path)
+    if force_workdir:
-    tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app)
+        # This is when e.g. calling app_remove() from the upgrade-failed case
+        # where we want to remove using the *new* remove script and not the old one
+        # and also get the new manifest
+        # It's especially important during v1->v2 app format transition where the
+        # setting names change (e.g. install_dir instead of final_path) and
+        # running the old remove script doesnt make sense anymore ...
+        tmp_workdir_for_app = tempfile.mkdtemp(prefix="app_", dir=APP_TMP_WORKDIRS)
+        os.system(f"cp -a {force_workdir}/* {tmp_workdir_for_app}/")
+    else:
+        tmp_workdir_for_app = _make_tmp_workdir_for_app(app=app)
+
+    manifest = _get_manifest_of_app(tmp_workdir_for_app)
+
     remove_script = f"{tmp_workdir_for_app}/scripts/remove"
 
     env_dict = {}

src/backup.py

@@ -52,6 +52,7 @@ from yunohost.app import (
     _make_environment_for_app_script,
     _make_tmp_workdir_for_app,
     _get_manifest_of_app,
+    app_remove,
 )
 from yunohost.hook import (
     hook_list,
@@ -1550,36 +1551,7 @@ class RestoreManager:
             else:
                 self.targets.set_result("apps", app_instance_name, "Error")
 
-                remove_script = os.path.join(app_scripts_in_archive, "remove")
+                app_remove(app_instance_name, force_workdir=app_workdir)
-
-                # Setup environment for remove script
-                env_dict_remove = _make_environment_for_app_script(
-                    app_instance_name, workdir=app_workdir
-                )
-                remove_operation_logger = OperationLogger(
-                    "remove_on_failed_restore",
-                    [("app", app_instance_name)],
-                    env=env_dict_remove,
-                )
-                remove_operation_logger.start()
-
-                # Execute remove script
-                if hook_exec(remove_script, env=env_dict_remove)[0] != 0:
-                    msg = m18n.n("app_not_properly_removed", app=app_instance_name)
-                    logger.warning(msg)
-                    remove_operation_logger.error(msg)
-                else:
-                    remove_operation_logger.success()
-
-                # Cleaning app directory
-                shutil.rmtree(app_settings_new_path, ignore_errors=True)
-
-                # Remove all permission in LDAP for this app
-                for permission_name in user_permission_list()["permissions"].keys():
-                    if permission_name.startswith(app_instance_name + "."):
-                        permission_delete(permission_name, force=True)
-
-                # TODO Cleaning app hooks
 
                 logger.error(failure_message_with_debug_instructions)