pypa / get-pip

Helper scripts to install pip, in a Python installation that doesn't have it.
https://bootstrap.pypa.io/pip/
MIT License
755 stars 297 forks source link

Error out on any changes to files in the repository after generation #159

Closed pradyunsg closed 1 year ago

pradyunsg commented 2 years ago

This ensures that untracked files are correctly accounted for.

pradyunsg commented 2 years ago

x-ref #158, which should be rebased once this is merged.

pfmoore commented 2 years ago

To confirm, the point of this check is to ensure that the generate script doesn't (inadvertently or maliciously) add extra files to the public tree, as those will be published on bootstrap.pypa.io. Is that correct?

If that's true, then what's the intended workflow for a change that modifies generate.py? Should that change include the newly generated files? Presumably that means that they would be tracked as part of the PR, so CI would work as expected? Of course, that means that any change to the generation process has to publish the newly generated (or changed) files immediately, and cannot wait until a new pip release. Is that what we want?

If we want this level of validation, while still allowing for the possibility of merging changes to the generation before publishing the changed files, we probably need to make generate.py build in a staging area and only have the release process modify public (by copying the staging area). Then we can either hard-code the files we want to be published into the release script (so any new files will show up in a change to that script) or we simply rely on the fact that the release manager should be manually checking that the release is sound before pushing the "publish" button. My preferred option is the latter.

pradyunsg commented 2 years ago

If that's true, then what's the intended workflow for a change that modifies generate.py? Should that change include the newly generated files?

Yes.

Of course, that means that any change to the generation process has to publish the newly generated (or changed) files immediately, and cannot wait until a new pip release. Is that what we want?

We can make the generation conditional on the current pip version, like only generating the zipapp for pip>=22.3.

That would mean landing the PR with the new generation code deactivated at the time of writing. I’d actually prefer to publish it for the current version of pip and test it, while not advertising it and having a cautionary comment in there. It’s easier than having a staging environment and achieves most of the goals we have anyway.

pfmoore commented 2 years ago

I’d actually prefer to publish it for the current version of pip and test it, while not advertising it and having a cautionary comment in there.

Cool, I'm completely on board with that. So this PR needs pip.pyz and pip-22.2.2.pyz adding to it. What do you mean by "a cautionary comment" though? I can add a note to the readme in this repo, but it would need removing when we release 22.3.

I still need to put together some docs for pip announcing the new .pyz, so until those are in place (which will be part of 22.3) the new .pyz will be undocumented, and hence unsupported by default.

pradyunsg commented 2 years ago

Disregard the cautionary comment bit, I was thinking of the pyz as a regular text/Python file instead; which it obviously isn’t. :)

pradyunsg commented 1 year ago

Alrighty, what should we do with the versioned .pyz files @pfmoore?

I've tentatively removed the versioned .pyz files with the intention of landing this with them removex (i.e. removing them from bootstrap.pypa.io). We can later add them in a separate follow up (or pivot them to be providing on a per-Python version specific manner). Would that be OK with you?

pfmoore commented 1 year ago

I don’t know if people rely on the versioned pyz files. As long as the unversioned one is there for now, I think that’s ok. But I do think we should work out a solution so we can publish versioned ones (pip version, not python version). I’m afraid I still don’t understand the reasoning behind the way this check works, so I may be being naive, but in my view if the check is giving us issues publishing versioned pyz files, it’s the check that should change, not what we publish.

In practical terms, though, I concede that we have no evidence yet that real users care either way 🙂

pradyunsg commented 1 year ago

We might also want to add per-version get-pip.py files at that point, which... isn't the worst idea. :)