Skip to content

PEP 751 experimental pip lock command #13213

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

Merged
merged 11 commits into from
Apr 16, 2025
Merged

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Feb 8, 2025

This is a rough PoC for a pip lock command that writes PEP 751 compliant toml to stdout.

Use with:

  • pip --quiet lock -o - . to lock the local project to stdout.
  • pip lock -e . to lock the local project in editable mode to pylock.toml

I wrote this mainly as a way to better understand the PEP.

A few comments:

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch 2 times, most recently from 0b02620 to 914bd0b Compare February 8, 2025 19:31
@potiuk
Copy link
Contributor

potiuk commented Feb 8, 2025

Nice. I would love to help with making it happen (of course it depends on PEP 751 to be approved). We are currently doing our own custom and pretty "poor" implementation of locks with cosntraints in Airlfow and having a "standard" way of reproducible installs would be a great thing.

Not sure what is the status now of the PEP and whether it has a chance to be approved soon, but that looks really great that we are trying to standardise it now.

@notatallshaw
Copy link
Member

notatallshaw commented Feb 8, 2025

I appreciate this is draft, but some small early feedback.

Firstly, I am advocating for an "output file" option. Writing pip's output to a file in other commands has several footguns, e.g knowing the correct flags to turn off non-relevant output, determining if pip hit errors or warnings when you do, and writing non-UTF-8 files when you direct standard out from powershell on Windows.

pip-compile (and uv pip compile) solve this by having an output file option which is considered best practice to use. That said, I don't know the history of why other commands don't offer this, perhaps it's non-trivial.

I removed the platform/abi/implementation/python-version options due to their limitations

I agree, I think pip should start as limited as possible around known difficult issues, and expand from there. This will give the chance for the lock API to evolve, if able.

@pfmoore
Copy link
Member

pfmoore commented Feb 8, 2025

Cool. I did some early PoC work on the PEP, but things have evolved drastically since then, and I never got back to it. But one of my goals was always for the pip installation report format to be easily convertible into a lockfile. Another thing I'd like to see pip implement (I may well get round to this myself, in due course, it's not a request for someone else to pick it up!) is pip freeze --format=lockfile. I'd hope that would be relatively easy to implement.

I'd very definitely aim pip at solely producing "environment reproduction" lockfiles, and not cross-platform ones. To that end, I think that disallowing the --platform etc., arguments1 is the right call.

I don't have a strong opinion on the question of an --output-file option, but I think that defaulting to writing to stdout is a reasonable place to start. With an output file option, we'd have to consider validating the provided filename to match the standard-required form pylock[.xxx].toml, which IMO is a distraction from the main exercise of ensuring that we produce a correct lockfile.

Not sure what is the status now of the PEP and whether it has a chance to be approved soon

The PEP is in its final stages before being submitted for pronouncement. At this point, it's extremely unlikely that any major changes will occur. Speaking as the PEP-delegate, I can say that I've been heavily involved in the discussions on the PEP, and I'm confident that it has a good chance of being accepted2.

Footnotes

  1. Or, as you say, putting them under a "use at your own risk" warning.

  2. I'm not giving anything away here - I've always been supportive of the idea of a lockfile standard, but the problem has been getting community consensus on what it should look like. At this point, we're the closest we've ever been to consensus.

@sbidoul
Copy link
Member Author

sbidoul commented Feb 8, 2025

pip freeze --format=lockfile

I think a pre-requisite for that is PEP 710

@notatallshaw
Copy link
Member

notatallshaw commented Feb 8, 2025

I don't have a strong opinion on the question of an --output-file option, but I think that defaulting to writing to stdout is a reasonable place to start. With an output file option, we'd have to consider validating the provided filename to match the standard-required form pylock[.xxx].toml, which IMO is a distraction from the main exercise of ensuring that we produce a correct lockfile.

I'm fine with this being in a follow up PR if there are non-trivial questions to answer.

Though looking now at the PEP I do think a pedantic reading of it would be that the data should only ever be written as a file and not outputted as stdout as that would technically violate those same file name specifications 😉. (If not made clear by emoji I think that's terrible and that stdout should absolutely be an option at least).

@pfmoore
Copy link
Member

pfmoore commented Feb 8, 2025

🙂 I wasn't trying to be pedantic, just noting the likelihood that someone would ask the question about validating the filename. Getting the encoding correct by using the --output-file option is a much more important point, though.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch from 914bd0b to a29a039 Compare February 9, 2025 10:20
@sbidoul
Copy link
Member Author

sbidoul commented Feb 9, 2025

I choose not to emit version for direct URL packages, as the version could by dynamic. This is to avoid problems with projects using setuptools-scm, where the version possibly changes with each commit and committing the lock file would change the version.

Also, I choose to emit a relative path for directory packages, so the common use case (namely pip lock .) produces something that is portable.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch 3 times, most recently from d124afc to 4b4f84a Compare February 9, 2025 11:18
@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch 6 times, most recently from ef393e6 to f77d21c Compare February 9, 2025 18:06
@sbidoul
Copy link
Member Author

sbidoul commented Apr 1, 2025

Now that PEP 751 is accepted, let's see if we want to move forward with this pip lock command.

A few questions I have:

  • Is this CLI good as it is now ?
  • Do we want to emit environments ? The only thing that can be done easily is to emit the full current environment, but that is of course way too restrictive in most cases if installers would refuse to install if the installation environment does not match the lock environment. I'm enclined to leave it out in a first iteration.
  • The output format could be optimized by inlining some tables. Do we want to improve that ? tomli-w does not support that. tomlkit could be an option but it is heavier and harder to use leading to less readable code. Is there another option? We could also postpone inlining and see if this is actually a problem in practice.

@pfmoore
Copy link
Member

pfmoore commented Apr 1, 2025

+1 from me. I think the CLI is good (we can make changes later if needed). I agree, let's not emit environments for now at least.

Let's not worry about inlining for now. Layout is an area where there are a number of possible options, and I imagine tools will ultimately converge on an approach that provides good readability/auditability and easy generation. It's possible that someone could even produce a dedicated lockfile writer module. I'd rather we stuck with something simple, and with a smaller new library to vendor, until we have a better feel for where the community is going here.

@sbidoul
Copy link
Member Author

sbidoul commented Apr 1, 2025

Good, I'll see if I can add some tests in time for 25.1.

Except for the absence of tests (and docs?), this is ready to review.

@sbidoul sbidoul marked this pull request as ready for review April 1, 2025 10:04
@pfmoore pfmoore added this to the 25.1 milestone Apr 1, 2025
@pfmoore
Copy link
Member

pfmoore commented Apr 1, 2025

I've added the 25.1 milestone, but if you don't have the time (we're about 2 weeks away from the release) feel free to reassign to 25.2 or just remove the milestone.

@sbidoul sbidoul changed the title [PoC] PEP 751 pip lock command PEP 751 pip lock command Apr 1, 2025
@sbidoul sbidoul mentioned this pull request Apr 1, 2025
1 task
@ichard26
Copy link
Member

ichard26 commented Apr 1, 2025

I haven't reviewed the PR at all, but if we're going to land this feature for pip 25.1, would it be beneficial to mark this new command as experimental like pip index version? As I said, I haven't reviewed the PR so maybe this command is already well-designed, but I'd rather be honest and give ourselves some breathing room as the ecosystem plays with the new standard. Despite the multi-year discussion, I don't think we'll be able to nail the UI side of lockfiles on our first try.

@uranusjr
Copy link
Member

uranusjr commented Apr 1, 2025

+1 to marking this as experiemental.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch from 29a4564 to 748f3de Compare April 12, 2025 11:27
@sbidoul
Copy link
Member Author

sbidoul commented Apr 12, 2025

I documented that the lock file is single platform, and added a first basic test.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch 4 times, most recently from ddd72c0 to 0ad694a Compare April 13, 2025 09:30
@sbidoul
Copy link
Member Author

sbidoul commented Apr 13, 2025

There is now decent test coverage. This should be good to go.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch from ec2f8fd to 685f56a Compare April 15, 2025 08:00
@sbidoul
Copy link
Member Author

sbidoul commented Apr 15, 2025

Thanks a lot for the review @uranusjr.

I rebased, fixed the news entry, and added an attional Path.resolve() in the relative path computation.

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Got a minute to read through the diff. These are mostly nitpick, but I think the long option concern should probably be addressed before merge.

Examples
========

#. Emit a ``pylock.toml`` for the the project in the current directory
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
#. Emit a ``pylock.toml`` for the the project in the current directory
#. Emit a :file:`pylock.toml` for the the project in the current directory

Copy link
Member Author

Choose a reason for hiding this comment

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

@webknjaz we don't seem to have used that role in the pip documentation so far, so I'll leave that out for now. We can consider a separate PR to do that on a broader level.

Copy link
Member

Choose a reason for hiding this comment

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

Ack. FTR, Furo currently renders this role the same as regular inline code. Here's an example: https://packaging.python.org/en/latest/specifications/pyproject-toml/#pyproject-toml-license.

@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch from 685f56a to 0d69c52 Compare April 15, 2025 22:02
sbidoul added 10 commits April 16, 2025 11:10
It could be dynamic, so not emitting it is a better default. In the future we could consider emitting it when we know it is not dynamic.
This option of relative_to appeared in Python 3.12.
I estimate supporting this is not worth the additional complexity
at the moment.
@sbidoul sbidoul force-pushed the pip-lock-pep751-sbi branch from 0d69c52 to 5c7025d Compare April 16, 2025 09:14
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Looks ready.

@sbidoul
Copy link
Member Author

sbidoul commented Apr 16, 2025

Thanks for the review everyone!

@sbidoul sbidoul merged commit 122692d into pypa:main Apr 16, 2025
29 checks passed
@sbidoul sbidoul deleted the pip-lock-pep751-sbi branch April 16, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided C: dependency resolution About choosing which dependencies to install PEP implementation Involves some PEP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants