"v52.betas.to52beta1" puts the Python import machinery in an inconsistent state

[d-maurer]

Python 2.7, plone.app.upgrade==2.0.34, Products.ResourceRegistries installed:
After the call of to52beta1 import Products.ResourceRegistries fails with cannot import name "config" - even though config.py exists in Products.ResourceRegistries.

The reason: to52beta1 hacks the Python import machinery and brings it into an inconsistent state regarding Products.ResourceRegistries. The import machinery raises an ImportError for from m import n if n happens to be a module in sys.modules and is not an attribute of m. to52beta1 deletes Products.ResourceRegistries but lets Products.ResourceRegistries.config in sys.modules -- causing the inconsistency and the error above.

I recommend to remove all modules related to Products.ResourceRegistries from sys.modules in v52beta1 to properly clean up after the import machinery hack.

In recent versions (e.g. plone.app.upgrade==2.0.34), to52beta1 has got at its end an install_product('plone.staticresources'). This fails for us with the ImportError above (due to the import machinery inconsistency). Apparently, this ImportError is hidden by upper layers of the upgrade machinery (this likely should not happen).

The problems possibly only occur if Products.ResourceRegistries is installed.

[mauritsvanrees]

So that is this function.

And the cleanup here is not enough?

Note that we also have some sys modules hackery for resource registries in the __init__.pyhttps://github.com/plone/plone.app.upgrade/blob/2.0.34/plone/app/upgrade/__init__.py#L162-L164), although that does not touch the config module.

How do we reproduce this? From reading the above, my guess is (without trying it):

• Create a Plone 5.1 site.
• Upgrade the buildout to 5.2, still Python 2.7.
• Make sure that Products.ResourceRegistries is explicitly in the eggs.
• Start 5.2, run the migration at @@plone-upgrade, and it crashes.

[d-maurer]

Maurits van Rees wrote at 2020-9-22 11:24 +0000: Thank you for looking into this.

So that is [this function](https://github.com/plone/plone.app.upgrade/blob/2.0.34/plone/app/upgrade/v52/betas.py#L116-L142).

Yes.

And the [cleanup here](https://github.com/plone/plone.app.upgrade/blob/2.0.34/plone/app/upgrade/v52/betas.py#L133-L134) is not enough?

The cleanup actually starts in line 130. But, it is not enough: it deletes `Products.ResourceRegistries` from `sys.modules` but keeps `Products.ResourceRegistries.config`. This prevents a later `import Products.ResourceRegistries`.

Note that we also have some sys modules hackery for resource registries in the [`__init__.py`]()https://github.com/plone/plone.app.upgrade/blob/2.0.34/plone/app/upgrade/__init__.py#L162-L164), although that does not touch the `config` module.

I have seen this. Apparently, it takes care for the case when `Products.ResourceRegistries` is not installed. This is not out case. To work for both cases (`Products.ResourceRegistries` installed and not installed) it might be better to restore the original state in `to52beta1` rather than delete modules from `sys.modules`.

How do we reproduce this? From reading the above, my guess is (without trying it): - Create a Plone 5.1 site. - Upgrade the buildout to 5.2, still Python 2.7. - Make sure that `Products.ResourceRegistries` is explicitly in the eggs. - Start 5.2, run the migration at `@@plone-upgrade`, and it crashes.

It may not crash visibly -- as the `ImportError` resulting from `install('plone.staticresource')` seems to be silently swallowed up (by upper levels). The scenario in which I observe the problem: - We come from Plone 4 with a Plone 4 site "p" (but likely, this is not relevant) - In a script, we call `p.portal_migration.upgrade(p, swallow_errors=False)` This (apparently) succeeds (because the `ImportError` is swallowed - We import a proprietary `GenericSetup` profile -- this fails due to an `ImportError` importing `Products.ResourceRequistries`. You should be able to reproduce the problem by directly trying an `import Products.ResourceRegistries`. Note that it may be important, that `Products.ResoureRegistries` is installed. The behaviour might be very different if this is not the case.