New index schema and registry API for extended feature syntax. #9111
Conversation
The intent here is to make it more flexible to create different registry setups, and to reuse code a little more easily.
The intent here is to make it easier to work with API errors. This also includes some new tests for checking network errors.
This moves the new syntax for namespaced features and weak dependency
features into a new field ("features2") in the index and API. This
is necessary to avoid breaking Cargo versions older than 1.19,
which fail to parse the index even if there is a Cargo.lock file.
This switches the API url for publishing from `api/v1/crates/new` to `api/v2/crates/new` if the package contains the new features syntax. This is necessary because if the registry does not know about the new feature syntax, it will (likely) silently accept the uploaded package without the `features2` field, which will end up creating an index with an invalid entry (missing features).
This adds a `v` field which indicates a format version for an index entry. If Cargo encounters a version newer than it understands, it will ignore those entries. This makes it safer to make changes to the index entries (such as adding new things), and not need to worry about how older cargos will react to it. This version field is also part of the publish API. Version `2` indicates that it uses the new features field.
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I'm not too happy with this. So...which seems worse?
|
|
This includes a test which is ignored by default which can test the behavior of every toolchain. Here's what its output looks like:
This shows that without a Cargo.lock, every version will select a dependency with the new feature syntax (and will essentially build them as-if the features aren't activated). With a I'm unable to get test results for 1.0 through 1.5. For some reason they are unable to extract the tar files from the testsuite. I suspect it is something related to #2327, but my energy for investigating has run low. |
|
Thanks for putting this all together! I think this all seems pretty reasonable for "let's keep 1.18 and prior building with a lockfile", but I've still got a few questions.
I personally continue to not really be all that interested in worrying about older Cargo's picking up newer versions of crates. I feel like misinterpreting Cargo features is the least of our worries in that respect. I suspect almost no crates published in the last year even compile with Rust 1.0 due to feature usage of libstd or the language. Basically what I mean to say is that we never really designed Cargo in mind with I do think that it seems reasonable to have a version field in the index entry though. It's not harming anyone and it could help tooling later on down the road too. |
If you publish the new
It is not. I have not started that work, I wanted to see how this goes first. I'm hoping it is not too difficult. I'm also uncertain when we should do that, since that is permanently committing to the new format, and I'm not sure how to decide when that's ready.
That sounds reasonable. I'll need to take a look at the crates.io code to see how well that will fit in.
Yea, that seems like a reasonable stance. However, I was thinking that for a stabilization timeline, I'd like to see this PR in at least 3 versions back before stabilizing, to avoid too much frustration for people. That is, if this goes in 1.51, then the earliest it should be stabilized is 1.54. That allows people using stable-1 to continue to safely run I'm uncertain how quickly projects will jump on using this new syntax. Since it is an MSRV bump, I assume most will delay for a while, but I'd like to be a little cautious. |
|
Oh I was imagining that if we don't change the publish path that we wouldn't send
Actually on second though this may not be a great idea. I wouldn't want a malicious actor being able to place whatever they want into the index, so crates.io I think will want to maintain tight control over what goes into the index. That does mean, though, that if we bump the version and change the syntax we'll need to always update crates.io as well.
While I don't mind being conservative this does seem overly conservative to me. I don't feel this is really all that much different than a new language feature being stabilized. If a crate wants to work with stable-1 then everything it depends on has to have the same policy and no one would use this feature until it hits stable-1 anyway. In any case though if you'd prefer to be more conservative I don't mind! |
Oh, good point! Let me know if you have any other questions. This does seem like a semi-major change, but I'm uncertain of what else to consider. |
|
Here's a few more possible suggestions:
Other than that though I don't have much else on this either, so while I agree it's somewhat major it seems fine to go ahead and start rolling this out. This is unfortunately more work than I thought it would be to handle 1.19.... One option is to add the index entry versioning and we could add that ASAP to Cargo and even backport it to ensure at least that part hits stable as soon as it can? |
|
I dunno I'm also a little wishy-washy on the versioning in the index entry. I'm not really sure it buys us anything unless we actually have a plan to drop Rust versions at some point. If we don't ever drop Rust versions then we're always stuck with 1.18 and prior. Similarly we're always stuck with Cargo versions that don't look for an index entry version field. Given our current trajectory it seems like we'll never make use of this version field since we'll always be handling those older versions? |
|
A version field lets us change the meaning of fields that are newer then when the version field was added. It is limited. It can't change the presence of existing fields (while we support 1.19). It can't change the meaning of existing fields (while we support version before it was added). But even the little slice of flexibility seems useful. |
|
That's a good point yeah! While I'm not too sure how useful the ability will be in the future that I think convinces me that we may as well go ahead and implement it. |
Add some documentation for index and registry stuff. This adds some internal docs for index and registry things. Split out of #9111.
Add RegistryBuilder for tests, and update crates-io error handling. This adds `RegistryBuilder` to the test suite to make it more flexible to create different registry setups, and to reuse code a little more easily. This also makes a small adjustment to the registry API to add a `ResponseError` type to make it easier to work with API errors. As part of this, some tests were added to validate the API behavior for response errors. There are only a few very small changes here: * Extra newlines are removed from the headers printed in the error message. * The UTF-8 error now also includes the text "invalid response from server". * The "file too large" crates.io publish error now displays the tarball size. (There is no test for this because it is only issued for talking to `crates.io`.) Split from #9111.
|
☔ The latest upstream changes (presumably #9125) made this pull request unmergeable. Please resolve the merge conflicts. |
ehuss
removed
the
S-waiting-on-review
This introduces a new
features2field in the index to segregate the new feature syntax for namespaced features and weak dependency features. The intent here is to prevent versions of Cargo older than 1.19 from breaking, since they will fail to run even with aCargo.lockfile.To prevent publishing from breaking when talking to a registry that does not understand this new field, this switches the publish API to a new URL (
api/v2/crates/new).This also introduces a new schema field to the index and publish API, so that in the future changes can be made, and versions of cargo after this is introduced will silently ignore entries it doesn't understand.
This is split into a number of different commits, with more information in each commit.