Skip to content

Implement compatibility with pip 25.3 - #2253

Merged
webknjaz merged 7 commits into
jazzband:mainfrom
shifqu:fix/remove-opt-pep517
Nov 11, 2025
Merged

Implement compatibility with pip 25.3#2253
webknjaz merged 7 commits into
jazzband:mainfrom
shifqu:fix/remove-opt-pep517

Conversation

@shifqu

@shifqu shifqu commented Oct 26, 2025

Copy link
Copy Markdown
Contributor

Removed use_pep517 reference to enable compatibility with pip 25.3+

Resolves #2252.

Contributor checklist
  • Included tests for the changes.
  • A change note is created in changelog.d/ (see changelog.d/README.md for instructions) or the PR text says "no changelog needed".
Maintainer checklist
  • If no changelog is needed, apply the skip-changelog label.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided A mark meaning that a new change log entry is present within the patch. label Oct 26, 2025
@shifqu

shifqu commented Oct 26, 2025

Copy link
Copy Markdown
Contributor Author

I don't think a test is required as it's a bugfix that depends on upstream pip. If I do need to include a test, say the word and I will give it a go.

@shifqu

shifqu commented Oct 26, 2025

Copy link
Copy Markdown
Contributor Author

closes #2252

Comment thread piptools/utils.py
@webknjaz webknjaz changed the title Fix/remove opt pep517 Implement compatibility with pip 25.3 Oct 27, 2025
Comment thread changelog.d/2252.bugfix.md Outdated

@webknjaz webknjaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for starting this effort!

Here's a few other things that are required to make this mergeable:

  • Bump 25.2 to 25.3 in the supported marker in tox.ini — so that we're actually testing this change in CI
  • Deprecate the --use-pep517, at least when pip is 25.3+ with a plan to drop in from the user-facing CLI later. Add a deprecation change note about this.
  • Have a regression test for the change or make sure that it is already covered elsewhere (having the CLI setting true/false); plus a test for the deprecation.

Comment thread changelog.d/2252.feature.md
Comment thread changelog.d/2252.feature.md
Comment thread piptools/resolver.py
Comment thread piptools/utils.py
Comment thread piptools/utils.py Outdated
@shifqu

shifqu commented Oct 27, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for starting this effort!

Here's a few other things that are required to make this mergeable:

  • Bump 25.2 to 25.3 in the supported marker in tox.ini — so that we're actually testing this change in CI
  • Deprecate the --use-pep517, at least when pip is 25.3+ with a plan to drop in from the user-facing CLI later. Add a deprecation change note about this.
  • Have a regression test for the change or make sure that it is already covered elsewhere (having the CLI setting true/false); plus a test for the deprecation.

I have bumped. Could you provide some guidance for the deprecation? The use-pep517 option was never explicitly supported and thus, is never actually mentioned.

I will check the tests whenever the check check has completed so we can see the report on it

@webknjaz

Copy link
Copy Markdown
Member
  • Deprecate the --use-pep517, at least when pip is 25.3+ with a plan to drop in from the user-facing CLI later. Add a deprecation change note about this.

Could you provide some guidance for the deprecation? The use-pep517 option was never explicitly supported and thus, is never actually mentioned.

@shifqu are you saying this is coming from --pip-args and not our own CLI parser? If that's the case, maybe we don't need a deprecation but should probably warn or error out when pip is incompatible. And we could mention the practical impact of this in the change note as well.

  • Have a regression test for the change or make sure that it is already covered elsewhere (having the CLI setting true/false); plus a test for the deprecation.
    I will check the tests whenever the check check has completed so we can see the report on it

I approved the CI run and it's now evident that a bunch of other tests are failing. Things like AttributeError("'InstallRequirement' object has no attribute 'global_options'") pop up in the logs, maybe more. So it looks like the scope of this PR will have to be expanded to cover those as well.

P.S. Note that if you're planning to stick around and get involved more, you can log in at https://jazzband.co to join the org. This will result in you not having to go through the CI approvals. Alternatively, if you get some other small PR merged, GH will stop requiring approvals in your following PRs too.

Comment thread piptools/resolver.py Outdated
@webknjaz webknjaz added this to the 7.5.1 milestone Oct 27, 2025
@webknjaz

Copy link
Copy Markdown
Member

@shifqu FYI you should be able to reproduce the failures locally with something like tox r -e pipsupported -qq.

@shifqu
shifqu force-pushed the fix/remove-opt-pep517 branch from c7cf286 to 0a5a68d Compare October 27, 2025 18:15
@sadikkuzu

This comment was marked as resolved.

@webknjaz webknjaz modified the milestones: 7.5.1, 7.5.2 Oct 27, 2025
@webknjaz

This comment was marked as resolved.

Comment thread piptools/utils.py Outdated
@shifqu

shifqu commented Oct 28, 2025

Copy link
Copy Markdown
Contributor Author
  • Deprecate the --use-pep517, at least when pip is 25.3+ with a plan to drop in from the user-facing CLI later. Add a deprecation change note about this.

Could you provide some guidance for the deprecation? The use-pep517 option was never explicitly supported and thus, is never actually mentioned.

@shifqu are you saying this is coming from --pip-args and not our own CLI parser? If that's the case, maybe we don't need a deprecation but should probably warn or error out when pip is incompatible. And we could mention the practical impact of this in the change note as well.

Indeed, this is coming from pip-args. I will check to see where exactly we can warn or error out.

  • Have a regression test for the change or make sure that it is already covered elsewhere (having the CLI setting true/false); plus a test for the deprecation.
    I will check the tests whenever the check check has completed so we can see the report on it

I approved the CI run and it's now evident that a bunch of other tests are failing. Things like AttributeError("'InstallRequirement' object has no attribute 'global_options'") pop up in the logs, maybe more. So it looks like the scope of this PR will have to be expanded to cover those as well.

P.S. Note that if you're planning to stick around and get involved more, you can log in at https://jazzband.co to join the org. This will result in you not having to go through the CI approvals. Alternatively, if you get some other small PR merged, GH will stop requiring approvals in your following PRs too.

Thanks, I have joined jazzband and ran the CI. Still some failures, but they are somewhat cryptic to me. The global_options one was very clear, but here it just says pip has failed. I am looking into it locally though, as I can reproduce there.

@webknjaz

Copy link
Copy Markdown
Member

Still some failures, but they are somewhat cryptic to me. The global_options one was very clear, but here it just says pip has failed. I am looking into it locally though, as I can reproduce there.

Yeah, I find it a bit annoying having to go through the Click testing layer when things fail. Here's how the CLI entrypoints are invoked: https://click.palletsprojects.com/en/stable/testing/ / https://click.palletsprojects.com/en/stable/api/#testing.

Sometimes, running a single test with breakpoint() ans/or pdb is helpful enough to get me through what's happening. I normally run something like tox r -e pipsupported -qq -- -n0 --no-cov -svvvvvv --pdb --full-trace --showlocals 'tests/test_cli_compile.py::test_locally_available_editable_package_is_not_archived_in_cache_dir[legacy]' to get me maximum visibility into things, and then poke around with the debugger.

@webknjaz

Copy link
Copy Markdown
Member

@shifqu I've extended the CI output in #2254. We can get it in if you approve the PR. And then, your updates here will get a lot more additional output upon rebase.

@shabie

This comment has been minimized.

@webknjaz

This comment has been minimized.

@shifqu
shifqu force-pushed the fix/remove-opt-pep517 branch from b172e9e to 4a6b1dc Compare October 29, 2025 20:14
@shifqu

shifqu commented Oct 29, 2025

Copy link
Copy Markdown
Contributor Author

So locally I have found out that for some tests the output is Could not find setuptools>=40.8.0. A quick search showed me that pip has an open issue for this: pypa/pip#12050. Do we have to rebuild our whl files or something?

@webknjaz

Copy link
Copy Markdown
Member

@shifqu not sure — do you have a log to post? Any isolated repro?

@shifqu

shifqu commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

@webknjaz yes -- I have rebased the rerun failed tests in this PR and the outputs show the same thing. For tests with legacy resolver the setuptools error is present, for backtracking resolver tests, it shows a more generic, less useful error.

FAILED tests/test_cli_compile.py::test_local_file_uri_package[legacy resolver-generate_hashes1-Local project with subdirectory] - AssertionError: assert 1 == 0
 +  where 1 = <Result <InstallationSubprocessError(reference='subprocess-exited-with-error', message='[green]installing build dependencies for small-fake-a[/] did not run successfully.\nexit code: 1', context=<text '[4 lines of output]\nLooking in links: /Users/runner/work/pip-tools/pip-tools/tests/test_data/minimal_wheels, /Users/runner/work/pip-tools/pip-tools/tests/test_data/minimal_wheels\n\x1b[31mERROR: Could not find a version that satisfies the requirement setuptools>=40.8.0 (from versions: none)\x1b[0m\x1b[31m\n\x1b[0m\x1b[31mERROR: No matching distribution found for setuptools>=40.8.0\x1b[0m\x1b[31m\n\x1b[0m\n[end of output]' [Span(0, 19, 'red'), Span(381, 396, 'red')] ''>, note_stmt='This error originates from a subprocess, and is likely not a problem with pip.', hint_stmt=None)>>.exit_code
FAILED tests/test_cli_compile.py::test_local_file_uri_package[backtracking resolver-generate_hashes0-Local project URI] - assert 1 == 0
 +  where 1 = <Result FailedToPrepareCandidate("Failed to build 'file:///Users/runner/work/pip-tools/pip-tools/tests/test_data/packages/small_fake_with_deps' when installing build dependencies")>.exit_code

To reproduce locally with pdb I used the same command you suggested:
tox r -e pipsupported -qq -- -n0 --no-cov -svvvvvv --pdb --full-trace --showlocals 'tests/test_cli_compile.py::test_local_file_uri_package[backtracking resolver-generate_hashes0-Local project URI]'
and
tox r -e pipsupported -qq -- -n0 --no-cov -svvvvvv --pdb --full-trace --showlocals 'tests/test_cli_compile.py::test_local_file_uri_package[legacy resolver-generate_hashes1-Local project with subdirectory]'

The commands are for the failing tests I pasted above. One for backtracking and one for legacy respectively.

Unfortunately I am unable to make much of these errors as I don't have a very good understanding of the tests (yet)

Edit:
I checked the out.output of the backtracking error and it gives the same as the legacy output.

(Pdb) out.output
'    error: subprocess-exited-with-error\n    \n    × installing build dependencies did not run successfully.\n    │ exit code: 1\n    ╰─> [3 lines of output]\n        Looking in links: redacted\\pip-tools\\tests\\test_data\\minimal_wheels, redacted\\pip-tools\\tests\\test_data\\minimal_wheels\n        ERROR: Could not find a version that satisfies the requirement setuptools>=40.8.0 (from versions: none)\n        ERROR: No matching distribution found for setuptools>=40.8.0\n        [end of output]\n    \n    note: This error originates from a subprocess, and is likely not a problem with pip.\n'

@webknjaz

Copy link
Copy Markdown
Member

@sirosen we could get #2268 in and then rebase, w/o blocking on the plugin/infra changes. This hopefully will get that single job a fraction of a percent to reach desired coverage.

Alternatively, we could go for dropping Python 3.8 before this PR (although, that would mean this change would land in/post a major release rather than a minor). I don't have strong opinions but these are the options that seem awailable rn.

@webknjaz

Copy link
Copy Markdown
Member

Oh wait.. It just occurred to me that we could set PYTEST_ADDOPTS in GHA since it's already being injected in tox.ini. And that would address the py vs. py38 mismatch, I think. Just need to do this conditionally.

@shifqu
shifqu force-pushed the fix/remove-opt-pep517 branch from ea4ab48 to a1918eb Compare November 11, 2025 12:12
@shifqu

shifqu commented Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

It just occurred to me that we could set PYTEST_ADDOPTS in GHA

After reading up on GHA environment variables, I added a conditional step that sets the options for 3.8. I hope it's how you had envisioned it. And I also hope the tests pass now :D

@shifqu

shifqu commented Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

Alright, the CI is (finally) green.

I wonder, since it's 51 commits, should I rework them into less, but more descriptive/targeted, commits?
Then, is there anything else that needs to be done for this PR to get merged?

cc @sirosen @webknjaz

@webknjaz

Copy link
Copy Markdown
Member

@shifqu I'll take a closer look later but meanwhile, combining related commits sounds lovely.

Comment thread .github/workflows/ci.yml
if: matrix.python-version == '3.8'
shell: bash
run: |
echo "PYTEST_ADDOPTS=--cov-fail-under=98" >> $GITHUB_ENV

@webknjaz webknjaz Nov 11, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a conditional step that sets the options for 3.8. I hope it's how you had envisioned it.

Not precisely what I had in mind but I guess it'll do. This sets the env var for the entire job (the rest of the steps). I originally thought that I'd scope it to one single step as follows:

env:
  PYTEST_ADDOPTS: >-
    ${{
      matrix.python-version == '3.8'
      && '--cov-fail-under=98'
      || ''
    }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had thought about it, but the syntax is more obscure (IMO) and now we effectively see in the gha report whether or not the pytest_addopts step was skipped or not.

I do understand that having it scoped would entirely remove the possibility of having conflicts in further steps. However, I don't think the pytest_addopts variable will cause conflicts any time soon.

In any case, I will let you decide, @webknjaz, would you like me to scope it to the single step or do I leave it like this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that with us thinking of dropping Python 3.8 soon after, we can keep it and not overthink — it'll be removed anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can keep it and not overthink — it'll be removed anyway.

cc @sirosen , here @webknjaz confirms he is ok with the solution.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shifqu these threads are difficult to dig up so I sometimes intentionally leave them unresolved for visibility. I'll mark this one as such.

shifqu and others added 6 commits November 11, 2025 17:33
This is done because pip 25.3 removed the pep517 options and uses this by default.
global_option and build_option is also removed from pip

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Tests using MINIMAL_WHEELS_PATH have also been updated to use this new session-scoped fixture

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
The user is warned when they pass any arguments that are no longer supported for their used pip-version.

These unsupported arguments will also be ignored by pip-tools

Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@shifqu
shifqu force-pushed the fix/remove-opt-pep517 branch from a1918eb to b7e8f9b Compare November 11, 2025 16:34
@shifqu

shifqu commented Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

combining related commits sounds lovely.

Reduced it to 6 commits, which is nicer for the commit history, hehe.

Comment thread tests/test_cli_compile.py Outdated
Comment thread tests/test_cli_compile.py Outdated
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>

@sirosen sirosen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shifqu, thank you so much for picking this body of work up and sticking with it through the issues that cropped up along the way!

There's only one thing I want to double-check (commented inline) before we merge. After that, I'll see what we want to add for v7.5.2 and what we feel can wait for a later version, and we'll ship the fix! 🚀

Comment thread .github/workflows/ci.yml
if: matrix.python-version == '3.8'
shell: bash
run: |
echo "PYTEST_ADDOPTS=--cov-fail-under=98" >> $GITHUB_ENV

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have all sorts of thoughts about improvements (including ones we're working on!), but I'm okay with this as a current solution that gets us unblocked.

@webknjaz, I want to confirm that you're also alright with this solution before we merge. For that reason I'm not yet adding this to the merge-queue.

@shifqu shifqu Nov 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sirosen, and a big thanks to you (and @webknjaz) for the reviews and patience exercised. What seemed like an easy fix did indeed crop up quite some issues along the way. But I am very happy to have been able to work on this PR. Quite some lessons learned wrt open source contributing! So thanks again.

Looking forward to seeing this be merged 🚀

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I actually said that it's fine but that thread go collapsed and isn't as visible.

FWIW, other people can't reset blocking reviews in PRs so I'll go ahead and change it to approved.

@webknjaz
webknjaz enabled auto-merge November 11, 2025 21:25

@webknjaz webknjaz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shifqu thanks for working on this with us so thoroughly!

@webknjaz
webknjaz added this pull request to the merge queue Nov 11, 2025
Merged via the queue into jazzband:main with commit d33539c Nov 11, 2025
52 checks passed
@webknjaz

Copy link
Copy Markdown
Member

v7.5.2 is out: https://github.com/jazzband/pip-tools/actions/runs/19286477040

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

Labels

bot:chronographer:provided A mark meaning that a new change log entry is present within the patch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pip-tools 7.5.1 is not compatible with pip 25.3

8 participants