classification
Title: distutils.LooseVersion fails to compare number and a word
Type: behavior Stage:
Components: Distutils Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, DLitz, Natalia, YP, barry, eric.araujo, ncoghlan, piotr, pitrou, samuel.lai, serhiy.storchaka, tarek
Priority: normal Keywords: patch

Created on 2012-05-23 20:42 by Natalia, last changed 2018-10-19 17:54 by serhiy.storchaka.

Files
File name Uploaded Description Edit
14894.patch Natalia, 2012-06-03 15:39 review
Messages (27)
msg161453 - (view) Author: Natalia (Natalia) Date: 2012-05-23 20:42
$ python2.7 -c 'from distutils.version import LooseVersion as V; print V("a") > V("1")'
True
$ python3.2 -c 'from distutils.version import LooseVersion as V; print(V("a") > V("b"))'
False
$ python3.2 -c 'from distutils.version import LooseVersion as V; print(V("a") > V("1"))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3.2/distutils/version.py", line 70, in __gt__
    c = self._cmp(other)
  File "/usr/lib/python3.2/distutils/version.py", line 343, in _cmp
    if self.version < other.version:
TypeError: unorderable types: str() < int()
msg161466 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-23 22:13
Thanks for the report.  How did you find this?  According to the doc of LooseVersion, 'a' is not valid, so I would like a real example to accept this as a bug.
msg161467 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2012-05-23 22:25
On May 23, 2012, at 10:13 PM, Éric Araujo wrote:

>
>Thanks for the report.  How did you find this?  According to the doc of
>LooseVersion, 'a' is not valid, so I would like a real example to accept this
>as a bug.

It works in Python 2.7 so I think it was viewed as a regression.

Python 2.7.3 (default, Apr 20 2012, 22:39:59) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from distutils.version import LooseVersion as v
>>> v('a') < v('0')
False

But if 'a' is invalid, then LooseVersion should refuse to accept it in its
constructor, right?
msg161490 - (view) Author: Natalia (Natalia) Date: 2012-05-24 08:54
Hello, as a GSoC student, I'm working on PyPI to Debian repository converter. I wanted to compare  versions of packages available in PyPI and it broke while comparing appwsgi, wsgi-design ('default') and gar ('prototype.1') versions.
msg161515 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-24 14:59
> Hello, as a GSoC student, I'm working on PyPI to Debian repository converter.
Great!  I hope you’re aware of previous efforts like stdeb (unfortunately requires setuptools) and py2rpm (for rpm systems but some parts can be inspiration, like the download code).  I follow debian-python so we may encounter each other there.

> I wanted to compare  versions of packages available in PyPI and it broke while comparing appwsgi,
> wsgi-design ('default') and gar ('prototype.1') versions.
I wonder why you need to compare versions from different projects.  What is “it” in “it broke”?  If it’s a regular distutils command, please paste the full command and log; if you meant you used the LooseVersion class in your code, then I’m afraid I will say this won’t be fixed: this is not a public class.  distutils2.version however does contain a public class that implements PEP 386; even if the project is still in alpha (and I want to make a few renamings and API changes), you can still use it (and when I break your code a quick look at the CHANGES file will help you update).
msg161516 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-24 15:03
About Barry’s reply:

> But if 'a' is invalid, then LooseVersion should refuse to accept it in its constructor, right?
It’s complicated.  The doc does not say much, the docstring however clearly states that versions should start with a digit, but a ton of projects use invalid-but-not-rejected formats.  distutils2 makes a clean break with clear rules (PEP 386); I’m not sure it would be acceptable for distutils to suddenly reject these versions.  It sounds useful but even with that change people would not always comply with PEP 386, so better let them use broken versions with distutils and force a switch to a fully compliant format with d2.  Does that make sense?
msg161517 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2012-05-24 15:12
On May 24, 2012, at 03:03 PM, Éric Araujo wrote:

>> But if 'a' is invalid, then LooseVersion should refuse to accept it in its
>> constructor, right?

>It’s complicated.  The doc does not say much, the docstring however clearly
>states that versions should start with a digit, but a ton of projects use
>invalid-but-not-rejected formats.  distutils2 makes a clean break with clear
>rules (PEP 386); I’m not sure it would be acceptable for distutils to
>suddenly reject these versions.  It sounds useful but even with that change
>people would not always comply with PEP 386, so better let them use broken
>versions with distutils and force a switch to a fully compliant format with
>d2.  Does that make sense?

It does, but in that case, I think the comparison should continue to succeed,
with whatever results it produces in Python 2.7.
msg162213 - (view) Author: Natalia (Natalia) Date: 2012-06-03 14:46
Hi, I'm attaching a patch that fixes this issue:)
msg162214 - (view) Author: Natalia (Natalia) Date: 2012-06-03 15:39
I had a wrong return value in one of unit tests, fixed.
msg163898 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-06-25 05:59
> I think the comparison should continue to succeed,
> with whatever results it produces in Python 2.7
The point I was trying to make is that this is not something done on purpose, nor useful or guaranteed, but only a side effect of 2.x’s mixed comparisons.  As undefined behavior, I don’t want to change 3.x to match it.

Natalia, could you reply to my questions in msg161515 ?  Thanks.
msg174376 - (view) Author: Dwayne Litzenberger (DLitz) Date: 2012-10-31 21:49
As far as a real-world example is concerned, if you're using git-describe to generate your version numbers, you can pretty easily end up with something like "ab25c6fe95ee92fac3187dcd90e0560ccacb084a".
msg177188 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-12-09 00:29
Thanks Dwayne.  I think that most tools (e.g. setuptools) generate things like 1.0.2+gitab25c6fe95ee92fac3187dcd90e0560ccacb084a i.e. real version set by maintainer + hash, not just the hash, so I am still not convinced a change is needed.
msg177301 - (view) Author: Dwayne Litzenberger (DLitz) Date: 2012-12-10 16:39
"git describe --tags --always" will return a bare commit id if there is no previous tag.  This is pretty common to have when you're working on a new package that hasn't been released yet:

$ git init
Initialized empty Git repository in /tmp/repo/.git/
$ touch foo
$ git add foo
$ git commit -m 'Initial commit'
[master (root-commit) cd7dd74] Initial commit
 0 files changed
 create mode 100644 foo
$ git describe --tags --always
cd7dd74
$ git tag v1.0
$ git describe --tags --always
v1.0
msg177304 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-12-10 18:54
> As far as a real-world example is concerned, if you're using
> git-describe to generate your version numbers, you can pretty easily
> end up with something like "ab25c6fe95ee92fac3187dcd90e0560ccacb084a".

And how are you supposed to compare that with anything else?

I think `V("a") > V("b")` should raise TypeError in Python 3.
msg177305 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-12-10 19:01
Don’t know if you’ve seen my previous message:

I think that most tools (e.g. setuptools) generate things like 1.0.2+gitab25c6fe95ee92fac3187dcd90e0560ccacb084a i.e. real version set by maintainer + hash, not just the hash.

I was talking about a version number set by the author, typically 0.1, not a VCS tag.
msg177306 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-12-10 19:03
> Don’t know if you’ve seen my previous message:

I'm a bit lost, how are you replying to? :)
msg177307 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-12-10 19:03
Or, rather, who :-S
msg177308 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-12-10 19:04
I was replying to Dwayne.
msg197515 - (view) Author: Sam Lai (samuel.lai) Date: 2013-09-12 07:30
I have a more realistic example of this bug. In the docstring for distutils.LooseVersion, it says '1.5.1' and '3.2.p10' are both valid version numbers. If instead of '3.2.p10', we use '1.5.p10', the following occurs -

>>> v1 = LooseVersion('1.5.1')
>>> v2 = LooseVersion('1.5.p10')
>>> v1>v2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python33\Lib\distutils\version.py", line 70, in __gt__
    c = self._cmp(other)
  File "C:\Python33\Lib\distutils\version.py", line 343, in _cmp
    if self.version < other.version:
TypeError: unorderable types: int() < str()
>>> v1.version
[1, 5, 1]
>>> v2.version
[1, 5, 'p', 10]

The reason it occurs is pretty clear when the .version list is printed. (That's also explains why I had to use 1.5.p10 instead because otherwise the comparison would not proceed past the first element of both lists.)

In my real-life example, I'm trying to compare '1.1.0-3' and '1.1.0-beta-10'.

>>> v3=LooseVersion(ll[0])
>>> v4=LooseVersion(ll[1])
>>> v3>v4
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python33\Lib\distutils\version.py", line 70, in __gt__
    c = self._cmp(other)
  File "C:\Python33\Lib\distutils\version.py", line 343, in _cmp
    if self.version < other.version:
TypeError: unorderable types: int() < str()
>>> v3.version
[1, 1, 0, '-', 3]
>>> v4.version
[1, 1, 0, '-', 'beta', '-', 10]
msg213227 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-03-12 09:16
I’m still torn between improving this as best as we can and waiting for clean new spec and code.  Last PyCon Nick said he had a PEP in mind to specify how distutils would be updated to support new standards; I think we’ll revisit this issue when that policy exists.
msg213253 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-03-12 12:02
Unfortunately, the current likely answer re updating vanilla distutils to handle the new standards is "we won't". The backwards compatibility issues involved are just too hairy for us to start enabling by default in the standard library, and I've become convinced that coupling the build and installation tools to the language version is fundamentally a mistake anyway (hence the bundling approach in PEP 453).

Instead, we're hacking around the metadata side of the problem through the fact that pip always runs setup.py under setuptools (even if the setup.py only used vanilla distutils) and encouraging the use of cross-version compatible tools in other cases (with setuptools being the current de facto choice, since the distlib APIs are still considered experimental).

For this particularly case, I don't see any harm in bringing distutils in Py3 back in line with Py2, especially if it's also consistent with setuptools.
msg213443 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-03-13 19:17
> Unfortunately, the current likely answer re updating vanilla distutils to handle the new
> standards is "we won't". The backwards compatibility issues involved are just too hairy
> for us to start enabling by default in the standard library, and I've become convinced
> that coupling the build and installation tools to the language version is fundamentally a
> mistake anyway (hence the bundling approach in PEP 453).

This makes sense.  Given that distutils is still a basic packaging solution included in the stdlib, and no longer under a feature freeze, what kind of improvements do you think it can get?

* Can we switch to SSL with certificate checking?  I think it’s a big yes.
* Can we improve the UI of some commands in a backward-compatible way?  (Existing example: allowing the upload command to send an existing file, instead of requiring to build and upload in one command line)
* Can we add new commands like upload_docs and test (that would just run unittest discovery)?  It may be judged wasted time, if the whole world uses setuptools (but does it?).
* Can we add support for wheel?  Building extensions with the stable ABI?  New version or metadata formats?

> For this particularly case, I don't see any harm in bringing distutils in Py3 back in
> line with Py2, especially if it's also consistent with setuptools.

It’s basically adding code to support comparison of mismatched types, i.e. time and effort to add something similar to a Python 2 design flaw.  (setuptools’ version class is different from both distutils’ classes.)
msg213444 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-03-13 19:20
Forgot one:

* Can we add tar.xz support to sdist?
msg213486 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-03-13 22:30
Yes, I agree we should allow backwards compatible distutils RFEs if people
want to work on them. It's not going away any time soon, we just need to be
careful with any internal refactoring.
msg328043 - (view) Author: YP (YP) Date: 2018-10-19 13:22
Hi,

Just wanted to know if there was anything new on this?
When managing softwares version we often are not at all master of the naming convention used: we just have to deal with it.

"LooseVersion" was very nice as it would take any string as input and do "the best" with it.

Now that it's broken for some type of version string, it become quite useless.

I overloaded the comparison part more or less using the Natalia patch idea (but done after the argument split). But shouldn't that be default?

Thanks.
msg328056 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2018-10-19 17:34
I am inclined to reject the patch.

LooseVersion is not meant to be a general utility for all modules; I know it’s used in the wild but projects should define their parsing/comparison function or make up a specific module for that.

If the problem happens from distutils or pip when comparing real distributions that really define version='a' and version='1', then modern setuptools or flit or twine will warn about that.
msg328058 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-10-19 17:54
If you want to make LooseVersion a tiny bit more robust, I suggest to borrow the algorithm from the platform module (see issue26544).
History
Date User Action Args
2018-10-31 23:53:21eric.araujolinkissue35129 superseder
2018-10-19 17:54:10serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg328058
2018-10-19 17:34:57eric.araujosetmessages: + msg328056
2018-10-19 13:22:18YPsetnosy: + YP
messages: + msg328043
2017-05-05 18:26:05berker.peksaglinkissue30272 superseder
2015-01-30 21:16:07eric.araujosetassignee: eric.araujo ->
versions: + Python 3.5, - Python 3.3, Python 3.4
2015-01-30 21:15:48eric.araujolinkissue23348 superseder
2014-03-13 22:30:03ncoghlansetmessages: + msg213486
2014-03-13 19:20:28eric.araujosetmessages: + msg213444
2014-03-13 19:17:03eric.araujosetmessages: + msg213443
2014-03-12 12:02:26ncoghlansetmessages: + msg213253
2014-03-12 09:16:39eric.araujosetnosy: + ncoghlan

messages: + msg213227
versions: + Python 3.4, - Python 3.2
2013-09-12 07:30:51samuel.laisetnosy: + samuel.lai
messages: + msg197515
2012-12-14 08:33:43Arfreversetnosy: + Arfrever
2012-12-10 19:04:28eric.araujosetmessages: + msg177308
2012-12-10 19:03:53pitrousetmessages: + msg177307
2012-12-10 19:03:36pitrousetmessages: + msg177306
2012-12-10 19:01:51eric.araujosetmessages: + msg177305
2012-12-10 18:54:24pitrousetnosy: + pitrou
messages: + msg177304
2012-12-10 16:39:24DLitzsetmessages: + msg177301
2012-12-09 00:29:30eric.araujosetmessages: + msg177188
stage: needs patch ->
2012-10-31 21:49:59DLitzsetnosy: + DLitz
messages: + msg174376
2012-06-25 05:59:12eric.araujosetmessages: + msg163898
2012-06-03 15:39:31Nataliasetfiles: - 14894.patch
2012-06-03 15:39:18Nataliasetfiles: + 14894.patch

messages: + msg162214
2012-06-03 14:46:17Nataliasetfiles: + 14894.patch
keywords: + patch
messages: + msg162213
2012-05-24 15:12:58barrysetmessages: + msg161517
2012-05-24 15:03:30eric.araujosetmessages: + msg161516
2012-05-24 14:59:42eric.araujosetmessages: + msg161515
2012-05-24 08:54:29Nataliasetmessages: + msg161490
2012-05-23 22:25:07barrysetmessages: + msg161467
2012-05-23 22:13:01eric.araujosetmessages: + msg161466
2012-05-23 20:47:02barrysetnosy: + barry
2012-05-23 20:46:46pitrousetstage: needs patch
versions: + Python 3.3, - Python 3.1
2012-05-23 20:46:07piotrsetnosy: + piotr
2012-05-23 20:42:29Nataliacreate