get-pip icon indicating copy to clipboard operation
get-pip copied to clipboard

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

Open pradyunsg opened this issue 2 years ago • 5 comments

This ensures that untracked files are correctly accounted for.

pradyunsg avatar Jul 15 '22 13:07 pradyunsg

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

pradyunsg avatar Jul 15 '22 13:07 pradyunsg

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.

pfmoore avatar Sep 03 '22 10:09 pfmoore

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.

pradyunsg avatar Sep 04 '22 16:09 pradyunsg

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.

pfmoore avatar Sep 04 '22 16:09 pfmoore

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

pradyunsg avatar Sep 04 '22 17:09 pradyunsg

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?

pradyunsg avatar Jan 30 '23 23:01 pradyunsg

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 🙂

pfmoore avatar Jan 31 '23 01:01 pfmoore

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

pradyunsg avatar Jan 31 '23 01:01 pradyunsg