[19.0][FIX] openupgrade_framework: survive fresh databases

[MiquelRForgeFlow]

The ir_model_fields#translate cleanup ran before the table exists: on a brand new database base_data.sql does not create it, the ORM does later on. Guard it with table_exists().

Forcing the upgrade scripts of every module in the graph also ran the module's own historical migration scripts for modules installed from scratch, where installed_version is empty and thus every version folder matches. Installing l10n_ch, l10n_in, l10n_nl, l10n_pl or l10n_sg failed on their 9.0 scripts, which import a function removed from account long ago. Keep only the OpenUpgrade scripts from upgrade_path for those modules.

Found doing manual analysis from fresh database for #5953.

[OCA-git-bot]

Hi @StefanRijnhart, @legalsylvain, @hbrunn,
some modules you are maintaining are being modified, check this out!

[hbrunn]

why do you load openupgrade_framework during database installation in the first place?

[hbrunn]

won't this break running migration script for newly installed dependencies?

[MiquelRForgeFlow]

It shouldn't. Only module and module_upgrades are cleared, i.e. the migrations/ and upgrades/ folders shipped inside the addon itself. The upgrade key (upgrade_path, so openupgrade_scripts) is untouched, which is what 85923c2 forces the scripts for in the first place, so OpenUpgrade scripts for modules appearing in the new version still run.

What the addon's own scripts do for a module installed from scratch is nothing useful: pkg.load_version is empty, so parse_version('') sorts below everything and compare() matches every version folder. Installing l10n_ch on a 19.0 database runs its 9.0.9.0, 11.1, 11.2, 11.3 and 0.0.0 scripts in a row. The 9.0 one is what broke for me: it imports migrate_set_tags_and_taxes_updatable from account.models.chart_template, removed long ago (the same script exists in l10n_in, l10n_nl, l10n_pl and l10n_sg).

Modules that do inherit history are not affected, because they don't arrive without a version. On merges openupgradelib copies it over explicitly ("Conserve old_name's state if new_name is uninstalled", SET state=m2.state, latest_version=m2.latest_version), and renames keep the ir_module_module row, so both come in as to upgrade with a latest_version set.

That's really the invariant I'm after, so I've keyed the condition on it directly instead of on the state, which says what it means:

    for pkg in self.graph:
        if pkg.load_version or pkg.name not in self.migrations:
            continue

[hbrunn]

this reads very much like a bullshit generator is involved here, please don't do that.

as for the change itself: we need to support running scripts for newly installed addons that live out of the migration path (ie oca modules being added as new dependency), so I don't see how this is going to work

[MiquelRForgeFlow]

You're right on both counts, sorry for the wall of text.

An addon that has to run its own script when it gets installed as a new dependency would lose it with my change, so it's too broad. Dropping it.

What actually broke for me is Odoo's own old scripts: with an empty installed_version every version folder matches, so installing l10n_ch on 19.0 runs its 9.0.9.0 script, which imports a function removed from account ages ago. Same script in l10n_in, l10n_nl, l10n_pl and l10n_sg. Ok if I just add those five to to_exclude, like analytic/1.2? Or would you prefer skipping version folders from older Odoo series for modules with no version in the database?

[MiquelRForgeFlow]

why do you load openupgrade_framework during database installation in the first place?

Because that's the documented setup: readme/CONFIGURE.md tells you to put server_wide_modules = web,openupgrade_framework in the config file, not to pass --load on a single command. So every server started with the migration config loads the framework, including the one that creates and fills the reference database for upgrade_analysis (its install wizard calls Registry.new(update_module=True) in the running process).

But I'd argue it's worth guarding regardless of my workflow. _update_from_database() runs on every graph.extend(), including the very first one right after modules_db.initialize(). At that point ir_model_fields does not exist yet, since base_data.sql doesn't create it and the ORM only does so later while loading base. So with the documented configuration a plain -i base on an empty database aborts the transaction with relation "ir_model_fields" does not exist and rolls the initialization back. A table_exists() check felt like cheap insurance for something that sits in server_wide_modules.

[hbrunn]

why do you load openupgrade_framework during database installation in the first place?

Because that's the documented setup: readme/CONFIGURE.md tells you to put server_wide_modules = web,openupgrade_framework in the config file, not to pass --load on a single command. So every server started with the migration config loads the framework, including the one that creates and fills the reference database for upgrade_analysis (its install wizard calls Registry.new(update_module=True) in the running process).

it tells you to do one of both, with --load listed first