VersionReq with build metadata ignored?

[alexcrichton]

This program produces surprising results (to me at least):

extern crate semver;                                          
                                                              
fn main() {                                                   
    let a = semver::VersionReq::parse("=1.0.1+1.7.3").unwrap()
    println!("{:?}", a);                                      
    let b = semver::Version::parse("1.0.1+1.7.5").unwrap();   
    println!("{:?}", b);                                      
    println!("{}", a.matches(&b));                            
}                                                             

The last line prints true, but I'd expect it to print false? It looks like the +1.7.3 isn't being parsed in a?

[alexcrichton]

(originally discovered through alexcrichton/cargo-vendor#21)

[alexcrichton]

(also feel free to close if this is expected)

[mbrubeck]

+1.7.3 is not a pre-release version. It is build metadata. According to the semver spec,

Build metadata SHOULD be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence.

[alexcrichton]

In light of that... is there any way to request a precise version through a VersionReq 100% of the time?

Basically, given a Version, how is a VersionReq constructed that only matches that Version?

[mbrubeck]

Basically, given a Version, how is a VersionReq constructed that only matches that Version?

This is not possible with the current version of the semver crate, or with NPM-style semver ranges generally. Build metadata is simply ignored, so it will never distinguish versions that differ only by build metadata.

We could add a way to do this by inventing a new range operator for metadata-sensitive ranges. For example if the new operator was === then you could write a range ===1.0.0+foo that would only match version 1.0.0+foo.

[jsgf]

Does that mean that crates.io should disallow packages whose versions differ only by build metadata?

[alexcrichton]

Perhaps yeah, we don't seem to have many other options here

[sgrif]

This seems like it's misreading the spec to me. Metadata is ignored for precedence, but that does not mean it is ignored when determining equality.

Build metadata MUST be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence. Examples: 1.0.0-alpha+001, 1.0.0+20130313144700, 1.0.0-beta+exp.sha.5114f85.

Precedence refers to how versions are compared to each other when ordered.

It seems to me this would mean that sorting [1.0.0, 1.0.0+asdf, 1.0.0+otherbuild] can result in any order, and the VersionReq 1.0.0 would match all of those, but =1.0.0+asdf would not match 1.0.0+otherbuild, and =1.0.0 would not match =1.0.0+asdf or =1.0.0+otherbuild

[dtolnay]

I believe this behavior is consistent with how build metadata is intended to work, for better or worse. https://github.com/semver/semver/blob/efcff2c838c9945f79bfd21c1df0073271bcc29c/ranges.md is pretty clear about this:

"Implementations MAY reject Ranges with SemVer strings that include build metadata, but if they do not, then they MUST ignore any build metadata when evaluating versions for inclusion."

Therefore a requirement like =1.0.1+1.7.3 matches all the various versions 1.0.1, 1.0.1+1.7.3, 1.0.1+1.7.5 etc.

If Cargo wants to prioritize a different one of those matching versions when resolving such a requirement, I think that would be a separate action from matching. My suggestion would be something like:

use semver::{Op, Version, VersionReq};

fn main() {
    let req = "=1.0.1+1.7.3";
    let v = "1.0.1+1.7.5";
    println!("{}", matches(req, v));
}

fn matches(req_str: &str, version_str: &str) -> bool {
    let req = VersionReq::parse(req_str).unwrap();
    let v = Version::parse(version_str).unwrap();

    let same_patch_different_build = || {
        if req.comparators.len() != 1 {
            return false;
        }
        let cmp = &req.comparators[0];
        cmp.op == Op::Exact
            && cmp.major == v.major
            && cmp.minor == Some(v.minor)
            && cmp.patch == Some(v.patch)
            && cmp.pre == v.pre
            && !req_str.ends_with(version_str)
    };

    req.matches(&v) && !same_patch_different_build()
}

That said, this crate is now explicitly scoped as implementing Cargo's interpretation of SemVer, so if the suggestion above is not appealing or feasible for Cargo, then I would be prepared to change the behavior of matching to fit how Cargo chooses to interpret SemVer.