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

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

Open
memsharded opened this issue Sep 13, 2017 · 12 comments
Open

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

memsharded opened this issue Sep 13, 2017 · 12 comments

Comments

@memsharded
Copy link

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
Copy link
Owner

podhmo commented Sep 13, 2017

npm/node-semver#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
Copy link
Author

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

@memsharded
Copy link
Author

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
Copy link
Owner

podhmo commented Oct 2, 2017

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
Copy link
Owner

podhmo commented Oct 2, 2017

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
Copy link
Author

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

@perfhunter
Copy link

perfhunter commented Apr 27, 2018

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
Copy link
Owner

podhmo commented Sep 28, 2018

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
Copy link
Contributor

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

@podhmo
Copy link
Owner

podhmo commented Oct 10, 2018

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

@climblinne
Copy link
Contributor

First implementation to include prereleases: #22.

@podhmo
Copy link
Owner

podhmo commented Nov 1, 2018

0.5.0 is released.

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

4 participants