From ec6ac3a18871f9e630fa7235c1773c42d30cf316 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sun, 16 Aug 2026 05:33:28 -0400 Subject: [PATCH] [FIX] addons: stop calling a leftover report a failed command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit « Command returned error code: 1 » followed a theme that was removed correctly. 1 is this toolkit's « there are findings », but the report was the uninstaller's last command, so its code became the script's; and the COW check was still read through the capturing executor, which announces any non-zero code as an error. The leftovers can now be dealt with instead of only listed: keep by default, or delete after their content is written to private/. Listing fifteen attachments and stopping there meant composing an unlink() by hand, mid-migration, from identifiers read off a screen. --- FR --- [FIX] addons : cesser d'appeler « erreur » un rapport de restes « Command returned error code: 1 » suivait un thème correctement retiré. 1 veut dire « il y a des constats » dans cet outillage, mais le rapport était la dernière commande du désinstalleur, donc son code devenait celui du script ; et la vérification COW passait encore par l'exécuteur qui capture, lequel annonce tout code non nul comme une erreur. Les restes peuvent désormais être traités, pas seulement listés : garder par défaut, ou effacer après écriture de leur contenu sous private/. Lister quinze pièces jointes et s'arrêter là revenait à faire composer un unlink() à la main, en pleine migration. Assisted-by: Claude Opus 5 (cherry picked from commit 304ce32b2680e3d01e38c3224bb85c2798cd6f53) --- script/addons/theme_leftover.py | 133 +++++++++++++++++++++++- script/addons/uninstall_addons_theme.sh | 6 +- script/todo/todo_i18n.py | 28 +++++ script/todo/todo_upgrade.py | 10 +- test/test_uninstall_addons_theme.py | 118 +++++++++++++++++++++ 5 files changed, 289 insertions(+), 6 deletions(-) diff --git a/script/addons/theme_leftover.py b/script/addons/theme_leftover.py index 38b60da..80e70c2 100755 --- a/script/addons/theme_leftover.py +++ b/script/addons/theme_leftover.py @@ -121,12 +121,118 @@ def render(theme, attachments, views): return "\n".join(lines) + "\n" +def backup_attachments(database, theme, lst_row, filestore=None): + """Écrire le contenu des pièces jointes AVANT de les supprimer. + + C'est la condition pour pouvoir répondre « efface ». Sans elle, on + détruirait ce dont on vient d'écrire qu'il peut être la seule trace + d'une personnalisation. + """ + base = filestore or os.path.join( + os.path.expanduser("~"), + ".local", + "share", + "Odoo", + "filestore", + database, + ) + directory = os.path.join( + "private", "odoo", "migration", database, "theme_backup", theme + ) + os.makedirs(directory, exist_ok=True) + lst_saved = [] + for row in lst_row: + att_id = row.split("|")[0] + rows = run_psql( + database, + "SELECT COALESCE(store_fname, '') FROM ir_attachment" + f" WHERE id = {int(att_id)};", + ) + store_fname = rows[0].strip() if rows else "" + if not store_fname: + continue + source = os.path.join(base, store_fname) + if not os.path.isfile(source): + continue + target = os.path.join( + directory, f"{att_id}_" + os.path.basename(row.split("|")[1]) + ) + with open(source, "rb") as src, open(target, "wb") as dst: + dst.write(src.read()) + lst_saved.append(target) + return lst_saved + + +def delete_attachments(database, lst_row, config_path="./config.conf"): + """Supprimer par le shell d'Odoo, pour qu'il gère aussi le filestore. + + Un DELETE en SQL laisserait les fichiers orphelins et les caches + incohérents ; unlink() fait le ménage complet, dans toutes les versions. + """ + lst_id = [row.split("|")[0] for row in lst_row] + script = ( + f"env['ir.attachment'].browse({lst_id!r}).unlink()\n" + "env.cr.commit()\n" + ) + done = subprocess.run( + ["./odoo_bin.sh", "shell", "-c", config_path, "-d", database], + input=script, + capture_output=True, + text=True, + ) + return done.returncode, done.stdout + done.stderr + + +def prompt(database, theme, attachments, views, config_path, ask=input): + """Garder ou effacer. « Garder » par défaut, et la sauvegarde d'abord.""" + if not attachments: + return False + answer = ( + ask( + f"💬 {t('Delete these leftovers, or keep them?')}" + f" ({t('Enter = keep')}, d = {t('delete, after saving them')}) : " + ) + .strip() + .lower() + ) + if answer != "d": + print(f"ℹ -> {t('Kept. Nothing was deleted.')}") + return False + lst_saved = backup_attachments(database, theme, attachments) + print(f"📦 {t('Saved before deleting')} : {len(lst_saved)}") + if lst_saved: + print(f" {os.path.dirname(lst_saved[0])}") + status, output = delete_attachments(database, attachments, config_path) + print(output.strip()[-1500:]) + if status: + print(f"❌ {t('Deletion failed, nothing was removed.')}") + return False + print(f"✅ -> {len(attachments)} {t('attachment(s) deleted.')}") + return True + + def main(argv=None): parser = argparse.ArgumentParser( description=("List what an uninstalled theme left behind (read-only).") ) parser.add_argument("-d", "--database", required=True) parser.add_argument("-t", "--theme", required=True) + parser.add_argument( + "-c", + "--config", + default="./config.conf", + help="Odoo config used by the shell for --delete", + ) + parser.add_argument( + "--delete", + action="store_true", + help="delete them (WRITES; saves their content first)", + ) + parser.add_argument( + "--report-only", + action="store_true", + help="never ask anything, even in front of a terminal", + ) config = parser.parse_args(argv) try: attachments, views = collect(config.database, config.theme) @@ -134,7 +240,32 @@ def main(argv=None): print(f"❌ {exc}") return 2 print(render(config.theme, attachments, views)) - return 1 if (attachments or views) else 0 + if not attachments and not views: + return 0 + if config.delete: + lst_saved = backup_attachments( + config.database, config.theme, attachments + ) + print(f"📦 {t('Saved before deleting')} : {len(lst_saved)}") + status, output = delete_attachments( + config.database, attachments, config.config + ) + print(output.strip()[-1500:]) + if status: + print(f"❌ {t('Deletion failed, nothing was removed.')}") + return 2 + print(f"✅ -> {len(attachments)} {t('attachment(s) deleted.')}") + return 0 + if not config.report_only and sys.stdin.isatty(): + if prompt( + config.database, + config.theme, + attachments, + views, + config.config, + ): + return 0 + return 1 if __name__ == "__main__": diff --git a/script/addons/uninstall_addons_theme.sh b/script/addons/uninstall_addons_theme.sh index 5eafad4..aa45da1 100755 --- a/script/addons/uninstall_addons_theme.sh +++ b/script/addons/uninstall_addons_theme.sh @@ -89,4 +89,8 @@ fi # sont parties, mais elles survivent à toutes les migrations suivantes et # personne ne sait plus d'où elles viennent. On les signale, on ne les # supprime pas : leur contenu peut être la seule trace d'une personnalisation. -./script/addons/theme_leftover.py -d "$DATABASE" -t "$THEME" +# Son code de sortie dit « il reste des choses », pas « la désinstallation a +# échoué » : le laisser devenir celui du script faisait annoncer une erreur +# sur un thème correctement retiré. +./script/addons/theme_leftover.py -d "$DATABASE" -t "$THEME" -c "$CONFIG" || true +exit 0 diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 8d71d69..43a78f7 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -4950,6 +4950,34 @@ TRANSLATIONS = { "fr": "Choix inconnu, rien n'a été réinitialisé.", "en": "Unknown choice, nothing was reset.", }, + "Delete these leftovers, or keep them?": { + "fr": "Effacer ces restes, ou les garder ?", + "en": "Delete these leftovers, or keep them?", + }, + "Enter = keep": { + "fr": "Entrée = garder", + "en": "Enter = keep", + }, + "delete, after saving them": { + "fr": "effacer, après les avoir sauvegardés", + "en": "delete, after saving them", + }, + "Kept. Nothing was deleted.": { + "fr": "Conservés. Rien n'a été effacé.", + "en": "Kept. Nothing was deleted.", + }, + "Saved before deleting": { + "fr": "Sauvegardé avant effacement", + "en": "Saved before deleting", + }, + "Deletion failed, nothing was removed.": { + "fr": "L'effacement a échoué, rien n'a été retiré.", + "en": "Deletion failed, nothing was removed.", + }, + "attachment(s) deleted.": { + "fr": "pièce(s) jointe(s) effacée(s).", + "en": "attachment(s) deleted.", + }, "Nothing to decide yet": { "fr": "Rien à décider pour l'instant", "en": "Nothing to decide yet", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index d170eeb..27341a1 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -1624,11 +1624,13 @@ class TodoUpgrade: # -- before hours of migration -- leaves time to arbitrate. # Only the next bump can be predicted: the modes in database describe # the current version. - status, cmd_executed, output = self.todo_upgrade_execute( + # Sur le terminal, sans capturer : plus rien ne relit cette sortie — + # la décision se prend sur le code de retour — et la capture faisait + # annoncer « Command returned error code: 1 » sur un rapport qui va + # bien. 1 veut dire « des copies casseront », pas « l'outil a raté ». + status = self.run_on_terminal( f"{PYTHON_BIN} ./script/odoo/migration/check_cow_views.py" - f" -d {database_name} -t odoo{start_version + 1}.0", - get_output=True, - wait_at_error=False, + f" -d {database_name} -t odoo{start_version + 1}.0" ) # L'avertissement annonçait un problème, invitait à arbitrer, et # aucune question ne suivait : dire « la question viendra au palier » diff --git a/test/test_uninstall_addons_theme.py b/test/test_uninstall_addons_theme.py index bf22e3e..f8fd8c8 100755 --- a/test/test_uninstall_addons_theme.py +++ b/test/test_uninstall_addons_theme.py @@ -213,6 +213,124 @@ class TestTheMigrationOffersIt(unittest.TestCase): self.assertIn("name <> 'theme_default'", source) +class TestKeepOrDeleteTheLeftovers(unittest.TestCase): + """Signaler sans offrir le geste oblige à le composer soi-même. + + Le rapport listait quinze pièces jointes et s'arrêtait là. Les effacer + demandait de retrouver les identifiants et d'écrire un unlink() à la + main — au milieu d'une migration, c'est ce qu'on ne fait pas. + + « Garder » reste le défaut, et rien n'est effacé sans avoir été écrit sur + disque d'abord : c'est la condition pour pouvoir répondre « efface ». + """ + + def setUp(self): + from script.todo import todo_i18n + + self.addCleanup( + setattr, todo_i18n, "_current_lang", todo_i18n._current_lang + ) + todo_i18n._current_lang = "en" + self.rows = ["4457|/theme_x/static/a.scss|2021-03-04"] + self.saved = [] + self.deleted = [] + self.original_backup = theme_leftover.backup_attachments + self.original_delete = theme_leftover.delete_attachments + theme_leftover.backup_attachments = ( + lambda db, th, rows, fs=None: self.saved.append(rows) or ["/tmp/x"] + ) + theme_leftover.delete_attachments = ( + lambda db, rows, cfg="./config.conf": ( + self.deleted.append(rows), + (0, "ok"), + )[1] + ) + self.addCleanup( + setattr, + theme_leftover, + "backup_attachments", + self.original_backup, + ) + self.addCleanup( + setattr, theme_leftover, "delete_attachments", self.original_delete + ) + + def run_prompt(self, answer, rows=None): + import contextlib + import io + + out = io.StringIO() + with contextlib.redirect_stdout(out): + done = theme_leftover.prompt( + "db", + "theme_x", + self.rows if rows is None else rows, + [], + "./config.conf", + ask=lambda prompt: answer, + ) + return done, out.getvalue() + + def test_enter_keeps_them(self): + done, text = self.run_prompt("") + self.assertFalse(done) + self.assertEqual(self.deleted, []) + self.assertIn("Kept", text) + + def test_d_saves_before_deleting(self): + # L'ORDRE est le point : effacer d'abord rendrait la sauvegarde vide. + done, _ = self.run_prompt("d") + self.assertTrue(done) + self.assertEqual(len(self.saved), 1) + self.assertEqual(len(self.deleted), 1) + + def test_a_failed_deletion_says_nothing_was_removed(self): + theme_leftover.delete_attachments = ( + lambda db, rows, cfg="./config.conf": (1, "boom") + ) + done, text = self.run_prompt("d") + self.assertFalse(done) + self.assertIn("nothing was removed", text) + + def test_no_leftover_asks_nothing(self): + done, text = self.run_prompt("d", rows=[]) + self.assertFalse(done) + self.assertEqual(text, "") + + def test_the_prompt_stays_out_of_a_pipe(self): + with open(theme_leftover.__file__) as handle: + self.assertIn("sys.stdin.isatty()", handle.read()) + + +class TestTheMisleadingErrorCode(unittest.TestCase): + """« 1 » veut dire « il reste des choses », pas « ça a raté ».""" + + def test_the_uninstaller_does_not_fail_on_leftovers(self): + # Le rapport était la dernière commande du script : son code + # devenait celui du script, et la migration annonçait une erreur sur + # un thème correctement retiré. + with open(SCRIPT) as handle: + source = handle.read() + self.assertIn("theme_leftover.py", source) + queue = source[source.index("theme_leftover.py") :] + self.assertIn("|| true", queue) + self.assertIn("exit 0", queue) + + def test_the_cow_check_no_longer_goes_through_the_capturing_executor(self): + # exec_command_live imprime « Command returned error code: 1 » dès + # qu'un code non nul sort, y compris sur un rapport qui va bien. + import inspect + + from script.todo.todo_upgrade import TodoUpgrade + + source = inspect.getsource(TodoUpgrade.execute_odoo_upgrade) + avant = source[: source.index("check_cow_views.py")] + self.assertGreater( + avant.rfind("run_on_terminal("), + avant.rfind("todo_upgrade_execute("), + ) + + class TestExitCodes(unittest.TestCase): """0 rien, 1 des restes, 2 l'outil a échoué — comme les outils voisins."""