Upgrade version of semver used

[alexcrichton]

The most recent version of the semver repository has fixed a few bugs in version requirements that it rejects. If we update to it, however, it causes breakage, so we had to revert the recent update.

Looks like the semver crate has fixed bugs recently to support new version requirements with build metadata.

I think what we'll need to do is to add a strategy like this where we continue to parse old version requirements, but warn about them.

@steveklabnik, could you add a feature to the semver crate to support old version requirements like 0.2*? Once we do that we can update Cargo to using that, warn about old broken requirements, and start picking up bug fixes.

[steveklabnik]

Yes, I have planned to, I just have not had the time. It's going to be a big pain, but it's manageable.

[steveklabnik]

I think I'm going to try to tackle this thursday or friday, depending on how the release goes.

[alexcrichton]

Ok, thanks @steveklabnik! If it's too onerous we could also just vendor something locally in Cargo like an old regex or something like that.

[steveklabnik]

That's precisely the issue: the version of semver we're using in Cargo currently has nom for Versions, and a hand-rolled parser for VersionReqs, so it's going from one parser (semver-parser, which is regex based) to three.

I have a plan (shove the older parser under an old module), it's just that it's not super simple, and tedious, so I haven't made it happen. But I also really want to get Cargo on the new semver; if I hadn't been travelling the last three weeks, I probably could have found the time. Now this week is the release...

[alexcrichton]

I think it's probably ok to not literally use the same parser, we can probably just scrape all Cargo.toml files on crates.io and parse them to learn about all "invalid" specs.

Would that be helpful to generate such a list?

[steveklabnik]

Oh yeah, is there an easy way to get all Cargo.toml files? I would love that for my own regression testing.

[alexcrichton]

My plan was to just download all *.crate files, extract all files named Cargo.toml, and then parse them. So I guess not exactly easy :)

[steveklabnik]

hehe.

well, yeah, if we can get that data set, I can maybe make it work more easily. If you can do it, that'd be good. Primarily, it would mean not bringing back the nom dependency, which would be nice. Not because nom isn't nice, but to keep cargo lighter.

[alexcrichton]

Ok, here's the list: https://gist.github.com/alexcrichton/97d3fa76b90e232eb61af37a59da91ad

I downloaded every crate file from crates.io, parse all files that looked like Cargo.toml, and then looked at all TOML strings in that file. If it succeeded with the previous semver parser and failed with the most recent one, I printed it out.

The panic there is presumably an older bug in semver, so can be ignored, and word-like ones like "x-smtpapi" I think are false positives (but are odd to be accepted by the old parser!).

In short it looks like the complete list of versions we need to keep accepting are:

.*
*.0
0.1.0.
0.2*
0.3.1.3

Those are spread across a whole mess of crates though

[steveklabnik]

So, on Friday I tried to run the script to reproduce the list, but got bad results. I'm guessing something is wrong. Regardless, looking at those examples:

• .* should be *.
• *.0 is a bug, imho. Apparently npm parses it as *: Possible parsing bug? npm/node-semver#171
• 0.1.0. should be 0.1.0.
• 0.2* should be 0.2
• 0.3.1.3 actually used to parse as 0.3.13! Fix semver dependency lensvol/strfile-rs#1

Given that these are such a small number of cases, I think we can just handle them individually.

I've implemented a fix here: https://github.com/steveklabnik/semver/tree/deprecation One commit: dtolnay/semver@aae8a45

This will stop us from having to bring back in the entire old parser.

What do you think? @alexcrichton could you try to run your script again against that branch? If so, I will prepare a real PR against semver and Cargo. We can't print out the warning in semver, since you want to respect cargo's settings; the idea is that i'll add a new error case, Deprecated, that still gives you the fixed version, and we can handle that logic similarly to TOML.

[alexcrichton]

@steveklabnik sounds reasonable to me, but perhaps this behavior could be disabled by default? I doubt anyone actually wants to publish something with 0.2* as a version requirement, so Cargo will at least want to warn about invalid version requirements even if it parses them.

[steveklabnik]

What would "disabled by default" mean? You mean in the Cargo side of the patch? I'm certainly open to it.

[alexcrichton]

Oh imagine that Cargo would actively go out of its way to parse these version requirements. General users of the semver crate shouldn't automatically opt-in to these cases.

I'd be fine adding this specific logic to Cargo itself as opposed to semver.

[steveklabnik]

Ah yes. let me prepare the real patch, I think this will work as you want it to.

[steveklabnik]

#3154 was just merged. Closing!