podhmo / python-node-semver

python version of node-semver
MIT License
21 stars 15 forks source link

[question] Correct way to include prereleases in ranges #11

Open memsharded opened 7 years ago

memsharded commented 7 years ago

Having a look at the specification, and your tests, it is not fully clear what would be the correct way to include prereleases in the range expression, for example for a set as ["1.1.1-a.1", "1.1.1-a.11", "1.1.1-a.111", "1.1.1-a.21"], the expression "~1.1.1-*" will return None, but the "~1.1.1-" works. Also "~1.1.1-a" works.

Just to make sure, as "~1.1.1-" read a bit ugly (but I am fine with it)

Many thanks!

podhmo commented 7 years ago

https://github.com/npm/node-semver/issues/130

For historical reason, any(*) doesn't match prerelease. so this is same behaviour, but "~1.1.1-" is a bit ugly isn't ? (I think so too)

const semver = require("semver");
var cands = ["1.1.1-a.1", "1.1.1-a.11", "1.1.1-a.111", "1.1.1-a.21"];
console.log(semver.maxSatisfying(cands, "~1.1.1"));
console.log(semver.maxSatisfying(cands, "~1.1.1", true));
console.log(semver.maxSatisfying(cands, "~1.1.1-"));
console.log(semver.maxSatisfying(cands, "~1.1.1-", true)); // 1.1.1-a.111 (only this)
console.log(semver.maxSatisfying(cands, "~1.1.1-*")); 
console.log(semver.maxSatisfying(cands, "~1.1.1-*", true));
memsharded commented 7 years ago

Yes, it is a bit ugly, but if it is consistent, then perfect for me :)

memsharded commented 7 years ago

It seems that the expected result (at least for some of our users), is to accept pre-releases when there is no matching release. According to the semver specification, it says about precedence, but it is not that clear that should never be returned.

So something like this is what they would expect:

from semver import max_satisfying

def satisfy(versions, range_, result):
    sat = max_satisfying(versions, range_, loose=False)
    assert result == sat, "%s != %s" % (result, sat)

# If only pre-releases are available, then they should be returned
satisfy(['1.2.3-alpha.1'],          '~1.2.3', '1.2.3-alpha.1')
satisfy(['1.2.3-pre.1', '1.2.3'], '~1.2.3', '1.2.3')

What do you think?

podhmo commented 7 years ago

hmm, I want to keep this package is (almost) transparent node's semver fork. So, if possible, keeping same behavior of node's one.

podhmo commented 7 years ago

loose option

loose=False prevents patch-version. (only x.y.z).

custom satisify()

node-semver is just library, so it is OK that one application has ones specific the way of handling version number, i think.

casually,

from semver import max_satisfying

def satisfy(versions, range_, result):
    if range_.startswith("~"):
        range_ = range_ + "-"
    sat = max_satisfying(versions, range_, loose=True)
    assert result == sat, "%s != %s" % (result, sat)

# If only pre-releases are available, then they should be returned
satisfy(['1.2.3-alpha.1'], '~1.2.3', '1.2.3-alpha.1')
satisfy(['1.2.3'], '~1.2.3', '1.2.3')
satisfy(['1.2.3-pre.1', '1.2.3'], '~1.2.3', '1.2.3')
memsharded commented 7 years ago

Good point, I will give this a try. Thanks!

perfhunter commented 6 years ago

Hm... But if this solution works only for pre-releases matching a given version and later releases. It doesn't work for the pre-releases of later versions. I.e.:

satisfy(['1.2.3-alpha.1'], '~1.2.3', '1.2.3-alpha.1') - Ok
satisfy(['1.2.4'], '~1.2.3', '1.2.4') - Ok
satisfy(['1.2.4-alpha.1'], '~1.2.3', '1.2.4-alpha.1') - Error, no match at all

This is not the behavior I expect. In the context of Conan version matching I want the following: if I have e.g. a library bug fixed in version 1.2.3-alpha.4, and my package relies on that bug to be fixed, I want to write just

requires = "mylibrary/[>=1.2.3-alpha.4]@user/channel"

so all later versions will satisfy this requirement, either releases or pre-releases. It does not work that way now.

podhmo commented 6 years ago

this is your own task, but example is like a below.

from semver import max_satisfying, make_semver, sort
from collections import defaultdict

def custom_max_satisfying(candidates, query):
    if not query.startswith("~"):
        return max_satisfying(candidates, query, loose=True)
    else:
        normalized = defaultdict(list)
        for x in candidates:
            v = make_semver(x, loose=True)
            normalized["{}.{}.{}".format(v.major, v.minor, v.patch)].append(v)
        mv = max_satisfying(list(normalized), query)
        return sort(normalized[mv], loose=True)[-1].version

print(custom_max_satisfying(["1.2.3-pre"], "~1.2.3"))
# 1.2.3-pre
print(custom_max_satisfying(["1.2.3", "1.2.3-pre"], "~1.2.3"))
# 1.2.3
print(custom_max_satisfying(["1.2.3", "1.2.3-pre", "1.2.4-pre"], "~1.2.3"))
# 1.2.4-pre
climblinne commented 6 years ago

There is a new option includePrerelease in the node-semver project. Look at https://github.com/npm/node-semver/commit/ce969d11ced234f15483c745b06152aba76f91a4. I think, that would be a good option to go for. What are you thinking?

podhmo commented 6 years ago

Oh, I'm not catching-up original semver project, recentrly. It is helpful, maybe.

climblinne commented 5 years ago

First implementation to include prereleases: #22.

podhmo commented 5 years ago

0.5.0 is released.