Skip to content
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

PEP 453: only remove setuptools once pip drops support for legacy installs #2096

Merged
merged 5 commits into from
Jul 2, 2022

Conversation

graingert
Copy link
Contributor

No description provided.

@gvanrossum
Copy link
Member

Can you link to a discussion where this change was proposed?

@graingert
Copy link
Contributor Author

Can you link to a discussion where this change was proposed?

https://mail.python.org/archives/list/python-dev@python.org/message/AM7ATX4IWLNXKG54Z34GYZ2D7RJWQUNC/ @pfmoore

Copy link
Member

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

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

I don't think we want to try to pin down when setuptools gets removed. It is explicitly documented as an implementation detail, and I think it should simply be removed "when it's appropriate".

I'd actually rather see this statement removed altogether, although I'm personally just as happy to not bother - I don't think anyone is likely to try to insist that we have to remove setuptools just because the PEP says so, even if it makes things worse for end users.

@dstufft
Copy link
Member

dstufft commented Oct 2, 2021

I think there's little reason to change the PEP, once the PEP has been implemented they're pretty much considered historical documents anyways that document the intent at the time the change was made.

@graingert
Copy link
Contributor Author

I think the desire to remove setuptools here relates to the installation of easy_install as an additional console script - which has now been removed

@pradyunsg
Copy link
Member

pradyunsg commented Oct 3, 2021

If we do decide to go down the route of updating this PEP's wording, I'm also somewhat strongly opposed to the currently proposed phrasing. I would be much more comfortable with coupling this removal with pip's only other supported method of installation:

Once get-pip.py stops installing setuptools by default, then the private copy of setuptools will be removed from ensurepip in subsequent CPython releases.

I'll leave the question of whether the PEP should be updated to the PEP authors and PEP editors. :)

pep-0453.txt Outdated Show resolved Hide resolved
pep-0453.txt Outdated Show resolved Hide resolved
@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 24, 2021

In this case, I think it's worth updating an otherwise finished PEP, as the setuptools removal is a forward looking step that we haven't completed yet, and the stated trigger for it is outright wrong (hence Illia's confusion that prompted the linked mail thread).

The "remove it from ensurepip after the pip team removes it from get-pip.py" trigger is a good objective one, so I'm personally OK with the current wording of the PR, and I think that wording would be better than just dropping the paragraph completely (since that approach is more confusing for anyone that remembers seeing the old wording and is wondering where it went).

So approved by me as one co-author, but I'd suggest only merging once @dstufft (as the primary author) and @pfmoore (as the current standing delegate for packaging interoperability PEPs) have approved it.

@pfmoore
Copy link
Member

pfmoore commented Oct 24, 2021

I'm still concerned that someone might remove setuptools just because it says to do so in the PEP, whereas the true criterion (IMO) should be "when removing setuptools doesn't harm the user experience".

The "when get-pip.py no longer includes it" rule is reasonable, but it's something that should be applied by someone who knows the packaging ecosystem. The original post in the thread linked above is a good example here - someone coming along and saying "the PEP says we can remove setuptools, should we do so?" It wouldn't be much of a stretch to imagine someone raising a PR rather than asking on python-dev, and while I'd hope that any issues would be caught in PR review, "the PEP says it's OK" acts as a fairly strong temptation to not worry too much.

Can we maybe word the paragraph as something like:

The private copy of setuptools will be removed from ensurepip once it is no longer needed. This is likely to be at the point when get-pip.py stops installing setuptools by default.

@AA-Turner
Copy link
Member

@pfmoore is this PR still needed? I'm not sure where pip landed on this, or if more discussion has happened elsewhere.

A

@pfmoore
Copy link
Member

pfmoore commented Jan 22, 2022

I think it's still at the same point. @ncoghlan is OK with it, I'd like to see a more relaxed wording. And @graingert hasn't responded to my comment yet.

@AA-Turner
Copy link
Member

@Do9i -- giving you the benefit of the doubt, please note that "approving" changes repeatedly on pull requests without contributing to the discussion is not a good way to support the project, and you may be reported to GitHub for spam if you continue to do so.

If you have comments on PEP 453, please direct them to python-dev or Discourse.

A

@graingert graingert requested a review from pfmoore June 29, 2022 20:40
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Editorial changes

pep-0453.txt Outdated Show resolved Hide resolved
Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
@graingert graingert requested a review from CAM-Gerlach June 29, 2022 22:26
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM from an editorial perspective, but up to @pfmoore and co as to whether to merge the change

@pfmoore
Copy link
Member

pfmoore commented Jun 30, 2022

I don't have any problem with the new wording (obviously, as it's the wording I suggested 😉). I still don't actually care much whether this is merged or not - this is a core Python PEP, not a packaging PEP, and like @dstufft noted, I don't think we tend to modify those after approval.

I'm happy to leave it to the PEP editors to decide whether the "no edits after acceptance" policy applies here. I will note that whatever the wording, removal of setuptools should be done based on a judgement made at the time removal is proposed, and not blindly based on whatever is stated in this PEP.

@CAM-Gerlach
Copy link
Member

this is a core Python PEP, not a packaging PEP, and like @dstufft noted, I don't think we tend to modify those after approval.

In that case, as originally discussed on #2656 and done for PEP 632 (PEP-632), maybe we should drop Topic: Packaging from this PEP as well as the related PEP 477 (PEP-477) ? I can do it in #2690 since its closely related to the other changes in that PR (also, your input there on updating PEP 262 (PEP-262)'s status to reflect reality would be helpful).

I'm happy to leave it to the PEP editors to decide whether the "no edits after acceptance" policy applies here. I will note that whatever the wording, removal of setuptools should be done based on a judgement made at the time removal is proposed, and not blindly based on whatever is stated in this PEP.

Like the PEP text, that policy isn't blindly prescriptive either, but here's what my most recent revision to the relevant section of PEP 1 says:

If changes based on implementation experience and user feedback are made to Standards track PEPs while in the Provisional or (with SC approval) Accepted state, they should be noted in the PEP, such that the PEP accurately describes the implementation at the point where it is marked Final.

There appears to be a bit of a special case here—the PEP is marked Final, but the specific passage in question describes something that has not been implemented yet, so given it appears to be causing real-world confusion for users and trouble for maintainers, IMO it would be best if it accurately reflects the apparent reality that you described. Either the textual change is either normative/substantial enough to require SC approval, in which case SC approval would also be required to actually implement something other than what is stated there, or it is not (or was already given), in which case the change can be made with PEP author and editor approval.

@CAM-Gerlach
Copy link
Member

@graingert Not super important since this change is so small, but if want to fix the RTD preview build, you can rebase the PEP on the latest main—the branch is old enough that it doesn't have the RTD config.

@pfmoore
Copy link
Member

pfmoore commented Jun 30, 2022

maybe we should drop Topic: Packaging from this PEP as well as the related PEP 477 (PEP-477) ?

I don't know on that. It's packaging related in the sense that it is about packaging tools, so if people are looking for things that are "about" packaging, it does belong in the Packaging topic. But it's not a "PyPA specification" in the sense defined here, which is why I said that the normal PEP rules about modifications should apply, rather than the PyPA-specific process. Ultimately, I'm not actually sure what the rules are for when a PEP qualifies as being in the Packaging topic...

the PEP is marked Final, but the specific passage in question describes something that has not been implemented yet

Surely we've had other cases where what gets implemented is different from what the PEP described? But I don't want to get sucked into PEP process pedantry, which is why I'll leave it to you.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Jul 1, 2022

Ultimately, I'm not actually sure what the rules are for when a PEP qualifies as being in the Packaging topic...

Yeah, I don't think we ever really formalized anything, AFAIK.

It's packaging related in the sense that it is about packaging tools, so if people are looking for things that are "about" packaging, it does belong in the Packaging topic. But it's not a "PyPA specification" in the sense defined here, which is why I said that the normal PEP rules about modifications should apply, rather than the PyPA-specific process.

Yeah, exactly. That basically comes down to whether a Topic is considered more of a "Category" (i.e. the former), or a "Track" (i.e. the latter). I presented both alternatives at the beginning of the discussion, and both were favored at different points in its evolution; the initial consensus in the Discourse thread was something more like a "Track", but on the PR and naming of the "Topic" header that was eventually implemented (that allows multiple topics per PR), it ended up basically the latter, at least nominally.

However, with PEP 632 (PEP-632) being removed at the behest of @zooba in #2636 (comment) , given these two PEPs basically just concern adding/backporting a utility module from the core standard library rather than PyPA/PyPI standards, it seems to me to not make sense to keep those PEPs but not another very similar one about the deprecation and removal of what was the core packaging module. So, I suggest either removing those two, or adding back PEP 632.

Surely we've had other cases where what gets implemented is different from what the PEP described? But I don't want to get sucked into PEP process pedantry, which is why I'll leave it to you.

Sure, though generally either the PEP is updated, a new one is written, the canonical documentation/spec moves somewhere else—or simply no one notices, heh.

@pradyunsg
Copy link
Member

So, I suggest either removing those two, or adding back PEP 632.

Alternatively, we can defer doing anything here until the authors have a concern with being listed under the packaging index as well or till we see/identify more instances of something like this? In any case, this discussion should move to a separate issue IMO.


So... This is ready for a merge, assuming we're OK with making an edit in a Final PEP, to better reflect status-quo. If we're not, let's close this. That is a decision for someone with the PEP editor hat to make.

I don't think there's much point in any more discussion/nit-picks in this PR given that it's changing a single sentence to match a process detail -- and even that isn't going to be enforced in any way.

@zooba
Copy link
Member

zooba commented Jul 1, 2022

I think this is in the same boat as PEP 632, and shouldn't be tagged as a packaging PEP.

As a proposal of the rule I'd use, if we'd let the packaging delegate (i.e. Paul, right now) decide without going to python-dev or the SC, then it's a packaging PEP. Whether something is added to the standard library clearly falls outside of this scope, and so this PEP is standards track and not packaging.

@CAM-Gerlach
Copy link
Member

In any case, this discussion should move to a separate issue IMO.

Sorry for side-tracking the discussion, I should have moved to a new issue a long time ago.

In any case, I created #2696 , summarized the issue there, copied over the relevant existing discussion and responded to @pradyunsg and @zooba there.

So... This is ready for a merge, assuming we're OK with making an edit in a Final PEP, to better reflect status-quo. If we're not, let's close this. That is a decision for someone with the PEP editor hat to make.

I don't speak for all the PEP editors, only myself, but I'm ✔️ and can merge this unless someone else objects or gets to it first in the next day or two.

@AA-Turner
Copy link
Member

Merging, per Editor consensus (me, Hugo, Cam).

A

@AA-Turner AA-Turner merged commit 764ec27 into python:main Jul 2, 2022
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.