alternative registries stabilization

ashleygwilliams commented 5 years ago

This is a tracking issue. Representing the cargo team, @withoutboats has asked us to help with some items. This issue will keep track of those items, who is working on them, and their progress. This will be a recurring crates.io agenda item until the feature is stabilized.

(below is my brain dump, please edit!!!)

[ ] documentation
- [ ] how to use on cargo
- [ ] crates.io policy for how alt registries are leveraged (what can and cannot be published, and why)
[ ] crates.io publish policy checks should be implemented

withoutboats commented 5 years ago

cc @ehuss if you want to copy the checklist you made for the cargo team here that would be awesome! this issue is for tracking the work needed to stabilize the alternative registries feature

ehuss commented 5 years ago

Sure!

[x] Docs — I have written a chapter for the cargo reference that explains how to use registries, how they work, the index format, and the (minimum) web API. I'm thinking of holding off posting it until we're closer to being ready to stabilize it, but I can post it sooner if people want to see it. I definitely want crates-io team to review it, particularly the web API part, and anything else you want to add to it.
[x] crates.io support — Did you make a decision on https://github.com/rust-lang/crates.io/issues/1579? My preference would be to skip the whitelist for now, and just reject alt-reg deps. It sounds like people are leaning that way, too? Let me know if you want me to help with that. Just rejecting should be only a few lines of code, though I'm not familiar with the crates.io test suite.
[x] Index command — The RFC suggested adding a generate-index-metadata command, but I'm proposing implementing a general, yet simple cargo-index command/library out of tree. I expect to have that done soon.

Besides those 3 things, I don't see anything else left to do. I'll try to do some more final testing and try to find edge cases. There's also https://github.com/rust-lang/cargo/issues/4688, but I'm satisfied with the status of everything raised, so I don't think there's anything else to do there. I also think it might be helpful to ping other interested parties who are already using it to see if they have any final input.

I'm proposing to aim for stabilizing in 1.34 which lands on stable April 11. That means stabilizing on nightly sometime in late January. Does that sound reasonable?

sgrif commented 5 years ago

Did you make a decision on rust-lang/crates.io#1579? My preference would be to skip the whitelist for now, and just reject alt-reg deps. It sounds like people are leaning that way, too?

I agree, I'll bring it up in the next meeting to make sure we have consensus.

Does the index command require anything on the crates.io side?

ehuss commented 5 years ago

OK. And to be clear, I updated the issue, I don't think crates.io needs to do anything. Cargo already refuses to publish with alt-registries to crates.io.

I don't think so regarding the index command. The only thing I'll ask of the team is to review the alt-registry docs when they are posted (probably later this month).

sgrif commented 5 years ago

OK. And to be clear, I updated the issue, I don't think crates.io needs to do anything. Cargo already refuses to publish with alt-registries to crates.io.

We should still reject it on our side. We've had issues in the past where Cargo was checking things that we weren't, Cargo changed, but we still wanted to be rejecting those things. We've actually been meaning to do an audit of anything Cargo is checking that we aren't

jtgeibel commented 5 years ago

We should still reject it on our side.

I definitely agree, we don't want to rely on client-side checks. There could be other tools that attempt to package or upload crates.

I had started a patch before the break but hadn't had a chance to test it in staging. Now that I have, things don't appear to be working the way I expect.

I haven't been able to verify yet (as I'm testing over HTTPS so far), but I don't think cargo is sending the alternative registry in the dependency list within the metadata provided to the publish URL. It appears that the information is instead (or at least additionally) set in the generated Cargo.toml within the tarball.

# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# "normalize" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g. crates.io) dependencies
#
# If you believe there's an error in this file please file an
# issue against the rust-lang/cargo repository. If you're
# editing this file be aware that the upstream Cargo.toml
# will likely look very different (and much more reasonable)

cargo-features = ["alternative-registries"]

[package]
name = "test-crate"
version = "0.1.3"
authors = ["Justin Geibel <...>"]
description = "Test app"
license = "MIT"
[dependencies.build-info-test]
version = "0.1.1"
registry-index = "https://github.com/jtgeibel/crates.io-index"

Here is the contents of Cargo.toml.orig within the tarball:

cargo-features = ["alternative-registries"]

[package]
name = "test-crate"
version = "0.1.3"
authors = ["Justin Geibel <...>"]
description = "Test app"
license = "MIT"

[dependencies]
build-info-test = { version = "0.1.1", registry = "staging" }

Note that values in .cargo/config map my staging registry in the original TOML file to its full URL which is then included in the generated TOML file.

@ehuss do you know if this is the indented behavior? It appears that cargo may be saving this information within the tarball, rather than providing it in the metadata so that crates.io can include it in the index. I figured maybe there was an issue with my test setup, since I was using the same URL for the alternative registry in my config and on the command line via cargo publish --index https://github.com/jtgeibel/crates.io-index --token .... I then tried changing the "alternative" registry to point to the official index, changed the dependency to lazy_static, and tried to publish to my alternative registry. This then resulted in the following error message from cargo, which seems to be triggering the client side test with a misleading error message suggesting that I'm trying to upload lazy_static to crates.io.

error: crates cannot be published to crates.io with dependencies sourced from other
registries either publish `lazy_static` on crates.io or pull it into this repository
and specify it with a path and version
(crate `lazy_static` is pulled from registry `https://github.com/rust-lang/crates.io-index`)

ehuss commented 5 years ago

I checked out your branch and it seems to work for me. I got this error: error: api errors: Dependency `a1` is from an alternative registry. Depending on alternative registries is not allowed on crates.io.

I would ignore the Cargo.toml files, Cargo mostly does. The registry field should be set in the /new json data.

The way I tested it:

Created an index on my filesystem to represent my alternative registry, with a config.json file.
Added a crate to it.
Created a new crate with a registry value pointing to this alt registry.
Set up my local crates.io as a second registry ("ioalt").
cargo publish --registry=ioalt -Zunstable-options --allow-dirty

Can you show me your .cargo/config, Cargo.toml, and the command you used to publish with lazy_static? I want to change that crates.io detection function, and I want to make sure I understand how you triggered it.

I definitely agree, we don't want to rely on client-side checks.

Worst-case, someone uploads something with dependencies that point to the wrong crate, or it is rejected because the dependencies aren't found in the crates.io index.

jtgeibel commented 5 years ago

It looks like this is interacting strangely with cargo publish --index URL. When trying with cargo publish --registry staging -Zunstable-options I see the expected response from the API.

Here's is my .cargo/config, including a staging registry pointing to my index and an alias pointing to the crates.io index. (It's a bit confusing that my repo is named so similarly to the official one, but it is not a clone. It is a small index with a handful of crates, with its config file pointing to my staging instance on Heroku.)

[registries.staging]
index = "https://github.com/jtgeibel/crates.io-index"

[registries.crates-io-alias]
index = "https://github.com/rust-lang/crates.io-index"

Here is the Cargo.toml for the 2nd example, where I received the strange error:

cargo-features = ["alternative-registries"]

[package]
name = "test-crate"
version = "0.1.3"
authors = ["Justin Geibel <...>"]
description = "Test app"
license = "MIT"

[dependencies]
lazy_static = { version = "1.2.0", registry = "crates-io-alias" }

Here is the command I'm using when I see the error:

$ cargo publish --index https://github.com/jtgeibel/crates.io-index --token 123
    Updating `https://github.com/jtgeibel/crates.io-index` index
error: crates cannot be published to crates.io with dependencies sourced from other
registries either publish `lazy_static` on crates.io or pull it into this repository
and specify it with a path and version
(crate `lazy_static` is pulled from registry `https://github.com/rust-lang/crates.io-index`)

Switching to specifying the unstable registry option:

cargo publish --registry staging -Zunstable-options --token 123
    Updating `https://github.com/jtgeibel/crates.io-index` index
warning: manifest has no documentation, homepage or repository.
See http://doc.crates.io/manifest.html#package-metadata for more info.
   Packaging test-crate v0.1.4 (/home/jtgeibel/tmp/cargo/test-crate)
   Verifying test-crate v0.1.4 (/home/jtgeibel/tmp/cargo/test-crate)
    Updating crates.io index
   Compiling lazy_static v1.2.0
   Compiling test-crate v0.1.4 (/home/jtgeibel/tmp/cargo/test-crate/target/package/test-crate-0.1.4)
    Finished dev [unoptimized + debuginfo] target(s) in 0.58s
   Uploading test-crate v0.1.4 (/home/jtgeibel/tmp/cargo/test-crate)
error: api errors: Dependency `lazy_static` is from an alternative registry.  Depending on alternative registries is not allowed on crates.io.

I would ignore the Cargo.toml files [in the tarfiles], Cargo mostly does.

I am curious as to why cargo is recording this information here though. It wouldn't expect cargo to use it for anything, since it may not yet have the tarball when resolving dependencies. Maybe I'm missing some use case.

ehuss commented 5 years ago

Thanks, I've posted a PR at https://github.com/rust-lang/cargo/pull/6525 to better support --index.

why cargo is recording this information here though

It's complicated, and has flipped back and forth over time as to what it is used for. The resolver uses the index, but other parts of cargo expect a full manifest. I just added the registry field to cargo metadata a few days ago, and it was easier to use the information from Cargo.toml.

jtgeibel commented 5 years ago

Thanks, I've posted a PR at rust-lang/cargo#6525 to better support --index.

And it looks like this already landed! Thanks @ehuss. I tested with a local build and I'm no longer seeing any of the issues I was running into. I've opened a PR for the crates.io double-check.

ehuss commented 5 years ago

Proposal to stabilize has been posted at https://github.com/rust-lang/cargo/issues/6589

pietroalbini commented 5 years ago

@steveklabnik were you supposed to close this?

steveklabnik commented 5 years ago

No, I was not. I don't know how that happened :(

ehuss commented 5 years ago

This should have been closed, but bors didn't have permission here, and it fell through the cracks.

Stabilized and documented at rust-lang/cargo#6654 Documentation is at https://doc.rust-lang.org/nightly/cargo/reference/registries.html The current policy is to reject all alternative registries (https://github.com/rust-lang/crates.io/pull/1589) and is documented in the cargo reference. If a policy change is desired in the future, then I think a new issue should be opened.

rust-lang / crates-io-cargo-teams

alternative registries stabilization #21