semver: Better support for PEP440 suffixes

[DolceTriade]

If we have packages like

[package.dependencies]
pytz = "*"
setuptools = ">=0.7"
six = ">=1.4.0"
tzlocal = ">=2.0,<3.dev0 || >=4.dev0"

or

[[package]]
name = "ndg-httpsclient"
version = "0.5.1"
description = "Provides enhanced HTTPS support for httplib and urllib2 using PyOpenSSL"
optional = false
python-versions = ">=2.7,<3.0.dev0 || >=3.4.dev0"
files = [
    {file = "ndg_httpsclient-0.5.1-py2-none-any.whl", hash = "sha256:d2c7225f6a1c6cf698af4ebc962da70178a99bcde24ee6d1961c4f3338130d57"},
    {file = "ndg_httpsclient-0.5.1-py3-none-any.whl", hash = "sha256:dd174c11d971b6244a891f7be2b32ca9853d3797a72edb34fa5d7b07d8fff7d4"},
    {file = "ndg_httpsclient-0.5.1.tar.gz", hash = "sha256:d72faed0376ab039736c2ba12e30695e2788c4aa569c9c3e3d72131de2592210"},
]

We can get errors like error: Constraint "<3.0.dev0" could not be parsed. This PEP
(https://peps.python.org/pep-0440/#inclusive-ordered-comparison)
suggests that these versions are legit and thus poetry2nix should be
able to parse them.

This support isn't perfect because it's overly broad and ultimately
still uses builtins.compareVersions which inherently cannot be
compatible with pythonisms.

This seems to be good enough, however to allow these packages to be
built with the correct versions.

[andersk]

Based on PEP 440, it’s not just dev, there’s also a, b, c, rc, alpha, beta, pre, preview, post, rev, r, !, -, _.

[DolceTriade]

Is the right way to add those chars to the regex or is there a better to incorporate this?

[supermarin]

Think you want dev outside of [], maybe

version = "([0-9.*x]+(dev.*)?)"

[andersk]

In general, any regex with .* (outside of the brackets [], where it just means those two literal characters) is guaranteed to be broken.

In this case, for example, overly broad matches of .* will break version ranges:

1.2.3dev1 - 1.2.6
        ^^^^^^^^^
        matches .*

(Although it’s unclear to me why we support ranges with - here when, as far as I can see, Poetry has no such thing.)

[supermarin]

Yep you're right, also seen the comment above about PEP 440.
I can craft & test a regex that will match PEP 440, but to @DolceTriade 's point - is there a better way to do it, like utilize a python lib?

[DolceTriade]

Responded to this in the main comments, but unless I'm mistaken, this will require us to write to the store for every check if we want to use a shell/python script to check versions, so for that reason, I less than optimal, but generally working Nix version might be preferable.

[andersk]

Hmm, I think there’s a bigger problem here though. After parsing these versions, we’re passing them to builtins.compareVersions, which has no special understanding of these Pythonisms and incorrectly gives 1.2.3 < 1.2.3b1 < 1.2.3dev1 < 1.2.4.

[DolceTriade]

https://packaging.pypa.io/en/stable/version.html is a thing and it handles it pretty well...

v1 = Version("1.0a5")
v2 = Version("1.0")
v1
<Version('1.0a5')>
v2
<Version('1.0')>
v1 < v2
True
v1 == v2
False
v1 > v2
False
v1 >= v2
False
v1 <= v2
True

Should we shell out to this instead of nix based regex parsing?

Edit: I guess the right thing would be to just use poetry core if we're going to shell out...

[DolceTriade]

So I just updated the PR to use a more generic regex. It's intentionally overly broad to capture whatever people might want to put in the versions. I suppose there is no guarantee that PEP440 will be followed anyways so we should still allow these versions to be parsed.

Of course, noting these pythonisms will be broken when using builtins.compareVersions, but it's probably close enough to work for the majority of cases. The few cases that don't work might have to use manual overrides to correct the version.

Lastly, I looked into writing a python script, but that seems like it would require writing to the store for every check, which seems wasteful so I opted to not do that.

WDYT?

[supermarin]

Lastly, I looked into writing a python script, but that seems like it would require writing to the store for every check, which seems wasteful so I opted to not do that.

Mind elaborating on this one?

[DolceTriade]

Mind elaborating on this one?

My understanding is that if you want to shell out to an external program for nix, you have to create a derivation to run that script (which involves writing to the store to "build" it, and then writing to the store for when you write the result to ${out} and then use builtins.readFile to read the result. Is there a better way of doing this?

If we could shell out, then this would be trivial as we could use poetry's own libs to manage constraints.

[supermarin]

Coming back to this: there's 2 issues to tackle.

1. Regex to parse versions correctly. PEP 440 provides the regex and test case [1]. I don't think it hurts to mimic the official implementation regex.
2. Properly sorting versions. This could be done in pure nix, but in ideal world both would be done utilizing existing python infrastructure.

Regarding shelling out, I think you're correct, but it also seems like poetry2nix already does rely on python scripts.
@adisbladis would you please share some thoughts on using python for this?

[1] From PEP 440:

To test whether a version identifier is in the canonical format, you can use the following function:

import re
def is_canonical(version):
    return re.match(r'^([1-9][0-9]*!)?(0|[1-9][0-9]*)(\.(0|[1-9][0-9]*))*((a|b|rc)(0|[1-9][0-9]*))?(\.post(0|[1-9][0-9]*))?(\.dev(0|[1-9][0-9]*))?$', version) is not None

To extract the components of a version identifier, use the following regular expression (as defined by the packaging project):

VERSION_PATTERN = r"""
    v?
    (?:
        (?:(?P<epoch>[0-9]+)!)?                           # epoch
        (?P<release>[0-9]+(?:\.[0-9]+)*)                  # release segment
        (?P<pre>                                          # pre-release
            [-_\.]?
            (?P<pre_l>(a|b|c|rc|alpha|beta|pre|preview))
            [-_\.]?
            (?P<pre_n>[0-9]+)?
        )?
        (?P<post>                                         # post release
            (?:-(?P<post_n1>[0-9]+))
            |
            (?:
                [-_\.]?
                (?P<post_l>post|rev|r)
                [-_\.]?
                (?P<post_n2>[0-9]+)?
            )
        )?
        (?P<dev>                                          # dev release
            [-_\.]?
            (?P<dev_l>dev)
            [-_\.]?
            (?P<dev_n>[0-9]+)?
        )?
    )
    (?:\+(?P<local>[a-z0-9]+(?:[-_\.][a-z0-9]+)*))?       # local version
"""

_regex = re.compile(
    r"^\s*" + VERSION_PATTERN + r"\s*$",
    re.VERBOSE | re.IGNORECASE,
)

[cpcloud]

Perhaps the pyproject.nix stuff that @adisbladis has been working on has PEP440 support?

[supermarin]

Oh dang, completely missed that. It looks like he already implemented it here: https://github.com/adisbladis/pyproject.nix/blob/master/lib/pep440.nix .
@DolceTriade we can probably close this one since it looks like pyproject.nix changes have been merged 841aa10

[adisbladis]

Poetry2nix PEP440 support has been reimplemented on top of https://nix-community.github.io/pyproject.nix/lib/pep440.html in #1376.