Skip to content

Fix new dependency-groups feature to use the stdlib tomllib where possible #13356

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
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eli-schwartz
Copy link
Contributor

Previously, commit 88c9f31 modified pip to use the stdlib on versions of python where this module is in the stdlib. As justified there:

Although a tomli copy is vendored, doing this conditional import allows:

  • automatically upgrading the code, when the time comes to drop py3.10 support

  • slightly simplifying debundling support, as it's no longer necessary to depend on a tomli(-wheel)? package on sufficiently newer versions of python.

#13065 added a new feature, including a vendored "dependency_groups" library that likewise supports using the stdlib tomllib via dependency_groups/_toml_compat.py. But the code in pip itself to use dependency_groups manually loads pyproject.toml and passes it to dependency_groups, and fails to use the same compatibility dispatch as both the pre-existing pip code and dependency_groups itself.

Add back the conditional logic.

…sible

Previously, commit 88c9f31 modified pip
to use the stdlib on versions of python where this module is in the
stdlib. As justified there:

Although a tomli copy is vendored, doing this conditional import allows:
- automatically upgrading the code, when the time comes to drop py3.10
  support

- slightly simplifying debundling support, as it's no longer necessary
  to depend on a tomli(-wheel)? package on sufficiently newer versions
  of python.

pypa#13065 added a new feature, including a
vendored "dependency_groups" library that likewise supports using the
stdlib tomllib via `dependency_groups/_toml_compat.py`. But the code in
pip itself to use dependency_groups manually loads pyproject.toml and
passes it to dependency_groups, and fails to use the same compatibility
dispatch as both the pre-existing pip code and dependency_groups itself.

Add back the conditional logic.
@pfmoore
Copy link
Member

pfmoore commented Apr 28, 2025

Please hold off on merging this. I don't feel that it counts as a bug fix, and I want to avoid introducing extra changes into a potential 25.1.1 release, if needed.

I'm happy to review whether this should be included if we need a 25.1.1, but let's not pre-judge that question for now.

@NHOrus
Copy link

NHOrus commented Apr 28, 2025

It caused a problem for me
https://bugs.gentoo.org/955029

python3.13 -m pip install --user --upgrade pip
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/usr/lib/python3.13/site-packages/pip/__main__.py", line 24, in <module>
    sys.exit(_main())
             ~~~~~^^
  File "/usr/lib/python3.13/site-packages/pip/_internal/cli/main.py", line 77, in main
    command = create_command(cmd_name, isolated=("--isolated" in cmd_args))
  File "/usr/lib/python3.13/site-packages/pip/_internal/commands/__init__.py", line 119, in create_command
    module = importlib.import_module(module_path)
  File "/usr/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 "/usr/lib/python3.13/site-packages/pip/_internal/commands/install.py", line 24, in <module>
    from pip._internal.cli.req_command import (
    ...<2 lines>...
    )
  File "/usr/lib/python3.13/site-packages/pip/_internal/cli/req_command.py", line 31, in <module>
    from pip._internal.req.req_dependency_group import parse_dependency_groups
  File "/usr/lib/python3.13/site-packages/pip/_internal/req/req_dependency_group.py", line 3, in <module>
    import tomli
ModuleNotFoundError: No module named 'tomli'

@eli-schwartz
Copy link
Contributor Author

_____________________ test_basic_install_editable_from_git _____________________
[...]

E               tests.lib.TestFailure: expected egg link file missing: [PosixPath('venv/lib/python3.11/site-packages/testpackage.egg-link'), PosixPath('venv/lib/python3.11/site-packages/testpackage.egg-link')]

I don't understand this test failure, it doesn't seem possible to have happened because of the changes in this PR.

It is also reproducible in this scheduled cron job: https://github.com/pypa/pip/actions/runs/14697699388

@sbidoul
Copy link
Member

sbidoul commented Apr 28, 2025

I don't understand this test failure, it doesn't seem possible to have happened because of the changes in this PR.

Maybe due to setuptools 80, released yesterday, which changed editable installs.

@eli-schwartz
Copy link
Contributor Author

Until I hear otherwise, I'll assume my PR is entirely fine and passes all tests. They pass in my local environment to my satisfaction, though it's not the same as the CI environment so I cannot assert any guarantees.

@sbidoul
Copy link
Member

sbidoul commented Apr 28, 2025

Attempt to make tests green again in #13357

@notatallshaw
Copy link
Member

@NHOrus that looks like a bug with the Gentoo distribution of pip, if you think there is a bug with source pip or are asking to make a change based on helping distributions out please make a new issue and we can discuss there.

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

Successfully merging this pull request may close these issues.

5 participants