pypa / pyproject-hooks

A low-level library for calling build-backends in `pyproject.toml`-based project
https://pyproject-hooks.readthedocs.io/
MIT License
122 stars 49 forks source link

Improve interop with `importlib.metadata` #195

Closed abravalheri closed 6 days ago

abravalheri commented 5 months ago

In #192, it is reported that backends not only need the ability to import code from backend-path but also depend on other features of importlib (e.g importlib.metadata).

This PR is an attempt to return that ability.

Closes #192

/cc @jaraco


This issue is a bit tricky...

From https://github.com/pypa/pip/issues/11812 it is clear that only manipulating sys.path is not enough to guarantee compliance with the following part of the spec:

The backend code MUST be loaded from one of the directories specified in backend-path

The alternative is maybe to do both? (i.e. manipulate sys.path and add the meta path finder that guarantees that the backed is loaded from the backend-path)? I can prepare a second PR for that if this is the best direction to go.

(But then there is also the risk that another MetaPathFinder that implements find_distributions can take precedence and load metadata from outside backend-path...).

takluyver commented 5 months ago

Thanks for looking into this promptly!

I find importlib really hard to make sense of - the different types of finders & loaders, optional methods, deprecated methods... it's too much to keep in my head at once. It sounds like you and @jaraco both reckon that adding this method ought to solve the issue, though. Have you been able to confirm whether it actually fixes things for setuptools' test suite? Or can that only be verified once there's a release on PyPI?

jaraco commented 5 months ago

I find importlib really hard to make sense of - the different types of finders & loaders, optional methods, deprecated methods... it's too much to keep in my head at once.

I sympathize. I too struggle with it, and I have a latent working model in my head 😬 . I've taken a stab at improving the docs in https://github.com/python/cpython/pull/113187. I should probably get that merged so at least there's a bit more context as imperfect as it is. I've done my best to keep "deprecated" interfaces and concepts encapsulated so the concerns are separate and can be largely ignored. You may find the importlib_metadata implementation to be fresher and potentially less encumbered by legacy concerns.

takluyver commented 5 months ago

Thanks @jaraco !

Have you been able to run the setuptools with this branch to verify that the problem is solved? Or is it something that can only be properly tested after a release?

jaraco commented 5 months ago

Have you been able to confirm whether it actually fixes things for setuptools' test suite? Or can that only be verified once there's a release on PyPI?

We should definitely do this verification. I can potentially help with this. At the very least, it should be possible to verify by putting a release on TestPyPI if necessary.

abravalheri commented 5 months ago

I find importlib really hard to make sense of - the different types of finders & loaders, optional methods, deprecated methods... it's too much to keep in my head at once.

I couldn't agree more 😅.

The complexity of importlib is also what causes the need for a MetaPathFinder. Simply prepending sys.path does not work to provide the guarantees the build-system spec needs because sys.meta_path has "higher precedence" than sys.path. But that is a whole can of worms...

Have you been able to confirm whether it actually fixes things for setuptools' test suite? Or can that only be verified once there's a release on PyPI?

We can probably verify that by pinning pyproject-hooks @ git+https://github.com/abravalheri/pyproject-hooks@issue-192 in tox.ini, I will try to have a look on this at some point this week.

abravalheri commented 5 months ago

We can probably verify that by pinning pyproject-hooks @ git+https://github.com/abravalheri/pyproject-hooks@issue-192 in tox.ini, I will try to have a look on this at some point this week.

@jaraco, it is not very trivial to reconciliate importlib.metadata/importlib_metadata with setuptools.extern.importlib_metadata.

I hitting the following "dependency-version-conflict" when testing on setuptools:

Command '['/tmp/build-env-aon6abmf/bin/python', '/home/abravalheri/workspace/setuptools/.tox/py/lib/python3.8/site-packages/pyproject_hooks/_in_process/_in_process.py', 'get_requires_for_build_wheel', '/tmp/tmpp401rn2h']' returned non-zero exit status 1.
Traceback (most recent call last):
  File "/home/abravalheri/workspace/setuptools/.tox/py/lib/python3.8/site-packages/pyproject_hooks/_in_process/_in_process.py", line 392, in <module>
    main()
  File "/home/abravalheri/workspace/setuptools/.tox/py/lib/python3.8/site-packages/pyproject_hooks/_in_process/_in_process.py", line 376, in main
    json_out["return_val"] = hook(**hook_input["kwargs"])
  File "/home/abravalheri/workspace/setuptools/.tox/py/lib/python3.8/site-packages/pyproject_hooks/_in_process/_in_process.py", line 146, in get_requires_for_build_wheel
    return hook(config_settings)
  File "/home/abravalheri/workspace/setuptools/setuptools/build_meta.py", line 325, in get_requires_for_build_wheel
    return self._get_build_requires(config_settings, requirements=['wheel'])
  File "/home/abravalheri/workspace/setuptools/setuptools/build_meta.py", line 295, in _get_build_requires
    self.run_setup()
  File "/home/abravalheri/workspace/setuptools/setuptools/build_meta.py", line 311, in run_setup
    exec(code, locals())
  File "<string>", line 93, in <module>
  File "/home/abravalheri/workspace/setuptools/setuptools/__init__.py", line 103, in setup
    _install_setup_requires(attrs)
  File "/home/abravalheri/workspace/setuptools/setuptools/__init__.py", line 74, in _install_setup_requires
    dist.parse_config_files(ignore_option_errors=True)
  File "/home/abravalheri/workspace/setuptools/setuptools/dist.py", line 625, in parse_config_files
    self._parse_config_files(filenames=inifiles)
  File "/home/abravalheri/workspace/setuptools/setuptools/dist.py", line 476, in _parse_config_files
    opt = self.warn_dash_deprecation(opt, section)
  File "/home/abravalheri/workspace/setuptools/setuptools/dist.py", line 513, in warn_dash_deprecation
    self._setuptools_commands(),
  File "/home/abravalheri/workspace/setuptools/setuptools/dist.py", line 538, in _setuptools_commands
    return metadata.distribution('setuptools').entry_points.names
AttributeError: 'list' object has no attribute 'names'

That happens because the vendored version of importlib_metadata in setuptools relies on APIs that are implemented recently, isn't it?

jaraco commented 5 months ago

That happens because the vendored version of importlib_metadata in setuptools relies on APIs that are implemented recently, isn't it?

Yes. Probably. It probably implies that this project may need to use importlib_metadata on even later versions of Python (for better forward compatibility), likely 3.9 and earlier.

I was worried about this when we were vendoring importlib_metadata. It's already worrisome enough to support importlib.metadata and importlib_metadata, but introducing _vendor.importlib_metadata adds another layer of complexity. Ideally, I'd like for setuptools to be able to rely on whatever version of "importlib metadata" is available in the environment.

abravalheri commented 5 months ago

In https://github.com/abravalheri/setuptools/pull/12 I show that this patch addresses the problem in setuptools (provided a small change for backwards compatibility in setuptools is considered).

@jaraco, I think that is a better approach then imposing a new dependency on importlib_metadata to pyproject-hooks.

pfmoore commented 5 months ago

+1 on avoiding the dependency here. I'm a fairly minor contributor to this project, so my opinion shouldn't carry too much weight, but I'm rather concerned that the lower levels of the packaging stack are starting to get overburdened with dependencies, both vendored and unvendored, and managing those dependencies is becoming an exercise in itself. Anything we can do to limit that issue is worthwhile, IMO.

ofek commented 3 months ago

What are the next steps for this to be merged?

abravalheri commented 3 months ago

What are the next steps for this to be merged?

The version without importlib_metadata (only importlib.metadata) https://github.com/pypa/pyproject-hooks/pull/199 is more likely to be merged according to @takluyver.

ofek commented 3 months ago

I missed that, thanks!