[fix] Restructure app restauration and catch app script failure

* Do not copy again the restore app script
 * Set app folder permissions and properly remove it on failure
 * Add a raise_on_error argument to hook_exec
 * Review displayed messages

data/actionsmap/yunohost.yml

@@ -1346,3 +1346,6 @@ hook:
                 -a:
                     full: --args
                     help: Arguments to pass to the script
+                --raise-on-error:
+                    help: Raise if the script returns a non-zero exit code
+                    action: store_true

lib/yunohost/backup.py

@@ -162,31 +162,21 @@ def backup_create(name=None, description=None, output_directory=None,
         for app_id in apps_filtered:
             app_setting_path = '/etc/yunohost/apps/' + app_id
 
-            tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id)
+            # Check if the app has a backup and restore script
-            
-            # Check if the app has a backup script
             app_script = app_setting_path + '/scripts/backup'
+            app_restore_script = app_setting_path + '/scripts/restore'
             if not os.path.isfile(app_script):
                 logger.warning("backup script '%s' not found", app_script)
-                msignals.display(m18n.n('unbackup_app', app_id),
+                msignals.display(m18n.n('unbackup_app', app=app_id),
                                  'warning')
                 continue
-                
+            elif not os.path.isfile(app_restore_script):
-            # Copy the app restore script
+                logger.warning("restore script '%s' not found",
-            app_restore_script = app_setting_path + '/scripts/restore'
+                               app_restore_script)
-            if os.path.isfile(app_script):
+                msignals.display(m18n.n('unrestore_app', app=app_id),
-                try:
-                    filesystem.mkdir(tmp_app_dir, 0750, True, uid='admin')
-                    shutil.copy(app_restore_script, tmp_app_dir)
-                except:
-                    logger.exception("error while copying restore script of '%s'", app_id)
-                    msignals.display(m18n.n('restore_app_copy_failed', app=app_id),
-                                 'warning')
-            else:
-                logger.warning("restore script '%s' not found", app_script)
-                msignals.display(m18n.n('unrestorable_app', app_id),
                                  'warning')
 
+            tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id)
             tmp_app_bkp_dir = tmp_app_dir + '/backup'
             msignals.display(m18n.n('backup_running_app_script', app_id))
             try:
@@ -196,7 +186,8 @@ def backup_create(name=None, description=None, output_directory=None,
 
                 # Copy app backup script in a temporary folder and execute it
                 subprocess.call(['install', '-Dm555', app_script, tmp_script])
-                hook_exec(tmp_script, args=[tmp_app_bkp_dir, app_id])
+                hook_exec(tmp_script, args=[tmp_app_bkp_dir, app_id],
+                          raise_on_error=True)
             except:
                 logger.exception("error while executing backup of '%s'", app_id)
                 msignals.display(m18n.n('backup_app_failed', app=app_id),
@@ -351,7 +342,7 @@ def backup_restore(name, hooks=[], apps=[], ignore_apps=False, ignore_hooks=Fals
     if not ignore_hooks:
         if hooks is None or len(hooks)==0:
             hooks=info['hooks'].keys()
-        
+
         hooks_filtered=list(set(hooks) & set(info['hooks'].keys()))
         hooks_unexecuted=set(hooks) - set(info['hooks'].keys())
         for hook in hooks_unexecuted:
@@ -359,49 +350,67 @@ def backup_restore(name, hooks=[], apps=[], ignore_apps=False, ignore_hooks=Fals
             msignals.display(m18n.n('backup_hook_unavailable', hook), 'warning')
         msignals.display(m18n.n('restore_running_hooks'))
         hook_callback('restore', hooks_filtered, args=[tmp_dir])
-    
+
     # Add apps restore hook
     if not ignore_apps:
+        from yunohost.app import _is_installed
+
         # Filter applications to restore
         apps_list = set(info['apps'].keys())
         apps_filtered = set()
-        if not apps:
+        if apps:
-            apps=apps_list
+            for a in apps:
-            
+                if a not in apps_list:
-        from yunohost.app import _is_installed
+                    logger.warning("app '%s' not found in the backup '%s'",
-        for app_id in apps:
+                                   a, archive_file)
-            if app_id not in apps_list:
+                    msignals.display(m18n.n('backup_archive_app_not_found',
-                logger.warning("app '%s' not found", app_id)
+                                            app=a),
-                msignals.display(m18n.n('unrestore_app', app_id), 'warning')
+                                     'error')
-            elif _is_installed(app_id):
+                else:
+                    apps_filtered.add(a)
+        else:
+            apps_filtered = apps_list
+
+        for app_id in apps_filtered:
+            tmp_app_dir = '{:s}/apps/{:s}'.format(tmp_dir, app_id)
+
+            # Check if the app is not already installed
+            if _is_installed(app_id):
                 logger.warning("app '%s' already installed", app_id)
-                msignals.display(m18n.n('restore_already_installed_app', app=app_id), 'warning')  
+                msignals.display(m18n.n('restore_already_installed_app',
-            elif not os.path.isfile('{:s}/apps/{:s}/restore'.format(tmp_dir, app_id)):
+                                        app=app_id),
-                logger.warning("backup for '%s' doesn't contain a restore script", app_id)
+                                'error')
-                msignals.display(m18n.n('no_restore_script', app=app_id), 'warning')                 
+                continue
-            else:
-                apps_filtered.add(app_id)
 
-        for app_id in apps_filtered:  
+            # Check if the app has a restore script
-            app_bkp_dir='{:s}/apps/{:s}'.format(tmp_dir, app_id)
+            app_script = tmp_app_dir + '/settings/scripts/restore'
+            if not os.path.isfile(app_script):
+                logger.warning("restore script for the app '%s' not found " \
+                               "in the backup '%s'", app_id, archive_file)
+                msignals.display(m18n.n('unrestore_app', app=app_id), 'warning')
+                continue
+
+            tmp_script = '/tmp/restore_' + app_id
+            app_setting_path = '/etc/yunohost/apps/' + app_id
+            msignals.display(m18n.n('restore_running_app_script', app=app_id))
             try:
-                # Copy app settings
+                # Copy app settings and set permissions
-                app_setting_path = '/etc/yunohost/apps/' + app_id
+                shutil.copytree(tmp_app_dir + '/settings', app_setting_path)
-                shutil.copytree(app_bkp_dir + '/settings', app_setting_path )
+                filesystem.chmod(app_setting_path, 0555, 0444, True)
-                
+                filesystem.chmod(app_setting_path + '/settings.yml', 0400)
-                # Execute app restore script
-                app_restore_script=app_bkp_dir+'/restore'
-                tmp_script = '/tmp/restore_%s_%s' % (name,app_id)
-                subprocess.call(['install', '-Dm555', app_restore_script, tmp_script])
-                hook_exec(tmp_script, args=[app_bkp_dir+'/backup', app_id]) 
 
+                # Execute app restore script
+                subprocess.call(['install', '-Dm555', app_script, tmp_script])
+                hook_exec(tmp_script, args=[tmp_app_dir + '/backup', app_id],
+                          raise_on_error=True)
             except:
                 logger.exception("error while restoring backup of '%s'", app_id)
                 msignals.display(m18n.n('restore_app_failed', app=app_id),
                                  'error')
-                # Cleaning settings directory
+                # Cleaning app directory
-                shutil.rmtree(app_setting_path + '/settings', ignore_errors=True)
+                shutil.rmtree(app_setting_path, ignore_errors=True)
-
+            finally:
+                filesystem.rm(tmp_script, force=True)
 
     # Remove temporary directory
     os.system('rm -rf %s' % tmp_dir)

lib/yunohost/hook.py

@@ -260,13 +260,14 @@ def hook_check(file):
         return {}
 
 
-def hook_exec(file, args=None):
+def hook_exec(file, args=None, raise_on_error=False):
     """
     Execute hook from a file with arguments
 
     Keyword argument:
         file -- Script to execute
         args -- Arguments to pass to the script
+        raise_on_error -- Raise if the script returns a non-zero exit code
 
     """
     from moulinette.utils.stream import NonBlockingStreamReader
@@ -344,6 +345,8 @@ def hook_exec(file, args=None):
             msignals.display(line.rstrip(), 'log')
     stream.close()
 
+    if raise_on_error and returncode != 0:
+        raise MoulinetteError(m18n.n('hook_exec_failed'))
     return returncode
 
 

locales/en.json

@@ -86,6 +86,7 @@
     "hook_name_unknown" : "Unknown hook name '{:s}'",
     "hook_choice_invalid" : "Invalid choice '{:s}'",
     "hook_argument_missing" : "Missing argument '{:s}'",
+    "hook_exec_failed" : "Script execution failed",
 
     "mountpoint_unknown" : "Unknown mountpoint",
     "unit_unknown" : "Unknown unit '{:s}'",
@@ -150,6 +151,7 @@
     "backup_archive_open_failed" : "Unable to open the backup archive",
     "backup_archive_name_unknown" : "Unknown local backup archive named '{:s}'",
     "backup_archive_name_exists" : "Backup archive name already exists",
+    "backup_archive_app_not_found" : "App '{app:s}' not found in the backup archive",
     "backup_app_failed" : "Unable to back up the app '{app:s}'",
     "backup_nothings_done" : "There is nothing to save",
     "backup_cleaning_failed" : "Unable to clean backup directory",
@@ -159,14 +161,13 @@
     "restore_confirm_yunohost_installed" : "Do you really want to restore an already installed system? [{answers:s}]",
     "restore_app_failed" : "Unable to restore the app '{app:s}'",
     "restore_running_hooks" : "Running restoration hooks...",
+    "restore_running_app_script" : "Running restore script of app '{app:s}'...",
     "restore_failed" : "Unable to restore the system",
     "restore_complete" : "Restore complete",
     "restore_already_installed_app": "An app is already installed with the id '{app:s}'",
-    "unbackup_app" : "App '{:s}' will not be saved",
+    "unbackup_app" : "App '{app:s}' will not be saved",
-    "unrestorable_app" : "App '{:s}' will not be restored",
-    "restore_app_copy_failed" : "Unable to copy the restore script of app '{app:s}'",
     "no_restore_script": "No restore script found for the app '{app:s}'",
-    "unrestore_app" : "App '{:s}' will not be restored",
+    "unrestore_app" : "App '{app:s}' will not be restored",
     "backup_delete_error" : "Unable to delete '{:s}'",
     "backup_deleted" : "Backup successfully deleted",