From c7ab378bd8f8d32a5fe6aa722cc91cc182aeac68 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sun, 23 Aug 2026 01:51:28 -0400 Subject: [PATCH] [FIX] migration: retirer web_responsive avant de monter en 18 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Monter en 18 mourait sur « MuK Backend Theme et Web Responsive sont incompatibles ». muk_web_theme n'excluait que web_enterprise en 16 et en 17 ; la 18 y ajoute web_responsive. Les deux cohabitaient donc légalement depuis la 12. On retire web_responsive au palier 17 → 18, pendant que l'état est encore légal ; rien n'en dépend. Deux défauts trouvés en le câblant. Odoo sort en 0 quand « --uninstall » ne retire rien : muk_web_theme a traversé quatre paliers en étant réputé parti. On lit maintenant l'état en base. Et les étapes désinstaller et installer rangeaient leur drapeau sous la clé de l'étape migration — à la reprise, OpenUpgrade était sauté pour ces paliers. --- EN --- Upgrading to 18 died on « MuK Backend Theme and Web Responsive are incompatible ». muk_web_theme excluded only web_enterprise in 16 and 17; 18 adds web_responsive. The pair had been legal since 12. We drop web_responsive at the 17 → 18 step, while the state is still legal; nothing depends on it. Wiring it up surfaced two defects. Odoo exits 0 when « --uninstall » removes nothing: muk_web_theme crossed four steps while believed gone. We now read the state back from the database. And the uninstall and install steps stored their flag under the migrate step's key — on resume, OpenUpgrade was skipped for those steps. Assisted-by: Claude Opus 5 --- ...install_module_list_odoo140_to_odoo150.txt | 14 +- ...install_module_list_odoo170_to_odoo180.txt | 15 ++ script/todo/todo_i18n.py | 12 ++ script/todo/todo_upgrade.py | 50 ++++- test/test_uninstall_module_list.py | 133 ++++++++++++ test/test_uninstall_verified.py | 189 ++++++++++++++++++ 6 files changed, 405 insertions(+), 8 deletions(-) create mode 100644 script/odoo/migration/uninstall_module_list_odoo170_to_odoo180.txt create mode 100644 test/test_uninstall_module_list.py create mode 100644 test/test_uninstall_verified.py diff --git a/script/odoo/migration/uninstall_module_list_odoo140_to_odoo150.txt b/script/odoo/migration/uninstall_module_list_odoo140_to_odoo150.txt index 5917580..11c47bc 100644 --- a/script/odoo/migration/uninstall_module_list_odoo140_to_odoo150.txt +++ b/script/odoo/migration/uninstall_module_list_odoo140_to_odoo150.txt @@ -1 +1,13 @@ -muk_web_theme +# Modules à retirer AVANT de monter de 14.0 vers 15.0. +# +# Un module par ligne, une justification après « # ». + +# Raison d'origine NON consignée : la ligne arrive avec cbc43fde, sans un +# mot. Mesuré depuis, sur test_neutralize (12 → 18) : la commande part +# bien — « uninstall_addons.sh test_neutralize_upgrade_15 muk_web_theme » +# est dans le journal — et le module est pourtant « installed » de la 15 à +# la 18. Odoo ne cherche que l'état « installed » et le module traînait en +# « to remove » depuis la 13 ; il sort en 0 sans rien retirer. +# Le retrait n'a donc JAMAIS eu lieu, et rien n'a cassé pour autant. +# Avant de s'y fier, retrouver le motif — ou retirer la ligne. +muk_web_theme # motif d'origine inconnu ; retrait jamais effectif (mesuré) diff --git a/script/odoo/migration/uninstall_module_list_odoo170_to_odoo180.txt b/script/odoo/migration/uninstall_module_list_odoo170_to_odoo180.txt new file mode 100644 index 0000000..d197119 --- /dev/null +++ b/script/odoo/migration/uninstall_module_list_odoo170_to_odoo180.txt @@ -0,0 +1,15 @@ +# Modules à retirer AVANT de monter de 17.0 vers 18.0. +# +# Un module par ligne, une justification après « # ». + +# muk_web_theme n'excluait que web_enterprise en 16 et en 17 ; la 18 y +# ajoute web_responsive. Les deux cohabitaient donc légalement jusqu'ici, +# et la base arrive en 18 dans un état que la 18 interdit. Le chargement +# meurt dès que `Module.update_list()` installe un module auto_install, +# car Odoo revérifie alors toutes les exclusions : +# UserError: Les modules "MuK Backend Theme" et "Web Responsive" +# sont incompatibles. +# On retire web_responsive et l'on garde le thème MuK, qui porte +# l'apparence du back-office. L'inverse marcherait aussi : les deux +# rendent le même service, il faut simplement en choisir un. +web_responsive # exclu par muk_web_theme a partir de la 18.0 diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index fa00db3..5794700 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -1748,6 +1748,18 @@ TRANSLATIONS = { "fr": "Rien de désinstallé.", "en": "Nothing uninstalled.", }, + "The uninstall did not take:": { + "fr": "La désinstallation n'a pas pris :", + "en": "The uninstall did not take:", + }, + "Odoo exits 0 even when it removes nothing.": { + "fr": "Odoo sort en 0 même quand il ne retire rien.", + "en": "Odoo exits 0 even when it removes nothing.", + }, + "Could not verify the uninstall.": { + "fr": "Impossible de vérifier la désinstallation.", + "en": "Could not verify the uninstall.", + }, "Check the COW views that drifted": { "fr": "Vérifier les vues COW en retard sur leur vue module", "en": "Check the COW views that drifted", diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index 84793da..c644526 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2040,9 +2040,6 @@ class TodoUpgrade: next_version - 1, ) lst_module_uninstall_module[index] = True - self.dct_progression["state_4_module_migrate_odoo_lst"] = ( - lst_module_uninstall_module - ) self.write_config() self.dct_progression["config_state_4_uninstall_module"] = ( @@ -2087,9 +2084,6 @@ class TodoUpgrade: next_version - 1, ) lst_module_install_module[index] = True - self.dct_progression["state_4_module_migrate_odoo_lst"] = ( - lst_module_install_module - ) self.write_config() self.dct_progression["config_state_4_install_module"] = ( @@ -3198,6 +3192,34 @@ class TodoUpgrade: self.open_step_log(msg) print(f"🔷 {prefix}{sep}{t(label)}" if sep else f"🔷 {t(msg)}") + def still_installed(self, database_name, lst_module): + """Parmi ces modules, lesquels la base tient-elle ENCORE ? + + Odoo ne signale rien quand il ne retire rien : « --uninstall » ne + cherche que l'état « installed » et laisse filer en silence un module + resté en « to remove » d'une tentative précédente. Le code de sortie + vaut donc 0 pour une désinstallation qui n'a pas eu lieu — c'est ainsi + que muk_web_theme a traversé quatre paliers en étant réputé retiré. + + Rendre None, et non la liste vide, quand la base ne répond pas : + « je ne sais pas » et « rien ne reste » appellent des suites + différentes, et les confondre recrée le défaut qu'on corrige. + """ + if not lst_module: + return [] + noms = ", ".join(f"'{nom}'" for nom in sorted(set(lst_module))) + status, _cmd, output = self.todo_upgrade_execute( + f'psql -X -w -d {database_name} -tAc "SELECT name FROM' + f" ir_module_module WHERE name IN ({noms})" + " AND state <> 'uninstalled' ORDER BY name;\"", + get_output=True, + wait_at_error=False, + quiet=True, + ) + if status: + return None + return [line.strip() for line in (output or []) if line.strip()] + def installed_theme(self, database_name): """Thèmes installés, hors theme_default qui EST l'absence de thème.""" status, _cmd, output = self.todo_upgrade_execute( @@ -3773,12 +3795,26 @@ class TodoUpgrade: single_source_odoo=True, ) + lst_left = self.still_installed(database_name, lst_module_to_uninstall) + if lst_left is None: + # Base illisible : on ne sait pas. Le dire, plutôt que de trancher. + print(f"⚠️ {t('Could not verify the uninstall.')}") + lst_left = [] + elif lst_left: + self.add_comment_progression( + "uninstall - still installed: " + ", ".join(lst_left) + ) + print( + f"❌ {t('The uninstall did not take:')} {', '.join(lst_left)}" + ) + print(f" {t('Odoo exits 0 even when it removes nothing.')}") + # Update list installed module — only what was REALLY uninstalled, so # a module left in place stays counted as installed. self.dct_module_per_version[actual_version] = sorted( list( set(self.dct_module_per_version[actual_version]) - - set(lst_module_to_uninstall) + - (set(lst_module_to_uninstall) - set(lst_left)) ) ) self.dct_progression["dct_module_per_version"] = ( diff --git a/test/test_uninstall_module_list.py b/test/test_uninstall_module_list.py new file mode 100644 index 0000000..98b6cc2 --- /dev/null +++ b/test/test_uninstall_module_list.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Les modules qu'un palier doit retirer AVANT de monter. + +Certaines incompatibilités n'existent qu'à partir d'une version donnée. +`muk_web_theme` n'excluait que `web_enterprise` en 16 et en 17 ; la 18 y +ajoute `web_responsive`. Les deux cohabitaient donc légalement, et la +base arrive en 18 dans un état que la 18 interdit — le chargement meurt +dès qu'un module auto_install est installé, car Odoo revérifie alors +toutes les exclusions. + +Le retrait doit se faire pendant qu'on est ENCORE sur l'ancienne +version, là où l'état est légal et où l'ORM fonctionne. +""" + +import io +import os +import sys +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.todo.todo_upgrade import TodoUpgrade # noqa: E402 + +RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +DOSSIER = os.path.join(RACINE, "script", "odoo", "migration") + + +def lecteur(): + return TodoUpgrade.__new__(TodoUpgrade) + + +class TestTheSeventeenToEighteenList(unittest.TestCase): + def test_the_file_exists_where_the_driver_looks(self): + chemin = os.path.join( + DOSSIER, "uninstall_module_list_odoo170_to_odoo180.txt" + ) + self.assertTrue(os.path.isfile(chemin), chemin) + + def test_it_names_web_responsive(self): + modules, _detail = lecteur().read_uninstall_module_list(17, "peu") + self.assertIn("web_responsive", modules) + + def test_the_removal_is_justified(self): + # Sans justification, on retrouve un module retiré des mois plus + # tard sans pouvoir dire pourquoi, ni s'il faut le remettre. + _m, detail = lecteur().read_uninstall_module_list(17, "peu") + raisons = {module: raison for module, raison, _f in detail} + self.assertTrue(raisons.get("web_responsive"), raisons) + + def test_muk_web_theme_is_NOT_removed_here(self): + # Les deux rendent le même service : il faut en garder un, et + # c'est le thème qui porte l'apparence du back-office. + modules, _d = lecteur().read_uninstall_module_list(17, "peu") + self.assertNotIn("muk_web_theme", modules) + + +class TestEveryListIsWellFormed(unittest.TestCase): + def fichiers(self): + import glob + + return sorted( + glob.glob(os.path.join(DOSSIER, "uninstall_module_list_*.txt")) + ) + + def test_there_is_at_least_one(self): + # Sans cette borne, le test suivant passerait en ne vérifiant + # rien le jour où le motif de nom change. + self.assertGreater(len(self.fichiers()), 0) + + def test_every_entry_is_justified(self): + for chemin in self.fichiers(): + for module, raison in TodoUpgrade.parse_module_list_file(chemin): + self.assertTrue( + raison, + f"{os.path.basename(chemin)} : {module} sans raison", + ) + + def test_no_entry_looks_like_a_stray_comment(self): + # Le parseur coupe à « # » : une ligne mal écrite produirait un + # nom de module fantôme, retiré en silence de rien du tout. + for chemin in self.fichiers(): + for module, _r in TodoUpgrade.parse_module_list_file(chemin): + self.assertRegex(module, r"^[a-z][a-z0-9_]*$", module) + + def test_the_name_encodes_the_bump_it_serves(self): + import re + + for chemin in self.fichiers(): + nom = os.path.basename(chemin) + trouve = re.match( + r"uninstall_module_list_odoo(\d+)_to_odoo(\d+)\.txt$", nom + ) + self.assertIsNotNone(trouve, nom) + depart, arrivee = (int(x) for x in trouve.groups()) + self.assertEqual(arrivee, depart + 10, nom) + + +class TestWhenItRuns(unittest.TestCase): + def test_the_uninstall_precedes_the_openupgrade_run(self): + # Retirer un module APRÈS la montée serait trop tard : c'est la + # montée elle-même qui refuse l'état. + with io.open( + os.path.join(RACINE, "script", "todo", "todo_upgrade.py"), + encoding="utf-8", + ) as handle: + src = handle.read() + self.assertLess( + src.index("- Uninstall module"), src.index("- Migrate database") + ) + + def test_a_private_list_wins_over_the_shared_one(self): + # Une base peut avoir ses propres retraits sans qu'on touche à la + # liste partagée de tout le monde. + source = io.open( + os.path.join(RACINE, "script", "todo", "todo_upgrade.py"), + encoding="utf-8", + ).read() + debut = source.index("def read_uninstall_module_list") + fin = source.index("def split_present_missing") + bloc = source[debut:fin] + self.assertLess( + bloc.index("PATH_MIGRATION_PRIVATE"), + bloc.index("PATH_MIGRATION_GLOBAL"), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_uninstall_verified.py b/test/test_uninstall_verified.py new file mode 100644 index 0000000..a38b8f8 --- /dev/null +++ b/test/test_uninstall_verified.py @@ -0,0 +1,189 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Une désinstallation se MESURE, elle ne se suppose pas. + +`odoo-bin --uninstall` ne cherche que l'état « installed ». Un module resté +en « to remove » d'une tentative précédente est ignoré en silence, et Odoo +sort en 0. Le pilote tenait ce 0 pour une réussite : c'est ainsi que +muk_web_theme a traversé quatre paliers de 12 → 18 en étant réputé retiré, +alors qu'il était « installed » de la 15 à la 18. +""" + +import ast +import io +import os +import sys +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.todo.todo_upgrade import TodoUpgrade # noqa: E402 + +RACINE = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +SOURCE = os.path.join(RACINE, "script", "todo", "todo_upgrade.py") + + +class FauxPilote(TodoUpgrade): + """Un pilote qui n'exécute rien, mais respecte le contrat de retour.""" + + def __init__(self, survivants=(), lisible=True): + self.survivants = list(survivants) + self.lisible = lisible + self.commandes = [] + self.commentaires = [] + self.dct_progression = {} + self.dct_module_per_version = {} + + def todo_upgrade_execute(self, cmd, **kwargs): + self.commandes.append(cmd) + if kwargs.get("get_output"): + # Le vrai rend TROIS valeurs quand on demande la sortie, et un + # statut NON nul veut dire « raté ». + if not self.lisible: + return 1, cmd, None + return 0, cmd, list(self.survivants) + return 0, cmd + + def write_config(self): + pass + + def add_comment_progression(self, msg): + self.commentaires.append(msg) + + def split_present_missing(self, lst): + return list(lst), [] + + +class TestStillInstalled(unittest.TestCase): + def test_it_names_what_survived(self): + pilote = FauxPilote(survivants=["muk_web_theme"]) + self.assertEqual( + pilote.still_installed("db", ["muk_web_theme", "web_responsive"]), + ["muk_web_theme"], + ) + + def test_an_unreadable_database_says_UNKNOWN_not_empty(self): + # Rendre [] serait affirmer « tout est parti » sans l'avoir lu : + # exactement le défaut qu'on corrige. + pilote = FauxPilote(lisible=False) + self.assertIsNone(pilote.still_installed("db", ["web_responsive"])) + + def test_nothing_to_check_asks_the_database_nothing(self): + pilote = FauxPilote() + self.assertEqual(pilote.still_installed("db", []), []) + self.assertEqual(pilote.commandes, []) + + def test_it_counts_to_remove_as_still_there(self): + # « to remove » n'est pas « uninstalled » : c'est justement l'état + # que --uninstall refuse de traiter, donc celui qu'il faut voir. + pilote = FauxPilote() + pilote.still_installed("db", ["web_responsive"]) + self.assertIn("state <> 'uninstalled'", pilote.commandes[0]) + + def test_the_module_names_are_quoted_for_sql(self): + pilote = FauxPilote() + pilote.still_installed("db", ["web_responsive"]) + self.assertIn("'web_responsive'", pilote.commandes[0]) + + +class TestTheBookkeepingTellsTheTruth(unittest.TestCase): + def pilote(self, survivants): + pilote = FauxPilote(survivants=survivants) + pilote.dct_module_per_version = { + 17: ["web_responsive", "muk_web_theme"] + } + return pilote + + def test_a_module_left_in_place_stays_counted_as_installed(self): + pilote = self.pilote(["web_responsive"]) + pilote.uninstall_from_database(["web_responsive"], "db", 17) + self.assertIn("web_responsive", pilote.dct_module_per_version[17]) + + def test_a_module_really_gone_is_dropped(self): + pilote = self.pilote([]) + pilote.uninstall_from_database(["web_responsive"], "db", 17) + self.assertNotIn("web_responsive", pilote.dct_module_per_version[17]) + # …et sans emporter le voisin au passage. + self.assertIn("muk_web_theme", pilote.dct_module_per_version[17]) + + def test_the_survivor_is_recorded_where_someone_will_read_it(self): + pilote = self.pilote(["web_responsive"]) + pilote.uninstall_from_database(["web_responsive"], "db", 17) + trace = " ".join(pilote.commentaires) + self.assertIn("still installed", trace) + self.assertIn("web_responsive", trace) + + def test_an_unreadable_database_does_not_crash_the_migration(self): + # « je ne sais pas » revient en None : le traiter comme une liste + # ferait tomber la migration sur un TypeError, six heures après le + # départ, pour un renseignement qui n'était que confortable. + pilote = FauxPilote(lisible=False) + pilote.dct_module_per_version = {17: ["web_responsive"]} + pilote.uninstall_from_database(["web_responsive"], "db", 17) + self.assertEqual(pilote.dct_module_per_version[17], []) + + def test_a_silent_success_leaves_no_alarm(self): + pilote = self.pilote([]) + pilote.uninstall_from_database(["web_responsive"], "db", 17) + self.assertEqual( + [c for c in pilote.commentaires if "still installed" in c], [] + ) + + +class TestNoStepWritesAnotherStepsFlag(unittest.TestCase): + """Chaque drapeau `state_4_*` ne doit porter QUE sa propre liste. + + Trois étapes rangeaient leurs drapeaux sous + `state_4_module_migrate_odoo_lst`. Sans effet dans la course en cours — + la locale est lue une fois, au début — mais à la REPRISE cette clé est + relue comme « OpenUpgrade est passé », et la migration du palier est + sautée. Un test structurel se justifie ici : conduire une reprise + complète coûterait des heures, et la faute est visible dans l'écriture. + """ + + def assignations(self): + with io.open(SOURCE, encoding="utf-8") as handle: + arbre = ast.parse(handle.read()) + vues = {} + for noeud in ast.walk(arbre): + if not isinstance(noeud, ast.Assign): + continue + for cible in noeud.targets: + if not ( + isinstance(cible, ast.Subscript) + and isinstance(cible.value, ast.Attribute) + and cible.value.attr == "dct_progression" + and isinstance(cible.slice, ast.Constant) + and str(cible.slice.value).startswith("state_4_") + ): + continue + if isinstance(noeud.value, ast.Name): + vues.setdefault(cible.slice.value, set()).add( + noeud.value.id + ) + return vues + + def test_the_scan_actually_finds_something(self): + # Sans cette borne, le test suivant passerait sur un dictionnaire + # vide le jour où la forme de l'écriture change. + self.assertGreater(len(self.assignations()), 2) + + def test_each_flag_is_written_from_one_list_only(self): + for cle, noms in sorted(self.assignations().items()): + self.assertEqual( + len(noms), 1, f"{cle} écrit depuis {sorted(noms)}" + ) + + def test_the_migrate_flag_comes_from_the_migrate_list(self): + self.assertEqual( + self.assignations().get("state_4_module_migrate_odoo_lst"), + {"lst_module_migrate_odoo"}, + ) + + +if __name__ == "__main__": + unittest.main()