cargo icon indicating copy to clipboard operation
cargo copied to clipboard

`cargo add` refuses to add new non-vendored packages due to source replacement

Open k3d3 opened this issue 2 years ago • 10 comments

Problem

When a vendored directory is established, cargo add no longer adds new packages. Instead, it tries to translate a package name into a package that already exists in the vendored directory.

In this case, I've added xml-rs, then proceeded to vendor my dependencies. Now, no matter what I try to cargo add, it always tries to add xml-rs.

I've even tried to run cargo +beta add cbindgen --registry crates-io, and it still gives me the same behaviour. The only way I can get cargo add to work is to remove .cargo/config.toml file and place it back afterwards.

image

Steps

Commands to run:

cargo new example
cd example
cargo +beta add xml-rs  # The same behaviour is exhibited in +nightly as well
cargo vendor ./vendor

# (add the output to .cargo/config.toml)

cargo +beta add --build cbindgen  # Doesn't need to be a build dependency, and "cbindgen" can be anything, even crates that don't exist
cargo +beta add cbindgen --registry crates-io  # This also does not work

Contents of .cargo/config.toml:

[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "./vendor"

Possible Solution(s)

I assume the reason for this is because what's added in .cargo/config.toml is source replacement, meaning any dependencies that try to access crates-io will access the vendored directory instead. I think I understand why cargo add does what it does here, however I feel it's fairly unintuitive combined with vendoring.

I haven't figured out any way to work around this (at least, in a way that I can continue to use cargo add). I've tried to add to config.toml:

[source.actual-crates]
registry = "https://github.com/rust-lang/crates.io-index"

(I've also tried git in place of https, and adding .git to the end of the URL)

and if I try to use cargo add cbindgen --registry actual-crates, I get:

error: source `actual-crates` defines source registry `crates-io`, but that source is already defined by `crates-io`
note: Sources are not allowed to be defined multiple times.

If I put the following into cargo.toml:

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

and run the same cargo add command, I get the same behaviour:

warning: translating `cbindgen` to `xml-rs`
      Adding xml-rs v0.8.4 to dependencies.

For now, I can easily add my dependencies by modifying Cargo.toml myself, however the cargo add command is very convenient, especially when introducing Rust to a team that's new to the language.

Notes

While I am identifying this as a bug (or at least, a problem), I'm not sure what the best way is to fix it.

Making cargo add always grab from crates.io, despite a configured source replacement, feels like the wrong way to go about it because it has the potential to break other setups, especially ones that use mirrored (or alternative) registries.

Perhaps a flag could be given to cargo add? Maybe cargo add --no-source-replacement or something less wordy?

Alternatively, maybe there should be a new command cargo vendor-add that calls cargo-add code, but without the source replacement? Though that would likely have to be special-cased only to "directory" source-replacement, or you'd need to pass the vendor path as well, like you do with cargo vendor.

Version

I've tested this on both beta and nightly cargo.

Beta's output:
cargo 1.62.0-beta.3 (4751950cc 2022-05-27)
release: 1.62.0-beta.3
commit-hash: 4751950ccc948c07047e62c20adf423d7e5f668c
commit-date: 2022-05-27
host: x86_64-pc-windows-msvc
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:Schannel)
os: Windows 10.0.19044 (Windows 10 Pro) [64-bit]

Nightly's output:
cargo 1.63.0-nightly (38472bc19 2022-05-31)
release: 1.63.0-nightly
commit-hash: 38472bc19f2f76e245eba54a6e97ee6821b3c1db
commit-date: 2022-05-31
host: x86_64-pc-windows-msvc
libgit2: 1.4.2 (sys:0.14.2 vendored)
libcurl: 7.83.1-DEV (sys:0.4.55+curl-7.83.1 vendored ssl:Schannel)
os: Windows 10.0.19044 (Windows 10 Pro) [64-bit]

k3d3 avatar Jun 04 '22 00:06 k3d3

It looks like this happens because a fuzzy search for a PathSoruce returns all packages and cargo_add enables fuzzy searching in get_latest_dependency.

The simple solution would be to turn off fuzzy searching but I don't know if that is the correct course of action as it would stop linked_hash_map from being turned into linked-hash-map.

Muscraft avatar Jun 04 '22 02:06 Muscraft

Hmm, yeah that would at least get rid of the warning message, though you're right, that would cause other issues.

Though even if that were changed, wouldn't that cause cargo add to outright fail, rather than adding the crate dependency to Cargo.toml?

k3d3 avatar Jun 04 '22 03:06 k3d3

Yeah it would cause it to outright fail. I think throwing an error here is the best course of action as bypassing a source replace feels wrong as it wouldn't be vendored then. You would have to run cargo add then cargo vendor ./vendor.

I personally think throwing an error if their is a source replace may be the best course of action and then have a community subcommand to deal with vendor. This is just my opinion and one of the maintainers will probably have a better idea for the path forward.

Muscraft avatar Jun 04 '22 03:06 Muscraft

I'm a bit annoyed with the disparate implementations of fuzzy searching. Its difficult to provide a consistent user experience and is easy to make mistakes like this where you assume "its a registry, of course fuzzy searching is safe" when its easy to overlook the interaction with other features like source replacements.

Fundamentally, I think that is what needs to be fixed though to fix that would probably require a major refactoring to keep the current, expected behavior working. This would also make solving #10680 less ambiguous

As for vendoring in general, we had not yet fleshed out that use case (see #10472). Mind creating a separate issue for this? I had missed that when creating the initial batch of cargo-add issues.

epage avatar Jun 04 '22 12:06 epage

Sure! So just to make sure I'm on the same page, this issue should be related to the fuzzy searching, and a new issue should be made for the cargo-add-with-vendoring use case?

Do you want me to rewrite this current issue, or keep it as-is?

k3d3 avatar Jun 05 '22 21:06 k3d3

So just to make sure I'm on the same page, this issue should be related to the fuzzy searching, and a new issue should be made for the cargo-add-with-vendoring use case?

Yes, let's keep this issue about the fuzzy searching

epage avatar Jun 06 '22 13:06 epage

I'd propose we do the following

  1. Combine the query with fuzzy query methods, where separate
  2. Switch from a bool to an enum (Strict, Fuzzy)
  3. Add the following cases to the enum: Normalize and Suggestions
  4. Explore replacing Fuzzy with a mixture of other enum cases

epage avatar Jun 14 '22 19:06 epage

#10883 was a refactor that laid the ground work for fixing this.

Remaining steps:

  • Add QueryKind::Normalized
    • Initially, we can make it like Exact for path sources and like Fuzzy for registry sources
  • Switch cargo-add to using that Kind everywhere (both where Exact and Fuzzy are used)
  • A test to ensure this fixed it
  • Rename Fuzzy to Alternatives, Suggestions or something else to clarify its actual intent

epage avatar Jul 21 '22 15:07 epage

I am presently running into this issue and I have no idea how to find my way out.

JosiahParry avatar Sep 04 '23 22:09 JosiahParry

claim first, question it later~ @rustbot claim

LuuuXXX avatar Dec 05 '23 06:12 LuuuXXX