regression: * deps no longer parsed

[Manishearth]

On cargo 0.13.0-nightly (c205132 2016-08-09), deps like 0.2* no longer parse.

I'm not sure if these are valid, but they used to work, e.g on stable (cargo 0.11.0-nightly (259324c 2016-05-20))

Minimal example:

[package]

name = "foo"
version = "0.3.0"

[lib]
name = "foo"
path = "src/lib.rs"

[dependencies]
plugin = "0.2*"

works fine on stable, gives the following on nightly:

$ cargo build --verbose
error: failed to parse manifest at `/Users/manishearth/sand/bar/tempcrate/Cargo.toml`

Caused by:
  you have already provided an operation, such as =, ~, or ^; only use one

[steveklabnik]

They are not valid per semver, you need the .

[Manishearth]

Can we then phase them out slowly? This is a breaking change that affects people pinning to iron 3 (e.g. rust-playpen).

[steveklabnik]

It's going to be a giant pain for me.

We could always revert cargo back to a previous semver version as a quick fix.

[steveklabnik]

So, thinking more: I could backport a warning to the 2.x series of semver, we could make cargo use that instead. That probably wouldn't be too bad, though part of the reason why I re-wrote all that code was because it is a giant mess...

[Manishearth]

Yeah, I don't want to muck up semver's code too much. But I'm not sure how this can be done on the pure-cargo side either.

Perhaps a crater run would help us gauge the full impact and determine if this is necessary? @brson

[steveklabnik]

So, another thing:

I currently do a regression test across all of crates.io: https://github.com/steveklabnik/semver/blob/master/tests/regression.rs

But note that's just for the version, not for dependencies. it looks like that's supported https://frewsxcv.github.io/rust-crates-index/crates_index/struct.Dependency.html

So I could also figure it out that way, and probably should add that regardless.

[steveklabnik]

So, uh https://travis-ci.org/steveklabnik/semver/jobs/152783177

That is, this seemgly passed on semver master for every crate on crates.io.

@Manishearth I can't actually find a released iron that has this dep. v3.0: iron/iron@16c858e

Shows them actually fixing this. And it looks like 2.6 wasn't tagged?

[steveklabnik]

I used cargo-clone to grab iron 2.6, and it does have 0.2*

So, now I'm confused about my tests :(

[steveklabnik]

okay so https://travis-ci.org/steveklabnik/semver/jobs/152791793

testing crate iron at version: 0.2.6

testing dependency: ^0.1

testing dependency: ^0.2

testing dependency: ^0.7

testing dependency: ^0.3

testing dependency: 0.2.*

testing dependency: ^0.3

testing dependency: ^0.1

testing dependency: ^0.5

testing dependency: ^0.1

testing dependency: ^0.7

apparently, the crates-index crate is not returning the same contents as the actual cargo.toml.

Looking at https://github.com/rust-lang/crates.io-index/blob/master/ir/on/iron#L47

{"name":"iron","vers":"0.2.6","deps":[{"name":"modifier","req":"^0.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"num_cpus","req":"^0.2","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"conduit-mime-types","req":"^0.7","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"log","req":"^0.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"plugin","req":"0.2.*","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"typemap","req":"^0.3","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"error","req":"^0.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"url","req":"^0.5","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"lazy_static","req":"^0.1","features":[],"optional":false,"default_features":true,"target":null,"kind":"normal"},{"name":"hyper","req":"^0.7","features":[],"optional":false,"default_features":false,"target":null,"kind":"normal"},{"name":"time","req":"*","features":[],"optional":false,"default_features":true,"target":null,"kind":"dev"}],"cksum":"6d02706746b2d95c50f463fc619de64efe10682adfa17fa18dc95e0a92b10237","features":{"default":["ssl"],"ssl":["hyper/ssl"]},"yanked":false}

This also has 0.2.*.

@alexcrichton do you have any idea what's going on here?

[steveklabnik]

From IRC:

20:43 <~acrichto> steveklabnik: I think we round trip
20:43 <~acrichto> in the sense of we parse the requirement
20:43 <~acrichto> and then when we emit the json we emit that requirement to a string
20:43 < steveklabnik> would that be in cargo or in crates.io?
20:45 <~acrichto> steveklabnik: oh probably both actually
20:45 <~acrichto> as in, Cargo's sending requests to crates.io
20:45 <~acrichto> but crates.io is then re-parsing and re-encoding

So yeah, @brson a crater run to check this out might be a good idea. idk. uuuugh

[steveklabnik]

That said, this change has been in nightly for a while, and we also haven't seen many people file issues about breakage. So maybe things are okay? I dunno 😦

[alexcrichton]

Hm ok, so I think we'll want to revert this for now and go back to the old semver version. @steveklabnik could you send a PR for that? Otherwise I can do so later tonight.

A similar situation here is that I fixed a parsing bug in TOML but Cargo's still issuing warnings about it. From time to time I still see that url's 0.5.* manifest doesn't parse correctly, so it'll take awhile to flesh all this out.

[steveklabnik]

@alexcrichton

It doesn't look like you ended up doing it, so let me see if I can cut a new version of the older semver with some kind of warning about this. If I can't get it done quickly, I'll just send it for the exiting version.

so it'll take awhile to flesh all this out.

Yeah, we should talk about strategies here.

[alexcrichton]

@steveklabnik

So to clarify what happened with TOML, awhile ago the parser was discovered to have a bug where it accepted invalid syntax. This was found in the wild as part of url-0.5.9. To enable a smooth transition, I implemented an option in the TOML parser to accept the old, broken, syntax.

When toml was updated I added some logic which basically does:

• Attempt to parse TOML with the "real" parser. If that succeeds, we're good.
• Attempt to parse TOML with the old/broken parser. In this case flip a flag in the parser. If that succeeds, issue a warning but we're otherwise good.
• Otherwise, return the first error.

Could something like that be done for the semver crate as well? E.g. have a temporary option (off by default) to accept the old syntax?

[steveklabnik]

Conceptually, we can. It might make the crate's deps heavier, but I imagine most non-trivial programs will already have a regex dep...

On Aug 17, 2016, 13:55 -0400, Alex Crichton notifications@github.com, wrote:

@steveklabnik (https://github.com/steveklabnik)

So to clarify what happened with TOML, awhile ago the parser was discovered to have a bug (toml-rs/toml-rs#94) where it accepted invalid syntax. This was found in the wild as part of url-0.5.9. To enable a smooth transition, I implemented an option in the TOML parser (toml-rs/toml-rs@1ed6801) to accept the old, broken, syntax.

When toml was updated (https://github.com/rust-lang/cargo/pull/2680/files) I added some logic (

cargo/src/cargo/util/toml.rs

Lines 167 to 185 in 5593045

	let mut first_parser = toml::Parser::new(&toml);
	if let Some(toml) = first_parser.parse() {
	return Ok(toml);
	}
	let mut second_parser = toml::Parser::new(toml);
	second_parser.set_require_newline_after_table(false);
	if let Some(toml) = second_parser.parse() {
	let msg = format!("\
	TOML file found which contains invalid syntax and will soon not parse
	at `{}`.
	The TOML spec requires newlines after table definitions (e.g. `[a] b = 1` is
	invalid), but this file has a table header which does not have a newline after
	it. A newline needs to be added and this warning will soon become a hard error
	in the future.", file.display());
	try!(config.shell().warn(&msg));
	return Ok(toml)
	}

) which basically does:

Attempt to parse TOML with the "real" parser. If that succeeds, we're good.
Attempt to parse TOML with the old/broken parser. In this case flip a flag in the parser. If that succeeds, issue a warning but we're otherwise good.
Otherwise, return the first error.

Could something like that be done for the semver crate as well? E.g. have a temporary option (off by default) to accept the old syntax?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (#3001 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AABsilqpK2NCy8iieMKI49YxREdabNKAks5qg0rvgaJpZM4JlvM5).

[steveklabnik]

(and this would need coordination with cargo)

[alexcrichton]

@Manishearth note that we've pushed an upgrade of the semver dependency in #3154. I believe we've provided a transition path for any crates with these sorts of bad version requirements, but an extra pair of eyes is always helpful!

If you see any breakage, please be sure to let us know!

[Manishearth]

cc @nox, our resident overlord of versioning

[Manishearth]

I think iron was the only broken one for us, but that should have been fixed since then. Hopefully nothing breaks now.

Thanks!