From cb247a8140d9ee8a71c663a2da9b3d106ef969bf Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 4 Nov 2019 21:53:21 +0100 Subject: [PATCH] Get rid of app's status.json --- locales/en.json | 1 + src/yunohost/app.py | 95 ++----------------- .../0013_remove_app_status_json.py | 31 ++++++ src/yunohost/tools.py | 5 +- 4 files changed, 43 insertions(+), 89 deletions(-) create mode 100644 src/yunohost/data_migrations/0013_remove_app_status_json.py diff --git a/locales/en.json b/locales/en.json index 6c7d0b42d..9b322b322 100644 --- a/locales/en.json +++ b/locales/en.json @@ -320,6 +320,7 @@ "migration_description_0010_migrate_to_apps_json": "Remove deprecated applists and use the new unified 'apps.json' list instead", "migration_description_0011_setup_group_permission": "Set up user group and set up permission for apps and services", "migration_description_0012_postgresql_password_to_md5_authentication": "Force PostgreSQL authentication to use MD5 for local connections", + "migration_description_0013_remove_app_status_json": "Remove legacy status.json app files", "migration_0003_start": "Starting migration to Stretch. The logs will be available in {logfile}.", "migration_0003_patching_sources_list": "Patching the sources.lists…", "migration_0003_main_upgrade": "Starting main upgrade…", diff --git a/src/yunohost/app.py b/src/yunohost/app.py index f92adf9e4..29c55d276 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -234,8 +234,6 @@ def app_list(filter=None, raw=False, installed=False, with_backup=False): Keyword argument: filter -- Name filter of app_id or app_name - offset -- Starting number for app fetching - limit -- Maximum number of app fetched raw -- Return the full app_dict installed -- Return only installed apps with_backup -- Return only apps with backup feature (force --installed filter) @@ -310,8 +308,6 @@ def app_list(filter=None, raw=False, installed=False, with_backup=False): if raw: app_info_dict['installed'] = app_installed - if app_installed: - app_info_dict['status'] = _get_app_status(app_id) # dirty: we used to have manifest containing multi_instance value in form of a string # but we've switched to bool, this line ensure retrocompatibility @@ -338,13 +334,12 @@ def app_list(filter=None, raw=False, installed=False, with_backup=False): return {'apps': list_dict} if not raw else list_dict -def app_info(app, show_status=False, raw=False): +def app_info(app, raw=False): """ Get app info Keyword argument: app -- Specific app ID - show_status -- Show app installation status raw -- Return the full app_dict """ @@ -353,6 +348,9 @@ def app_info(app, show_status=False, raw=False): app_setting_path = APPS_SETTING_PATH + app + # Retrieve manifest and status + manifest = _get_manifest_of_app(app_setting_path) + if raw: ret = app_list(filter=app, raw=True)[app] ret['settings'] = _get_app_settings(app) @@ -370,28 +368,16 @@ def app_info(app, show_status=False, raw=False): ret['upgradable'] = upgradable ret['change_url'] = os.path.exists(os.path.join(app_setting_path, "scripts", "change_url")) - - manifest = _get_manifest_of_app(os.path.join(APPS_SETTING_PATH, app)) - ret['version'] = manifest.get('version', '-') return ret - # Retrieve manifest and status - manifest = _get_manifest_of_app(app_setting_path) - status = _get_app_status(app, format_date=True) - info = { 'name': manifest['name'], 'description': _value_for_locale(manifest['description']), - # FIXME: Temporarly allow undefined license 'license': manifest.get('license', m18n.n('license_undefined')), - # FIXME: Temporarly allow undefined version 'version': manifest.get('version', '-'), - # TODO: Add more info } - if show_status: - info['status'] = status return info @@ -699,10 +685,6 @@ def app_upgrade(app=[], url=None, file=None): app_setting_path = APPS_SETTING_PATH + '/' + app_instance_name - # Retrieve current app status - status = _get_app_status(app_instance_name) - status['remote'] = manifest.get('remote', None) - # Retrieve arguments list for upgrade script # TODO: Allow to specify arguments args_odict = _parse_args_from_manifest(manifest, 'upgrade') @@ -776,9 +758,8 @@ def app_upgrade(app=[], url=None, file=None): # Otherwise we're good and keep going ! now = int(time.time()) - # TODO: Move install_time away from app_setting app_setting(app_instance_name, 'update_time', now) - status['upgraded_at'] = now + app_setting(app_instance_name, 'current_revision', manifest.get('remote', {}).get('revision', "?")) # Clean hooks and add new ones hook_remove(app_instance_name) @@ -786,10 +767,6 @@ def app_upgrade(app=[], url=None, file=None): for hook in os.listdir(extracted_app_folder + '/hooks'): hook_add(app_instance_name, extracted_app_folder + '/hooks/' + hook) - # Store app status - with open(app_setting_path + '/status.json', 'w+') as f: - json.dump(status, f) - # Replace scripts and manifest and conf (if exists) os.system('rm -rf "%s/scripts" "%s/manifest.toml %s/manifest.json %s/conf"' % (app_setting_path, app_setting_path, app_setting_path, app_setting_path)) @@ -834,14 +811,6 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu if not os.path.exists(INSTALL_TMP): os.makedirs(INSTALL_TMP) - status = { - 'installed_at': int(time.time()), - 'upgraded_at': None, - 'remote': { - 'type': None, - }, - } - def confirm_install(confirm): # Ignore if there's nothing for confirm (good quality app), if --force is used # or if request on the API (confirm already implemented on the API side) @@ -897,7 +866,6 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu manifest, extracted_app_folder = _extract_app_from_file(app) else: raise YunohostError('app_unknown') - status['remote'] = manifest.get('remote', {}) # Check ID if 'id' not in manifest or '__' in manifest['id']: @@ -962,9 +930,9 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu app_settings = { 'id': app_instance_name, 'label': label if label else manifest['name'], + 'install_time': int(time.time()), + 'current_revision': manifest.get('remote', {}).get('revision', "?") } - # TODO: Move install_time away from app settings - app_settings['install_time'] = status['installed_at'] _set_app_settings(app_instance_name, app_settings) # Apply dirty patch to make php5 apps compatible with php7 @@ -1092,10 +1060,6 @@ def app_install(operation_logger, app, label=None, args=None, no_remove_on_failu for file in os.listdir(extracted_app_folder + '/hooks'): hook_add(app_instance_name, extracted_app_folder + '/hooks/' + file) - # Store app status - with open(app_setting_path + '/status.json', 'w+') as f: - json.dump(status, f) - # Clean and set permissions shutil.rmtree(extracted_app_folder) os.system('chmod -R 400 %s' % app_setting_path) @@ -2180,51 +2144,6 @@ def _set_app_settings(app_id, settings): yaml.safe_dump(settings, f, default_flow_style=False) -def _get_app_status(app_id, format_date=False): - """ - Get app status or create it if needed - - Keyword arguments: - app_id -- The app id - format_date -- Format date fields - - """ - app_setting_path = APPS_SETTING_PATH + app_id - if not os.path.isdir(app_setting_path): - raise YunohostError('app_unknown') - status = {} - - regen_status = True - try: - with open(app_setting_path + '/status.json') as f: - status = json.loads(str(f.read())) - regen_status = False - except IOError: - logger.debug("status file not found for '%s'", app_id, - exc_info=1) - except Exception as e: - logger.warning("could not open or decode %s : %s ... regenerating.", app_setting_path + '/status.json', str(e)) - - if regen_status: - # Create app status - status = { - 'installed_at': app_setting(app_id, 'install_time'), - 'upgraded_at': app_setting(app_id, 'update_time'), - 'remote': {'type': None}, - } - with open(app_setting_path + '/status.json', 'w+') as f: - json.dump(status, f) - - if format_date: - for f in ['installed_at', 'upgraded_at']: - v = status.get(f, None) - if not v: - status[f] = '-' - else: - status[f] = datetime.utcfromtimestamp(v) - return status - - def _extract_app_from_file(path, remove=False): """ Unzip or untar application tarball in APP_TMP_FOLDER, or copy it from a directory diff --git a/src/yunohost/data_migrations/0013_remove_app_status_json.py b/src/yunohost/data_migrations/0013_remove_app_status_json.py new file mode 100644 index 000000000..1cb5bc002 --- /dev/null +++ b/src/yunohost/data_migrations/0013_remove_app_status_json.py @@ -0,0 +1,31 @@ +import os + +from moulinette.utils.log import getActionLogger +from moulinette.utils.filesystem import read_json + +from yunohost.tools import Migration +from yunohost.app import app_setting, APPS_SETTING_PATH + +logger = getActionLogger('yunohost.migration') + +class MyMigration(Migration): + + """Remove legacy app status.json files""" + + def run(self): + + apps = os.listdir(APPS_SETTING_PATH) + + for app in apps: + status_file = os.path.join(APPS_SETTING_PATH, app, "status.json") + if not os.path.exists(status_file): + continue + + try: + status = read_json(status_file) + current_revision = status.get("remote", {}).get("revision", "?") + app_setting(app, 'current_revision', current_revision) + except Exception as e: + logger.warning("Could not migrate status.json from app %s: %s", (app, str(e))) + else: + os.system("rm %s" % status_file) diff --git a/src/yunohost/tools.py b/src/yunohost/tools.py index f4bb83c15..df462781b 100644 --- a/src/yunohost/tools.py +++ b/src/yunohost/tools.py @@ -525,8 +525,11 @@ def _list_upgradable_apps(): if app_dict["upgradable"] == "yes": + # FIXME : would make more sense for these infos to be computed + # directly in app_info and used to check the upgradability of + # the app... current_version = app_dict.get("version", "?") - current_commit = app_dict.get("status", {}).get("remote", {}).get("revision", "?")[:7] + current_commit = app_dict.get("settings", {}).get("current_revision", "?")[:7] new_version = app_dict.get("manifest",{}).get("version","?") new_commit = app_dict.get("git", {}).get("revision", "?")[:7]