[BUG] Incorrect wheel downloaded (wrong python version)

nishantvarma commented 2 months ago

setuptools version

setuptools=69.1.0

Python version

Python 3.10

OS

RHEL 8

Additional environment information

No response

Description

When I install a package using Buildout, Python 3.8 wheel is installed in a 3.10 environment.

Expected behavior

Python-version specific wheel is installed.

We have no distributions for pkg that satisfies 'pkg==7.3.0'.
root: Reading http://admin:pass@packages.org/pkg/
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.2-py38-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py310-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py3-none-any.whl
root: pkg==7.3.0 does not match pkg 7.3.2
root: pkg==7.3.0 does not match pkg 7.3.1
Getting distribution for 'pkg==7.3.0'.
Fetching pkg 7.3.0 from: http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
root: Downloading http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
Got pkg 7.3.0.

I think this issue is happening in setuptools. I am able to narrow it down to a problem here:

https://github.com/pypa/setuptools/blob/ac3fd97b55a6c3ea41a194b7af9dab07dfb6ff5a/setuptools/package_index.py#L513-L516

dist in requirement is satisfying for a 38 wheel:

>>> dist
pkg 7.3.0 (http://packages.org/pkg/pkg-7.3.0-py38-none-any.whl)
>>> requirement
Requirement.parse('pkg==7.3.0')
>>> dist in requirement
True # expected False

As self[requirement.key] contains all the packages in the server, I think the problem is in how dist in requirement works. requirement should capture the python version; check should fail for non-matching versions. I think it can be moved to setuptools?

Note: We create bytecode-wheels, so version is important. I will close this if this is something specific to Buildout.

How to Reproduce

I am using Buildout, so I am able to reproduce this with a config like this:

[buildout] 
parts = app 
index= ${pkgserver:url} 

[app] 
recipe = zc.recipe.egg 
eggs = pkg

[versions] 
pkg = 7.3.0

[pkgserver] 
url = http://${:username}:${:password}@${:hostname} 
hostname = packages.org 
username = admin 
password = pass

Ran via buildout -c test.cfg.

Output

We have no distributions for pkg that satisfies 'pkg==7.3.0'.
root: Reading http://admin:pass@packages.org/pkg/
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.2-py38-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py310-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
root: Found link: http://admin:pass@packages.org/pkg/pkg-7.3.0-py3-none-any.whl
root: pkg==7.3.0 does not match pkg 7.3.2
root: pkg==7.3.0 does not match pkg 7.3.1
Getting distribution for 'pkg==7.3.0'.
Fetching pkg 7.3.0 from: http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
root: Downloading http://admin:pass@packages.org/pkg/pkg-7.3.0-py38-none-any.whl
Got pkg 7.3.0.

abravalheri commented 2 months ago

Hi @nishantvarma, thank you very much for the report.

Could you please modify the reproducer to exclude the external software and focus only on setuptools? Buildout is not maintained by the same people as setuptools and it makes it more difficult to investigate.

nishantvarma commented 2 months ago

Thanks for the response @abravalheri. I was concerned about that too. I will try to reverse engineer what Buildout does and let you know. There is an alternative of creating a very simple Buildout environment using Docker; but I will try to do it the proper way itself.

abravalheri commented 2 months ago

Thanks for having a look on that @nishantvarma.

My first impressions, without the simplified reproducer, are:

The package_index.py file is quite old, so any change in it is subject to Hyrum's law and might break other people's code/workflow.
On a first glance the PackageIndex class seems to be working as a client for a PyPI-like index HTTP API. This could mean that someone can be using it for downloading distributions meant to be used with a different Python version... (Tools with installing capabilities nowadays can provision environments that are not coupled with the version of Python they run on themselves, e.g. pipx, tox, nox, etc...)

With that in mind, I would say that the safest approach here would be a backwards compatible change in the interface that allows users to opt-in for the extra filtering... But adding features to the API feels a bit wrong, because setuptools wants to get out of the business of working as a dependency resolver (and there is been a long lasting attempt of retiring easy_install/pkg_resources).

But I think that before delving into these conjectures we need a proper minimal reproducible example that removes the Buildout overhead, so that we can analyse the real root of the problem.

nishantvarma commented 2 months ago

I am able to reproduce this behaviour with the following snippet (it doesn't even need a package server to replicate!) :

from pkg_resources import Distribution, Requirement

dist = Distribution(
    project_name="pkg",
    version="1.0.0",
    location="http://packages.org/pkg/pkg-1.1.0-py38-none-any.whl"
)
requirement = Requirement.parse('pkg==1.0.0')

dist in requirement # True but False expected

Should it not be smarter, especially for bytecode-wheels ? Source code is available here.

abravalheri commented 2 months ago

Hi @nishantvarma, this particular code snippet seems to behave according to the docs:

__contains__(dist_or_version) Return true if dist_or_version fits the criteria for this requirement. If dist_or_version is a Distribution object, its project name must match the requirement’s project name, and its version must meet the requirement’s version criteria. If dist_or_version is a string, it is parsed using the parse_version() utility function. Otherwise, it is assumed to be an already-parsed version.

The Requirement object’s version specifiers (.specs) are internally sorted into ascending version order, and used to establish what ranges of versions are acceptable. Adjacent redundant conditions are effectively consolidated (e.g. ">1, >2" produces the same results as ">2", and "<2,<3" produces the same results as "<2"). "!=" versions are excised from the ranges they fall within. The version being tested for acceptability is then checked for membership in the resulting ranges.

This excerpt describes that the operation compares specifically name and version. So there seems to be a mismatch between expectation and what the docs are promising.

nishantvarma commented 2 months ago

Thanks for your help, @abravalheri, I will close this issue. Having py3 in the location doesn't seem to have any impact — but that's too much to ask for! I will see how pip etc., handles this. Will also check how C-Python packages are handled; there exact Python version is extremely crucial (similar to our problem).

pypa / setuptools