Skip to content

Issue with new find_distributions and incorrect metadata #211

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

Open
kulikjak opened this issue Apr 8, 2025 · 5 comments · May be fixed by #212
Open

Issue with new find_distributions and incorrect metadata #211

kulikjak opened this issue Apr 8, 2025 · 5 comments · May be fixed by #212

Comments

@kulikjak
Copy link

kulikjak commented Apr 8, 2025

After updating to pyproject_hooks 1.2.0, I am getting the following error when building setuptools_scm:

> /usr/bin/python3.9 -m build -n
* Getting build dependencies for sdist...
/scratch/userland-gate/components/python/setuptools_scm/setuptools_scm-8.1.0/src/setuptools_scm/_integration/setuptools.py:31: RuntimeWarning:
ERROR: setuptools==8.1.0 is used in combination with setuptools_scm>=8.x

even though my setuptools version is 73.0.1. The build process erroneously takes the setuptools_scm version and considers it the setuptools one.

(while this says ERROR, the build continues and finishes successfully; the only unexpected result is that the WHEEL file lists Generator as setuptools (8.1.0), which is not the case).

I also noticed similar issue with Python incremental.

I know very little about how the whole build machinery works, but I looked a little into this and found that the def find_distributions addition in 1.2.0 is the reason for this. It seems that importlib_metadata (which is used by setuptools to find its own __version__) uses _BackendPathFinder and the new find_distributions, but the passed context (which is correctly set to look for "setuptools") is ignored and replaced with new context, which is no longer correct for that case.

Updating the function with a check:

if context is None:
    context = DistributionFinder.Context(path=self.backend_path)

fixes the issue, but I have no idea whether that is a good idea - it was just a test.

This happens in 3.9.21 (with importlib_metadata 7.1.0), 3.11.11 and 3.13.1.

Also, both setuptools_scm and incremental use custom build backend and "backend-path" (with more than just ".").

@kulikjak
Copy link
Author

kulikjak commented Apr 8, 2025

I just noticed that the context overwriting was considered when initially integrated in #195. I am not sure about "My intuition is that we can simply discard the given context, and other finders will handle it." though because, at least in my case, the _BackendPathFinder is the first one on the list with find_distributions method and hence any further finders are ignored.

Also, here are the installed packages I used to test this.

build-1.0.3
importlib_metadata-7.1.0
packaging-24.1
pyproject_hooks-1.2.0
setuptools-73.0.1
tomli-2.1.0
wheel-0.42.0
zipp-3.19.2

I also tried newer setuptools and importlib_metadata, but nothing changed.

@abravalheri
Copy link
Contributor

abravalheri commented Apr 9, 2025

hence any further finders are ignored.

Hi @kulikjak, in importlib_metadata find_distributions, all instances with find_distributions are used.

The same is also valid for importlib.metadata in Python3.9.21:

So the reasoning behind the discussion in #195 is justified by the fact that _BackendPathFinder will only be able find the packages in backend_path. When the package is not in the backend_path, importlib.metadata will proceed with the other finders and the result will be the same as if _BackendPathFinder had never existed.

The fact that setuptools (8.1.0) is being found looks weird to me. Is it being found by _BackendPathFinder? If so, how v8.1.0 ended up in the backend_path? If not (which looks more likely1), then there seems to be a different finder that finds an old version of setuptools, and that has more priority that the one you installed manually... So which other finder would be that? Why does it have more priority?


Other than that, there seems to be 2 problems with versions of dependencies:

All things considered, when the dependency versions are fixed, things seem to be working well in a controlled environment, see the minimum reproducible example below:

> docker run --rm -it python:3.9-bookworm /bin/bash

cat <<EOF > /tmp/environment.txt
build==1.0.3
importlib_metadata==7.1.0
packaging==24.1
pyproject_hooks==1.2.0
setuptools==75.2.0  # -- Updated due to race condition bug
tomli==2.0.2  # -------- Downgraded due to setuptools-scm dependency constraint
wheel==0.42.0
zipp==3.19.2
EOF

python3 -m venv /tmp/venv
/tmp/venv/bin/python -m pip install -r /tmp/environment.txt

git clone https://github.com/pypa/setuptools-scm /tmp/setuptools-scm
cd /tmp/setuptools-scm
git fetch --tags
/tmp/venv/bin/python -m build --no-isolation
# ...
# Successfully built setuptools_scm-8.2.2.dev3+g8e11309.tar.gz and setuptools_scm-8.2.2.dev3+g8e11309-py3-none-any.whl

Footnotes

  1. since your suggestion with if context is None seems make _BackendPathFinder start to find something.

@kulikjak
Copy link
Author

kulikjak commented Apr 9, 2025

Hi @abravalheri, thanks for looking in this!

Hi @kulikjak, in importlib_metadata find_distributions, all instances with find_distributions are used (not ignored).

Ah, seems like I've got a little confused. But I still think it doesn't work as expected.

Here is the call stack when looking for the setuptools version:
def version -> def distribution -> Distribution.from_name -> def discover -> def _discover_resolvers

_discover_resolvers returns all resolvers - _BackendPathFinder.find_distributions and the standard MetadataPathFinder.find_distributions methods.

Then, in def discover, each of these methods is called with context (created with name=name - in this case "setuptools"). _BackendPathFinder.find_distributions creates a new context (ignoring the name=name) with path=self.backend_path. It's no longer looking for "setuptools", it's (I am guessing here) instead looking for any package in self.backend_path. It finds the setuptools_scm there, and returns it. The standard MetadataPathFinder.find_distributions is then called and finds the "setuptools" we are looking for; however, it's only second in the iterable def discover returns, and the from_name method simply returns the first one:

return next(iter(cls.discover(name=name)))

The importlib.metadata version looks a little different but seems to do the same thing.

I think that find_distributions should return an empty iterable in this case - returning [] when context is set seems to fix the issue. Though I guess that an even better solution would be to add path=self.backend_path to the original Context, keeping all the original arguments.


Other than that, there seems to be 2 problems with versions of dependencies:

Thanks for the catch - unfortunately, even with these two changes, nothing changes.

I am pretty confused why you are not seeing the same issues as I do, though....

@abravalheri
Copy link
Contributor

abravalheri commented Apr 9, 2025

Then, in def discover, each of these methods is called with context (created with name=name - in this case "setuptools"). _BackendPathFinder.find_distributions creates a new context (ignoring the name=name) with path=self.backend_path. It's no longer looking for "setuptools", it's (I am guessing here) instead looking for any package in self.backend_path. It finds the setuptools_scm there, and returns it. The standard MetadataPathFinder.find_distributions is then called and finds the "setuptools" we are looking for; however, it's only second in the iterable def discover returns, and the from_name method simply returns the first one:

return next(iter(cls.discover(name=name)))
The importlib.metadata version looks a little different but seems to do the same thing.

Ah, OK, I see. This makes sense to me! Thank you very much for the detailed explanation.

Though I guess that an even better solution would be to add path=self.backend_path to the original Context, keeping all the original arguments.

And the suggested change makes sense as well.

I think this functionality is not something that is very clear in the importlib_metadata docs. I think your explanation brings light to the way it works.

@kulikjak kulikjak linked a pull request Apr 10, 2025 that will close this issue
@kulikjak
Copy link
Author

kulikjak commented Apr 10, 2025

Thanks! I created #212 to address this.

I think this functionality is not something that is very clear in the importlib_metadata docs.

I agree. I had to test it with debug prints to find out what was happening.

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.

2 participants