Stabilizing SemVer

[steveklabnik]

Newest version of the proposal is here: http://discuss.rust-lang.org/t/stabilizing-semver/376/15?u=steveklabnik

I haven’t ever done this before, so all of this is extremely preliminary.

SemVer is a tiny crate, and I think we can make it stable. I’d even expect SemVer to become the first crate ever to be locked, but let’s not jump the gun here.

SemVer has only three public things: an enum with two variants, a struct, and a function. Easy.

pub enum Identifier {
    Numeric(uint),
    AlphaNumeric(String)
}

I think this is good, with one small reservation: we’ve been moving away from int/uint, and so I wonder if this shouldn’t be a u32 instead.

pub struct Version {
    pub major: uint,
    pub minor: uint,
    pub patch: uint,
    pub pre: Vec<Identifier>,
    pub build: Vec<Identifier>,
}

This is the same. Seems good, but maybe u32 is more appropriate.

pub fn parse(s: &str) -> Option<Version> {

This interface seems perfectly acceptable to me. I can’t think of a better one.

I do have some concerns making it stable immediately though, since nothing in the compiler itself actually depends on SemVer. Given that Cargo does, I would recommend, pending uint/u32 questions, that we move all of SemVer to ‘unstable’, pending some more real-world testing.

Thoughts?

[huon]

I agree about using u32, although a Numeric might make sense to be stored as a u64 e.g. a build ID of 201408131459 (the date+time) would overflow a 32-bit number. Not sure.

This interface seems perfectly acceptable to me. I can’t think of a better one.

I don’t know if this is better, but have you considered Result<Version, ParseError> with an informative ParseError enum? As it stands, it’s hard to tell users why a SemVer parse failed, e.g. I believe cargo just gives a nonspecific “could not parse ‘…’ as a semver” error.

[steveklabnik]

… that sounds great. And funny, because I was also thinking about looking at methods elsewhere that return Options that really should be Results.

I’ll investigate that further, but seems like the right thing to do

[seanmonstar]

I’ve been thinking it’d be nice to move Cargo’s version comparing with its lockfile into semver itself. Not the lockfile, but checking if a version matches a version rule.

[steveklabnik]

Do you mean something more elaborate than what’s there? They aleady support comparisons.

[steveklabnik]

I’ve got a branch up with both changes: https://github.com/steveklabnik/semver/tree/stabilize

However, there’s still just a GenericParseError error that could be more specific errors. I wonder if it’s worthwhile.

I also may write a Show implementation for it as well. Thoughts?

[seanmonstar]

I meant the version_req.rs in Cargo, that checks >=1.2.3 against 1.4.0 and returns true.

[steveklabnik]

Ah, Cargo is re-implementing a lot here:

let r = req("1.0.0");
assert!(r.to_string() == "= 1.0.0".to_string());

is

assert!(parse("1.0.0").to_string == "0.0.0".to_string()))

with SemVer.

[steveklabnik]

… that is actually incorrect. “1.0” is a valid req, but not a valid semver version. I agree with you, and so does @wycats. I’ll put that on the agenda.

[globin]

I’ve been hacking on some support for version ranges the last few days:

globin/semver

The basic structure is there but hardly any work has done on parsing the ranges and the code isn’t necesseraly that nice and worked out. i’m on the road today so I won’t be doing anything, maybe someone wants to continue from there, otherwise I’ll work on the rest during the next few days.

(VersionRange probably needs another attribute allow_dev: bool)

I think it should at least support those ranges (https://www.npmjs.org/doc/misc/semver.html) + maybe the syntax for Gemfiles if that’s compatible.

[steveklabnik]

That’s already been implemented as part of Cargo, and I have it ported over in my branch. I want to clean it up some though.

[steveklabnik]

I did a naieve port on top of my ‘stabilize’ branch: https://github.com/steveklabnik/semver/tree/range

All the version stuff is in one module, all the range stuff is in the other. Still more cleanup work to do, but I want to present the whole thing…

[seanmonstar]

With adding the range stuff, does that affect setting stable? What exactly does that even mean. The package will follow semver, such that breaking changes bump the major version?

[steveklabnik]

The semver module is currently ‘experimental.’ I would like to propose that we bump it up a notch, to 'unstable.' http://doc.rust-lang.org/rust.html#stability

This basically means it goes from “whoah this is in flux” to “this has settled down, but no guarantees yet.” Given that Cargo has been using both of these functionalities, I think that’s currently deserved. But I have to present my case. Initially, I only presented my case for what existed in the crate.

[steveklabnik]

Okay. On my range branch, I’ve implemented my proposal: https://github.com/steveklabnik/semver/tree/range

Here it is, from the top:

The semver crate has two modules, version and range.

semver::version

The Version struct represents a version.

pub struct Version {
    pub major: u32,
    pub minor: u32,
    pub patch: u32,
    pub pre: Vec<Identifier>,
    pub build: Vec<Identifier>,
}

A u32 should be big enough for everything.

An identifer represents the build metadata and prerelease information.

pub enum Identifier {
    Numeric(u64),
    AlphaNumeric(String),
}

We need a u64 here, because often a date is used as metadata, and those can be pretty large, number-wise.

pub fn parse(s: &str) -> Result<Version, ParseError>

This function creates a Version. I’m wondering if it should remain a free function, or if it should be a static method on Version. I think version::parse("1.0.0") is a pretty nice interface, but Version.parse("1.0.0") might too. This is kind of a taste thing. @aturon, what do you think?

pub enum ParseError {
    NonAsciiIdentifier,
    IncorrectParse(Version, String),
    GenericFailure,
}

Here’s the ways that a parse can fail right now. I just made GenericError to catch a few other kinds, if we like the way this proposal is going, I can do the work to figure out a better name, and/or break it up into two or three other kinds of errors. I’m not 100% sure how detailed we care to get here.

semver::range

range is a module imported from Cargo. @wycats supported the idea of pulling this out of Cargo and into SemVer. I’m calling it ‘range’ because it supports version ranges, but given that the only public thing in it is a VersionReq, maybe that would be a better name.

Also, VersionReq might be better as a VersionRequirement. That’s getting a bit long, though…

A VersionReq has no public fields, but some methods:

fn any() -> VersionReq

This produces a VersionReq that matches any other VersionReq, which is useful in many cases.

fn parse(input: &str) -> Result<VersionReq, String>

This is the equivalent of version::parse. I’m now noticing that this returns a String, maybe it should also be an enum. That this is a method might support turning version::parse into a method as well, or maybe the reverse.

fn exact(version: &Version) -> VersionReq

This checks to see if a VersionReq matches a Version exactly. Maybe this should be VersionReq == Version? Can that be done?

fn matches(&self, version: &Version) -> bool

This is the general match function, to see if a Version matches the range.

So, still some questions, but progress!

[seanmonstar]

This checks to see if a VersionReq matches a Version exactly. Maybe this should be VersionReq == Version? Can that be done?

PartialEq only compares self with Self. It’s possible to impl Equiv<Version> for VersionReq {}…

[steveklabnik]

Ahhh, this makes sense.

[steveklabnik]

Okay, well, I guess I’ll make a pull request.

[steveklabnik]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.