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

Is an empty string a valid LegacyVersion? #382

Closed
uranusjr opened this issue Jan 2, 2021 · 14 comments
Closed

Is an empty string a valid LegacyVersion? #382

uranusjr opened this issue Jan 2, 2021 · 14 comments

Comments

@uranusjr
Copy link
Member

uranusjr commented Jan 2, 2021

From pypa/pip#9405 (comment)

Current pip allows a user to specify requirements like packaging==. packaging parses the part after == as a LegacyVersion containing an empty string. Whether this is a bug or not, depending on how far the “LegacyVersion can contain anything” idea is taken, but the behaviour is quite confusing to both users.

Since distutils and setuptools both use 0.0.0 if a package is built without specifying a version (or an empty one), it is reasonable to disallow LegacyVersion("") IMO.

@pradyunsg
Copy link
Member

Let's do this. :)

@pfmoore
Copy link
Member

pfmoore commented Jan 3, 2021

Note that even if LegacyVersion("") is considered valid, I'd still consider there to be a question about whether a specifier of bare == should be allowed. PEP 440 states that "A version matching clause includes the version matching operator == and a version identifier", and it feels to me that == is missing the "version identifier" part. The PEP doesn't cover legacy versions (so in terms of the PEP an empty version is invalid), so it's not a conformance issue, but I feel that allowing a bare comparison operator goes against the spirit of the PEP.

Edit: Sorry, didn't refresh my browser, so I missed @pradyunsg's comment.

I agree, let's do it.

@pfmoore
Copy link
Member

pfmoore commented Jan 3, 2021

Actually, what's the timescale for the next major release of the project, because it appears that LegacyVersion is due to be removed (see here)...

@pradyunsg
Copy link
Member

Anytime this year. ;)

packaging also follows a CalVer cadence.

@pfmoore
Copy link
Member

pfmoore commented Jan 3, 2021

More to the point, then, I guess there's no reason to disallow "" as a LegacyVersion, because the same release that does that will also remove LegacyVersion (or am I reading too much into the statement "creating a LegacyVersion has been deprecated"?)

@pradyunsg
Copy link
Member

#321 (comment)

No, you're right. This project will drop Legacy Version very soon, and we probably don't need to fix this code before removing it.

@brettcannon
Copy link
Member

More reason to drop LegacyVersion then! 😁

@hrnciar
Copy link
Contributor

hrnciar commented Oct 21, 2021

#321 (comment)

No, you're right. This project will drop Legacy Version very soon, and we probably don't need to fix this code before removing it.

Hello, I see packaging 21.0 was released in July, but Legacy Version is still present. May I ask why it wasn't removed yet?

@pradyunsg
Copy link
Member

pradyunsg commented Oct 21, 2021

See #407 for the discussion. Basically, I didn't have time for the PR removing it, and that resulted in not removing legacy in the first release.

@hrnciar
Copy link
Contributor

hrnciar commented Oct 21, 2021

I see, thank you. I am asking because I am trying to write a parser for PEP 508 as discussed here. So I would like to know whether it should support Legacy Version or I can ignore these tests.

@pradyunsg
Copy link
Member

Definitely ignore them. Legacy versions are non-PEP 508 versions.

@pradyunsg
Copy link
Member

pradyunsg commented Oct 21, 2021

Could you file a new issue for the hand written parser for PEP 508 against this repo? It'll let us drop the dependency on pyparsing, which would be nice.

@hrnciar
Copy link
Contributor

hrnciar commented Oct 21, 2021

Could you file a new issue for the hand written parser for PEP 508 against this repo?

Sure thing, see #468

@pradyunsg
Copy link
Member

LegacyVersion is no longer a thing. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants