Reserved crate test doesn't check the [lib] key, which is the actual important part

[Manishearth]

We have a bunch of reserved crates, many of which come from internal rustc crates that Rust ships with. Using one of these names has the potential to cause a "multiple sources for dependency" clash.

However, we don't check the [lib] key in Cargo.toml, so you can still name a crate one thing and clash its actual crate name, which will lead to the same kind of problem.

This seems to be an oversight, and we should at least warn for these (ideally, error for new publishes)

[Manishearth]

Going through https://gist.github.com/ehuss/7cb353e318b01ddc86192fb44d473201 from #6827, the crates that currently break this are:

• a bunch of rustc-ap-foo autopublish crates maintained by rust that help rustfmt (also pulled in by racer)
• tester, which only compiletest_rs uses (I have a PR open to fix tester)
• debugln and uc, which nobody uses
• rust-libcore, which volatile_cell uses (it has no reverse deps and is very old)

We can coordinate with the autopublish folks so that they rename their libs, publish a major bump, and update in rustfmt. I co-maintain compiletest, so that can be fixed. Once those are fixed we could make it a hard error to publish new versions with reserved names in the [lib]. That can be done now as well (it's not a breaking change, it just forces crates to make a breaking change for any future publishes), but in the interest of making it not too surprising ideally we work with the autopublish folks to make it smooth.

[Manishearth]

The autopublish stuff seems to have already been fixed since that gist was created (https://docs.rs/crate/rustc-ap-arena/628.0.0/source/Cargo.toml, https://docs.rs/crate/rustc-ap-syntax/628.0.0/source/Cargo.toml )

[carols10cents]

So Cargo doesn't currently send the [lib] name in the metadata about the crate. Given that we can't change released Cargos to start sending this information, we need to look at the Cargo.toml in the tarball.

We currently do unpack the tarball to validate that it's well-formed, but we don't read the content of any of the files. This currently happens on the main thread, not in a background job.

So the bad news is we need to start reading a file's contents, and we should probably be doing that in a background job. The good news is this change should enable us to add other features. Eventually, we could probably stop using the metadata at all (but Cargo should probably still send it if other registries are using it?)

We'll also need to check all existing crates' lib names to make sure we've caught them all (and yank them? they likely don't work as expected today anyway)

[Manishearth]

We shouldn't yank them; they kind of work, it's just super brittle. non-nightly compiletest clients seem to have been doing fine with the tester crate.

I think we should just forbid subsequent publishes.

[sgrif]

I think we should assume this check will eventually be async, even if it's synchronous at first. In terms of UI for this, we could perhaps change Cargo to periodically poll an endpoint (let's say once per second) and not exit until that endpoint indicates a crate has successfully been processed or a timeout is reached.

This would also have the benefit of making it so cargo publish returning does mean that a crate can be immediately used from Cargo.toml

[carols10cents]

I think we should assume this check will eventually be async, even if it's synchronous at first. In terms of UI for this, we could perhaps change Cargo to periodically poll an endpoint (let's say once per second) and not exit until that endpoint indicates a crate has successfully been processed or a timeout is reached.

This would also have the benefit of making it so cargo publish returning does mean that a crate can be immediately used from Cargo.toml

It's already the case that cargo publish returning doesn't mean that a crate can be immediately used from Cargo.toml because the index updates are happening in a background job.

I understand why you're mentioning changes to Cargo here, but I don't think the potential changes in Cargo are related to the fix for this issue (especially because we can't change old Cargoes). Just wanted to make that clear for anyone else reading.