Fix publishing renamed dependencies to crates.io #5993
Conversation
This commit fixes publishing crates which contain locally renamed dependencies to crates.io. Previously this lack of information meant that although we could resolve the crate graph correctly it wouldn't work well with respect to optional features and optional dependencies. The fix here is to persist this information into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo to correctly deduce feature names as it does when it has `Cargo.toml` locally. A dual side of this commit is to publish this information to crates.io. We'll want to merge the associated PR (link to come soon) on crates.io first and make sure that's deployed as well before we stabilize the crate renaming feature. The index format is updated as well as part of this change. The `name` key for dependencies is now unconditionally what was written in `Cargo.toml` as the left-hand-side of the dependency specification. In other words this is the raw crate name, but only for the local crate. A new key, `package`, is added to dependencies (and it can be `None`). This key indicates the crates.io package is being linked against, an represents the `package` key in `Cargo.toml`. It's important to consider the interaction with older Cargo implementations which don't support the `package` key in the index. In these situations older Cargo binaries will likely fail to resolve entirely as the renamed name is unlikely to exist on crates.io. For example the `futures` crate now has an optional dependency with the name `futures01` which depends on an older version of `futures` on crates.io. The string `futures01` will be listed in the index under the `"name"` key, but no `futures01` crate exists on crates.io so older Cargo will generate an error. If the crate does exist on crates.io, then even weirder error messages will likely result. Closes rust-lang#5962
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
This commit is intended to be coupled with rust-lang/cargo#5993 where Cargo will start sending the registry more information about locally renamed crates to persist into the index.
|
r? @ehuss |
This is a radical thought for how to make sure that older cargos do not see this package, what if we rename the |
This commit is intended to be coupled with rust-lang/cargo#5993 where Cargo will start sending the registry more information about locally renamed crates to persist into the index.
|
@Eh2406 There's a trade off there: on the one hand, always erroring on data you don't understand makes sure you don't have any kind of spurious success; on the other hand, sometimes the program could've succeeded & now won't because it doesn't care about the name field (thinking of extension tools), but it still expects it to be there during deserialization. I think there's no risk of users downloading the wrong package, because if they get a different package named |
|
@Eh2406 an interesting though! I think it's certainly possible we could do that, although we've never really focused on this before. Historically all features we've added to Cargo which changed the index just made older Cargos confused. @withoutboats ah unfortunately checksums are only in place once you've got a lock file, typically this error would be hit when you're forming a lock file. In that sense I don't think the checksums protect us, but I don't think we need to bend over backwards too much here. |
|
@ehuss I think you mentioned on IRC, but does this strategy look ok to you? If so I'll get the crates.io PR merged and then we can merge this |
|
Ah, yea, this seems fine to me. 👍 There is the bigger question of schema compatibility and versioning to provide better error messages. However, that seems like a much bigger issue similar to the scope of RFC 2495, and I don't see that happening soon, and getting this bug fixed seems pretty important for this feature. |
This commit is intended to be coupled with rust-lang/cargo#5993 where Cargo will start sending the registry more information about locally renamed crates to persist into the index.
1487: Handle renamed dependencies coming from Cargo r=carols10cents a=alexcrichton This commit is intended to be coupled with rust-lang/cargo#5993 where Cargo will start sending the registry more information about locally renamed crates to persist into the index. Co-authored-by: Alex Crichton <alex@alexcrichton.com>
|
@bors: r=ehuss |
|
📌 Commit e82443d has been approved by |
|
⌛ Testing commit e82443d with merge 6a811f462b071663eba66f75c40968ed743113e1... |
|
💔 Test failed - status-appveyor |
|
@bors: retry |
Fix publishing renamed dependencies to crates.io This commit fixes publishing crates which contain locally renamed dependencies to crates.io. Previously this lack of information meant that although we could resolve the crate graph correctly it wouldn't work well with respect to optional features and optional dependencies. The fix here is to persist this information into the registry about the crate being renamed in `Cargo.toml`, allowing Cargo to correctly deduce feature names as it does when it has `Cargo.toml` locally. A dual side of this commit is to publish this information to crates.io. We'll want to merge the associated PR (link to come soon) on crates.io first and make sure that's deployed as well before we stabilize the crate renaming feature. The index format is updated as well as part of this change. The `name` key for dependencies is now unconditionally what was written in `Cargo.toml` as the left-hand-side of the dependency specification. In other words this is the raw crate name, but only for the local crate. A new key, `package`, is added to dependencies (and it can be `None`). This key indicates the crates.io package is being linked against, an represents the `package` key in `Cargo.toml`. It's important to consider the interaction with older Cargo implementations which don't support the `package` key in the index. In these situations older Cargo binaries will likely fail to resolve entirely as the renamed name is unlikely to exist on crates.io. For example the `futures` crate now has an optional dependency with the name `futures01` which depends on an older version of `futures` on crates.io. The string `futures01` will be listed in the index under the `"name"` key, but no `futures01` crate exists on crates.io so older Cargo will generate an error. If the crate does exist on crates.io, then even weirder error messages will likely result. Closes #5962
|
☀️ Test successful - status-appveyor, status-travis |
This commit fixes publishing crates which contain locally renamed dependencies
to crates.io. Previously this lack of information meant that although we could
resolve the crate graph correctly it wouldn't work well with respect to optional
features and optional dependencies. The fix here is to persist this information
into the registry about the crate being renamed in
Cargo.toml, allowing Cargoto correctly deduce feature names as it does when it has
Cargo.tomllocally.A dual side of this commit is to publish this information to crates.io. We'll
want to merge the associated PR (link to come soon) on crates.io first and make
sure that's deployed as well before we stabilize the crate renaming feature.
The index format is updated as well as part of this change. The
namekey fordependencies is now unconditionally what was written in
Cargo.tomlas theleft-hand-side of the dependency specification. In other words this is the raw
crate name, but only for the local crate. A new key,
package, is added todependencies (and it can be
None). This key indicates the crates.io package isbeing linked against, an represents the
packagekey inCargo.toml.It's important to consider the interaction with older Cargo implementations
which don't support the
packagekey in the index. In these situations olderCargo binaries will likely fail to resolve entirely as the renamed name is
unlikely to exist on crates.io. For example the
futurescrate now has anoptional dependency with the name
futures01which depends on an older versionof
futureson crates.io. The stringfutures01will be listed in the indexunder the
"name"key, but nofutures01crate exists on crates.io so olderCargo will generate an error. If the crate does exist on crates.io, then even
weirder error messages will likely result.
Closes #5962