Skip to content

Conversation

@vphilippon
Copy link
Member

Fixes #4169
PEP 427 has no specific requirements for the format of the version.
As pip wheel allows to create a wheel with a non-PEP 440 version, it should also be able to install that same wheel.

PEP 427 has no specific requirements for the format of the version.
As pip wheel allows to create a wheel with a non-PEP 440 version,
it should also be able to install that same wheel.
@vphilippon vphilippon force-pushed the install-whell-with-non-pep440-version branch from e0cee6c to 8378567 Compare March 28, 2017 03:12
@xavfernandez xavfernandez added this to the 10.0 milestone Mar 28, 2017
@pfmoore
Copy link
Member

pfmoore commented Mar 28, 2017

PEP 440 was accepted after PEP 427. I would consider it implicit that wheels should only use standard format versions, and hence once PEP 440 was standardised, that would apply by default. I don't think this change is something we should support. If we need a clarifying statement in PEP 427 saying that the version should be a PEP 440 format version, then let's do that.

What is the motivating use case for this change?

@xavfernandez
Copy link
Member

@pfmoore the motivation is summarized in issue #4169 and can be sumed up as:
"With an invalid PEP-440 version, pip install . works, pip wheel . works but the resulting built wheel is refused at install time".

So either we allow pip to install the wheels it just built (this current PR) or we make it error on pip wheel with invalid PEP-440 versions (but then, we'll have pip install . work on something pip wheel . will choke on).

Either way, I don't like it, that's why I decided to blindly follow the current specification.

@vphilippon I'm sorry to have directed you on this issue as we might not merge it in the end...

@pfmoore
Copy link
Member

pfmoore commented Mar 28, 2017

@xavfernandez Ah, OK. I didn't see the link to the original issue. My apologies.

IMO, it's a minor point either way (and in fact, I'm not particularly concerned even about the ambiguity - I'd probably have just said "well, don't do that" on #4169). My biggest concern is that it's not pip wheel itself that allows invalid versions, it's ultimately setuptools and/or bdist_wheel. If something else in the toolchain tightened up on validation of version numbers, we'd be in the opposite situation ("pip wheel won't let me create a wheel that pip install allows me to install").

I'm not going to block this going in, but I do think it should at least be considered as undocumented/not guaranteed behaviour.

@RonnyPfannschmidt
Copy link
Contributor

how about printing a warning on both creating and installing wrt the invalid version?

@vphilippon
Copy link
Member Author

vphilippon commented Mar 28, 2017

@xavfernandez It's allright, it's not like I spent 4 week on this.

Currently, wheels with a non-PEP 440 version will work, as long as the version starts with a digit
(i.e. simple-3invalid-py2-none-any.whl will work, if you look at the previous regex).
This PR just removes this constraint that isn't given anywhere.

So, if we go and actually refuse non-PEP 440 versions, I say we go all the way to be consistent, which is likely to break some people. That'd be a breaking change, for little gain IMO.

For your concern about another tool tightening up, @pfmoore, I'd say we can add such new constraints once this happens, if it happens.

The warning suggested by @RonnyPfannschmidt is a good idea. That might help users notice they are using non-PEP 440 versions by mistake (like having 1.2.3.goofy instead of 1.2.3+goofy).
Should I go with that?

@dstufft
Copy link
Member

dstufft commented Apr 1, 2017

I think we should merge this, we still support non PEP 440 versions everywhere else so this is the odd ball out. If we want to talk about mandating PEP 440 that is something we can talk about, but I think that should be it's own dedicated issue and should handle all of our version handling, not just one place.

@pfmoore
Copy link
Member

pfmoore commented Apr 1, 2017

Cool. With @xavfernandez and @dstufft in favour, I withdraw my objections.

@dstufft dstufft merged commit 197c057 into pypa:master Apr 1, 2017
@dstufft
Copy link
Member

dstufft commented Apr 1, 2017

Thanks!

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

auto-locked Outdated issues that have been locked by automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pip wheel should not create wheels that it will itself deem invalid

5 participants