Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zope needs to change the way it enumerate Products? #1239

Closed
dwt opened this issue Nov 26, 2024 · 6 comments · Fixed by #1240
Closed

Zope needs to change the way it enumerate Products? #1239

dwt opened this issue Nov 26, 2024 · 6 comments · Fixed by #1240

Comments

@dwt
Copy link
Contributor

dwt commented Nov 26, 2024

When Zope loads Products, it uses code like this:

Link to the code in Zope

for index, product_dir in enumerate(Products.__path__):
        product_names = os.listdir(product_dir)
        for product_name in product_names:
            if _is_package(product_dir, product_name):
                # i is used as sort ordering in case a conflict exists
                # between Product names.  Products will be found as
                # per the ordering of Products.__path__
                products.append((0, product_name, index, product_dir))

However the Python Packaging Guide seems to recommend something different

import importlib
import pkgutil

import myapp.plugins

def iter_namespace(ns_pkg):
    # Specifying the second argument (prefix) to iter_modules makes the
    # returned name an absolute name instead of a relative one. This allows
    # import_module to work without having to do additional modification to
    # the name.
    return pkgutil.iter_modules(ns_pkg.__path__, ns_pkg.__name__ + ".")

discovered_plugins = {
    name: importlib.import_module(name)
    for finder, name, ispkg
    in iter_namespace(myapp.plugins)
}

This seems relevant as I'm currently investigating a bug around imports of namespaces packages installed with recent versions of setuptools and pip, where pip deprecated the hardcoded special handling of setup.py/setup.cfg without a pyproject.toml file present. See details here

Looking at the blame of that code didn't quite allow me to understand the reasoning behind it, and it may very well be that in the past this was the correct way to get the contents of namespace packages.

But it seems this may have changed?

I would like your perspective on wether the old way might be better or if we should migrate to the new way and how to test if that brings up any issues.

@dataflake
Copy link
Member

It sounds like you're more or less speculating that the old code (which has been in place since the times of Zope 2) is causing the issue you're seeing. Unless there's proof I'd be wary of just changing that.

Testing that would involve answering questions like "are all products showing up?" and "do all products work?".

@d-maurer
Copy link
Contributor

d-maurer commented Nov 26, 2024 via email

@dwt
Copy link
Contributor Author

dwt commented Nov 26, 2024

In an experiment I've run, zope is installed normally, but I'm installing a branch of zms with a pyproject.toml in editable mode.

That in turn gives me these results:

>>> import importlib
>>> import pkgutil
>>> from pprint import pprint
>>>
>>> import Products
>>> # what the __path__ contains
>>> pprint(Products.__path__)
_NamespacePath(['/home/zope/venv/lib/python3.12/site-packages/Products', 'Products', '__editable__.ZMS-5.2.0.finder.__path_hook__', '/home/zope/venv/src/products-zms-skins/Products', '/home/zope/Products'])
>>> # what python packaging suggests
>>> pprint(list(pkgutil.iter_modules(Products.__path__, Products.__name__ + ".")))
[ModuleInfo(module_finder=FileFinder('/home/zope/Products'), name='Products.zms', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.BTreeFolder2', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.CMFCore', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.ExternalMethod', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.Five', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.GenericSetup', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.LDAPMultiPlugins', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.LDAPUserFolder', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.MailHost', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.OFSP', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.PageTemplates', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.PluggableAuthService', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.PluginIndexes', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.PluginRegistry', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.PythonScripts', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.SQLAlchemyDA', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.Sessions', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.SiteAccess', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.SiteErrorLog', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.StandardCacheManagers', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.TemporaryFolder', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.Transience', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.ZCTextIndex', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.ZCatalog', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.ZODBMountPoint', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.ZSQLMethods', ispkg=True),
 ModuleInfo(module_finder=FileFinder('/home/zope/venv/lib/python3.12/site-packages/Products'), name='Products.mcdutils', ispkg=True)]
>>> # what the __path__ contains
>>> pprint(Products.__path__)
_NamespacePath(['/home/zope/Products', '/home/zope/venv/lib/python3.12/site-packages/Products', 'Products', '__editable__.ZMS-5.2.0.finder.__path_hook__'])

As far as I understand, the '__editable__.ZMS-5.2.0.finder.__path_hook__' entry that pip created there is not understood by the old code, but seems to work fine with the recommendation. At least that is my interpretation? I'd love to learn more here.

@d-maurer
Copy link
Contributor

d-maurer commented Nov 26, 2024 via email

@dwt
Copy link
Contributor Author

dwt commented Nov 26, 2024

This same behavior also reproducible when installing everything with uv, even when zms does not have a pyproject.toml. It seems uv already follows the new behavior that seems to be scheduled to be enforced by pip in the beginning of 2025

@dwt dwt changed the title Wrong way to enumerate Products? Zope needs to change the way it enumerate Products? Nov 26, 2024
@dataflake
Copy link
Member

I was reading through https://ichard26.github.io/blog/2024/11/whats-new-in-pip-24.3/#psa-legacy-editable-installs-are-deprecated and noticed that we are already enforcing one of the fixes suggested. All packages that are maintained using the scripts in https://github.com/zopefoundation/meta will already get a pyproject.toml with a valid build system declaration. So as far as I can see that specific detail is already covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants