4fa33dfc (erp5: py3: func_{code,defaults} was replaced in Python3 by __{code,defaults}__., 2022-04-25 was not complete, two parts were missing, it resulted in errors during indexation:

Traceback (innermost last):
  Module Products.CMFActivity.ActivityTool, line 1640, in invokeGroup
    traverse(method_id)(expanded_object_list)
  Module Products.ERP5Catalog.CatalogTool, line 980, in catalogObjectList
    super(CatalogTool, self).catalogObjectList(tmp_object_list, **m.kw)
  Module Products.ZSQLCatalog.ZSQLCatalog, line 816, in catalogObjectList
    **kw
  Module Products.ZSQLCatalog.SQLCatalog, line 1276, in catalogObjectList
    idxs=idxs)
  Module Products.ZSQLCatalog.SQLCatalog, line 1432, in _catalogObjectList
    for x in self._getCatalogMethodArgumentList(method)
  Module Products.ZSQLCatalog.SQLCatalog, line 1457, in _getCatalogMethodArgumentList
    return method.__code__.co_varnames[:method.__code__.co_argcount]
AttributeError: __code__

First, we also needed to port the "script magic" change to force recompilation, this we can do by adding the patch where this problem was fixed in zope: https://github.com/zopefoundation/Products.PythonScripts/commit/590125ae7c20994055c12be9ba03ae0b43b102ec

Also, something was missing in the patch itself, original Zope 4 code is this this:

    if self.__defaults__ != defaults:
        self.__defaults__ = self.__defaults__ = defaults  # ⬅ not really correct, assigns to __defaults__, should ideally also assign to func_defaults for python 2 ( but that does not really matter )
    code = FuncCode(varnames, argcount)
    if self.__code__ != code:
        self.__code__ = self.__code__ = code  # ⬅ not really correct, assigns to __code__, should ideally also assign to func_code for python 2 ( but that does not really matter )

but our patch was like this:

    if self.func_defaults != defaults:  # ⬅ 😱 compares against func_defaults, but func_defaults is already correct, so __defaults__ does not get updated
        self.__defaults__ = self.func_defaults = defaults  # ⬅ here we assign to defaults, that's probably better
    code = FuncCode(varnames, argcount)
    if self.func_code != code:  # ⬅ 😱 same problem with func_code
        self.__code__ = self.func_code = code

So there was two problems 1) we don't recompile, 2) even if we recompile, we don't update __code__ .

Part 2) of the problem was addressed by these changes, using a patch like this:

    if self.__defaults__ != defaults:  # ⬅ compares against __defaults__, which needed also to backport the changes so that __defaults__ is set on the class
        self.__defaults__ = self.func_defaults = defaults  # ⬅ assign to both func_defaults and __defaults__
    code = FuncCode(varnames, argcount)
    if self.__code__ != code:
        self.__code__ = self.func_code = code

Now we should ask why this was not noticed by the test suite, the situation was that indexation was not working, so it should be detected by ERP5's testUpgradeInstanceWithOldDataFs and/or upgrade_test for software release should cover this. testUpgradeInstanceWithOldDataFs does not detect, because we have a patch in the test framework to compile python script on execution, to speed up. I did not try how much it impacts the execution time of scripts, but I think for tests running portal_components it does not change anything and for tests running on test node if we have to wait 1h or 1h and 5 minutes it does not really matter. I did not follow erp5@f537d4e9 but it seems this already bite us once ( commit message says "severe regression" ). If someone can take a closer look at this and evaluate removing this patch, please do it. I believe we can remove optimize fully, the rest is also problematic. upgrade_test from SlapOS tests did not catch the regression either, because it just tests that the promises are OK and the login page can be displayed. I have extended this test to do a bit more: create a document before upgrade, create another document after upgrade and search for both. This test failed as expected.

This also includes some remove removal of old eggs that are no longer needed