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

PLIP Replace pkg_resources in normal code #4126

Open
33 of 36 tasks
mauritsvanrees opened this issue Feb 26, 2025 · 7 comments
Open
33 of 36 tasks

PLIP Replace pkg_resources in normal code #4126

mauritsvanrees opened this issue Feb 26, 2025 · 7 comments

Comments

@mauritsvanrees
Copy link
Member

mauritsvanrees commented Feb 26, 2025

PLIP (Plone Improvement Proposal)

We already want to get rid of pkg_resources style namespaces, see issue #3928.

But pkg_resources in its entirety is basically deprecated, so we should stop calling it in "normal" Python code as well. In fact, some code already does not work anymore, but it depends on how you have installed Plone. See several comments by me starting here in a PR on CMFPlone.

Technically, this does not need to be a PLIP, and we do not need explicit improvement from any team, and this can all be done in a bugfix release of Plone. But I wanted to make it more visible and more easily findable. And this affects lots of packages.

Responsible Persons

Proposer: Maurits van Rees (@mauritsvanrees)

Seconder: Steve Piercy (@stevepiercy), Gil Forcada Codinachs (@gforcada)

Abstract

Go through all code used in Plone 6, including in Zope code. Main target is branches that are used in Plone 6.1 and 6.2. If we have time, fixing it in 6.0 would be welcome as well.

Motivation

Several uses of pkg_resources actually do not work anymore in Python 3.13. This was kind-of known, but I did not really notice any problems until now. But here it is.

In a checkout of buildout.coredev 6.2, start a Zope instance based on pip, so using the Makefile.

Very important: use Python 3.13. For example with export PRIMARY_PYTHON=python3.13. Or otherwise make sure that python3 resolves to python3.13.

Maybe first call make install twice, just to be sure, there is some stuff still to fix there. But technically, just one make command should be enough:

$ make zope-start
...
Traceback (most recent call last):
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/zope/configuration/xmlconfig.py", line 448, in endElementNS
    self.context.end()
    ~~~~~~~~~~~~~~~~^^
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/zope/configuration/config.py", line 748, in end
    self.stack.pop().finish()
    ~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/zope/configuration/config.py", line 918, in finish
    actions = self.handler(context, **args)
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/plone/autoinclude/zcml.py", line 37, in includePluginsDirective
    dists = loader.load_packages(target)
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/plone/autoinclude/loader.py", line 139, in load_packages
    z3c_dists = load_z3c_packages(target=target)
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/plone/autoinclude/loader.py", line 50, in load_z3c_packages
    dist = importlib.import_module(module_name)
  File "/Users/maurits/.pyenv/versions/3.13.1/lib/python3.13/importlib/__init__.py", line 88, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 1026, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/Users/maurits/community/plone-coredev/6.2/src/plone.app.upgrade/plone/app/upgrade/__init__.py", line 1, in <module>
    from plone.app.upgrade.utils import alias_module
  File "/Users/maurits/community/plone-coredev/6.2/src/plone.app.upgrade/plone/app/upgrade/utils.py", line 28, in <module>
    plone_version = pkg_resources.get_distribution("Products.CMFPlone").version
                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/pkg_resources/__init__.py", line 529, in get_distribution
    dist = get_provider(dist)
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/pkg_resources/__init__.py", line 412, in get_provider
    return working_set.find(moduleOrReq) or require(str(moduleOrReq))[0]
                                            ~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/pkg_resources/__init__.py", line 1065, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/pkg_resources/__init__.py", line 892, in resolve
    dist = self._resolve_dist(
        req, best, replace_conflicting, env, installer, required_by, to_activate
    )
  File "/Users/maurits/community/plone-coredev/6.2/.venv/lib/python3.13/site-packages/pkg_resources/__init__.py", line 933, in _resolve_dist
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'Products.CMFPlone' distribution was not found and is required by the application

What is happening here? With Python 3.13, in a pip-only install, and Products.CMFPlone in editable install, you cannot even start a Zope instance.

Similarly, running tests then also fails, see my comments that I link above.

BTW, good old Buildout is running fine, wondering what the fuzz is about, though it has its own problems. ;-)

There also seems to be a problem with entry points, probably caused by the setuptools change, resulting in an error during install:

error msg: TypeError: ('Expected str, Requirement, or Distribution', None)

See for example plone/buildout.coredev#990

Assumptions

We want to be able to run Plone with Python 3.13 in a pip-only environment, so without Buildout, and use editable install for some packages.

Proposal & Implementation

Go through all used packages and replace any usage of pkg_resources with something else. Ignore the namespaces, that is a different topic.

Some links:

Note that we don't need the back ports packages (those with underscores), but we can use the standard lib: importlib.metadata and importlib.resources.

Let's gather some code samples here:

- __version__ = pkg_resources.require("Products.CMFPlone")[0].version
+ from importlib.metadata import version
+ __version__ = version("Products.CMFPlone")

Another one:

+from importlib.metadata import distribution
+from importlib.metadata import PackageNotFoundError
-import pkg_resources

 try:
-    pkg_resources.get_distribution("Products.CMFPlacefulWorkflow")
+    distribution("Products.CMFPlacefulWorkflow")
-except pkg_resources.DistributionNotFound:
+except PackageNotFoundError:
    ...

And files as well:

+from importlib.resources import files
-import pkg_resources

-TEST_IMAGE = pkg_resources.resource_filename("plone.app.caching.tests", "test.gif")
+TEST_IMAGE = str(files("plone.app.caching") / "tests" / "test.gif")

Tip

Note that importlib.resources.files returns a pathlib.Path, but most of the times our code expects a string, that's why it is converted to it already

Version parsing:

-from pkg_resources import parse_version
+from packaging import version
 
-HAS_PLONE5 = parse_version(env.plone_version()) >= parse_version("5.0b2")
+HAS_PLONE5 = version.parse(env.plone_version()) >= version.parse("5.0b2")

Deliverables

Grepping in 6.2 code, I see at least these:

For the entry points problem, maybe some of it will be fixed when all of the above have been handled. But it may need some fixes in other places. Some issues/PRs:

Risks

If we find the correct replacements, this should have no adverse effects.

Participants

@stevepiercy
Copy link
Contributor

I second and will participate. Old packages that use Sphinx to build their docs might need to implement this fix in their conf.py:

https://github.com/plone/plone.api/pull/566/files

@gforcada
Copy link
Member

@mauritsvanrees you can add myself as seconder if you need someone :)

It would be nice to have a flake8 plugin for that, so we do not get regressions.

ale-rt added a commit to plone/plone.autoinclude that referenced this issue Feb 26, 2025
ale-rt added a commit to plone/plone.autoinclude that referenced this issue Feb 26, 2025
ale-rt added a commit to plone/plone.autoinclude that referenced this issue Feb 26, 2025
Forla03 pushed a commit to Forla03/plone.autoinclude that referenced this issue Feb 27, 2025
@jensens
Copy link
Member

jensens commented Feb 28, 2025

I support this PLIP.

@yurj
Copy link
Contributor

yurj commented Feb 28, 2025

Are you planning to convert code in place or have some utils functions that wraps up the calls to importlib? For example:

- __version__ = pkg_resources.require("Products.CMFPlone")[0].version
+ __version__ = plone.pgkutils.getversion("Products.CMFPlone")
+ from importlib.metadata import version
+ def getversion(packagename):
+
+    return version(packagename)

This thin wrapper can help in the future? Maybe there will be other changes in the packaging world in the future. Moreover, could be this wrap work an help to the future work on the namespace problem?

@mauritsvanrees
Copy link
Member Author

If we have multiple lines that belong together and are used often, then a wrapper function could help. But for 99% of the cases I could prefer to simply call importlib and friends directly.

@jensens
Copy link
Member

jensens commented Mar 1, 2025

Are you planning to convert code in place or have some utils functions that wraps up the calls to importlib?

I do not think one layer more is needed here. importlib in Python is stable, it in the standard library and not so part of an add-on.

mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Mar 1, 2025
Branch: refs/heads/master
Date: 2025-02-28T20:02:34+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@0a3497d

fix: replace pkg_resources with importlib

See plone/Products.CMFPlone#4126

Files changed:
M plone/app/contentmenu/menu.py
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-02-28T20:02:35+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@eeeec82

Add news entry

Files changed:
A news/4126.bugfix
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-03-01T22:14:11+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@e563170

Merge pull request #75 from plone/4126-replace-pkg-resources

Replace pkg_resources

Files changed:
A news/4126.bugfix
M plone/app/contentmenu/menu.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Mar 1, 2025
Branch: refs/heads/master
Date: 2025-02-28T20:02:34+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@0a3497d

fix: replace pkg_resources with importlib

See plone/Products.CMFPlone#4126

Files changed:
M plone/app/contentmenu/menu.py
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-02-28T20:02:35+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@eeeec82

Add news entry

Files changed:
A news/4126.bugfix
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-03-01T22:14:11+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@e563170

Merge pull request #75 from plone/4126-replace-pkg-resources

Replace pkg_resources

Files changed:
A news/4126.bugfix
M plone/app/contentmenu/menu.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Mar 1, 2025
Branch: refs/heads/master
Date: 2025-02-28T20:02:34+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@0a3497d

fix: replace pkg_resources with importlib

See plone/Products.CMFPlone#4126

Files changed:
M plone/app/contentmenu/menu.py
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-02-28T20:02:35+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@eeeec82

Add news entry

Files changed:
A news/4126.bugfix
Repository: plone.app.contentmenu

Branch: refs/heads/master
Date: 2025-03-01T22:14:11+01:00
Author: Gil Forcada Codinachs (gforcada) <[email protected]>
Commit: plone/plone.app.contentmenu@e563170

Merge pull request #75 from plone/4126-replace-pkg-resources

Replace pkg_resources

Files changed:
A news/4126.bugfix
M plone/app/contentmenu/menu.py
@gforcada
Copy link
Member

gforcada commented Mar 3, 2025

  • all PRs are done
  • plenty need to be merged
  • some have test failures that seem related to CMFPlone still using pkg_resources (as it has not been released)
  • some test failures are harder to investigate (like on plone.autoinclude) 😓

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

No branches or pull requests

5 participants