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

Installation error for package with un-normalized extra names in pip 24.1b1 #12688

Open
edgarrmondragon opened this issue May 8, 2024 · 10 comments · May be fixed by #12711
Open

Installation error for package with un-normalized extra names in pip 24.1b1 #12688

edgarrmondragon opened this issue May 8, 2024 · 10 comments · May be fixed by #12711
Labels
PEP implementation Involves some PEP !release blocker Hold a release until this is resolved state: needs reproducer Need to reproduce issue type: bug A confirmed bug or unintended behavior
Milestone

Comments

@edgarrmondragon
Copy link
Contributor

edgarrmondragon commented May 8, 2024

I think I found an issue with 24.1b1 with a scenario that works fine on 24.0.

Consider these requirements:

# requirements.txt
sqlalchemy==1.4.*

Create a virtualenv that uses pip 24.1b1:

VIRTUALENV_PIP=24.1b1 virtualenv .venv --python python3.8
source .venv/bin/activate

Install the requirements:

pip install -r requirements.txt

and it fails

Collecting sqlalchemy==1.4.* (from -r requirements.txt (line 1))
  Using cached SQLAlchemy-1.4.52.tar.gz (8.5 MB)
  Preparing metadata (setup.py) ... done
ERROR: Exception:
Traceback (most recent call last):
  File "/Users/me/repro/pip-24.1-sqlalchemy/.venv/lib/python3.8/site-packages/pip/_vendor/pkg_resources/__init__.py", line 2791, in requires
    deps.extend(dm[safe_extra(ext)])
KeyError: 'mariadb-connector'

Debug output:

pip install --no-cache-dir --debug -vvv -r requirements.txt > stdout.txt 2> stderr.txt

stdout.txt
stderr.txt

Originally posted by @edgarrmondragon in #12613 (comment)

@edgarrmondragon
Copy link
Contributor Author

Probably relevant (cc @hmc-cs-mdrissi):

I strongly suspect that error is related to hyphen/underscore normalization in extra names (mariadb-connector vs mariadb_connector) and pkg_resources vs packaging/pip having inconsistent normalization logic. I'd guess packaging upgrade may have touched extra normalization logic and I've seen inconsistencies across pkg_resources normalization/pip in past elsewhere as well with extras having hyphens.

edit: The last time I looked into this error my initial thought was fix seems best placed in pkg_resources itself and it doing lookups in normalized aware manner.

#12613 (comment)

@edgarrmondragon edgarrmondragon changed the title Installation error for package with un-normalized extra names Installation error for package with un-normalized extra names in pip 24.1b1 May 8, 2024
@hmc-cs-mdrissi
Copy link

pkg_resources comes from setuptools and this is key related issue, pypa/setuptools#3586.

The pip new packaging version may have made this more likely issue then before although I think fix probably should be in setuptools still.

@pradyunsg
Copy link
Member

Preparing metadata (setup.py) ... done

Huh, it looks like you have some global configuration instructing pip to not use PEP 517 style isolated builds and there's some statefulness involved as well.

The pip new packaging version may have made this more likely issue then before although I think fix probably should be in setuptools still.

Nah, setuptools isn't implicated here: we deal with the fact that pkg_resources isn't PEP 685 compliant yet via 5cc540b and related commits.

@pradyunsg
Copy link
Member

pradyunsg commented May 8, 2024

@edgarrmondragon Could you also run with -vvv in addition to --debug? It might be easier to post the output to https://gist.github.com/new and link to that.

@pradyunsg pradyunsg added type: bug A confirmed bug or unintended behavior !release blocker Hold a release until this is resolved state: needs reproducer Need to reproduce issue PEP implementation Involves some PEP labels May 8, 2024
@pradyunsg pradyunsg added this to the 24.1 milestone May 8, 2024
@edgarrmondragon
Copy link
Contributor Author

@edgarrmondragon Could you also run with -vvv in addition to --debug? It might be easier to post the output to https://gist.github.com/new and link to that.

@pradyunsg I've added the --debug -vvv output as files to the original comment. Let me know if a gist is still preferable.

@sbidoul
Copy link
Member

sbidoul commented May 19, 2024

This seems to arise out of some inconsistency in setuptools which generates Provides-Extra: mariadb-connector in PKG-INFO but mariadb_connector in requires.txt. And pkg_resources.safe_extra() is not converting one to the other as it preserves -. So 5cc540b is probably not doing what you think it does, @pradyunsg ?

@pradyunsg
Copy link
Member

Can we build a test for this failure? I've tried to reproduce this and been unsuccessful thus far.

@sbidoul
Copy link
Member

sbidoul commented May 19, 2024

@pradyunsg I could create such a test in #12709. Feel free to pick it.

@pradyunsg
Copy link
Member

pradyunsg commented May 19, 2024

OK, managed to reproduce it by using the sdist!

@sbidoul Thanks for that pointer! It turns out, the mapping that pkg_resources uses internally (for _dep_map) is based on requires.txt and the logic was loading the extra names from the METADATA file -- we can use an attribute on the Distribution object to get the keys of that mapping so that we aren't coupled/fragile against this failure now; or in the future.

@pradyunsg
Copy link
Member

Ah, I see 77533d8 (#12709) now! I'll add that into my PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PEP implementation Involves some PEP !release blocker Hold a release until this is resolved state: needs reproducer Need to reproduce issue type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants