rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.72k stars 2.42k forks source link

`cargo add` should not silently remove any `path` attribute on dependencies and should support both `version` and `path` attributes #14510

Open rnza0u opened 1 month ago

rnza0u commented 1 month ago

Problem

Cargo supports having both version and path attributes for dependencies declared in Cargo.toml.

When using cargo add crate-name@x.x.x, any path attribute on the crate is getting silently removed.

Also, providing both a remote crate and a path attribute (like cargo add crate-name@x.x.x --path some/path) is not supported.

Proposed Solution

Cargo should not remove the path attribute on the dependency when using cargo add crate-name@x.x.x.

Also, providing both a remote and a path option at the same time should be supported, since Cargo supports having both in Cargo.toml.

I believe it used to be supported by cargo-edit (see in this examples section https://crates.io/crates/cargo-edit-9).

Notes

No response

rnza0u commented 1 month ago

I found a little workaround that solves part of the problem.

Passing a --path option to cargo add does not remove the version attribute, only the opposite.

So if you know in advance what it is, or if you can parse and analyze the TOML, you can do the following for updating a dependency version without removing the path attribute:

# update the crate (removes the path attribute)
cargo add crate-name@x.x.x
# reset the path attribute (does not remove the updated version attribute)
cargo add --path ../path/to/local/crate

Still, not the best experience in my opinion.

epage commented 1 month ago

The command-line allows you to update previous entries. For "source" information, we replace it as a whole, rather than in parts

Part of this is coming down to intent. We don't know whether cargo add crate@x.y.z is meant to specify the minimum version requirement for an existing path or be used instead of the path. There might be arguments to be had about the fact that its local so it doesn't make sense to remove the path. However, that is different with git and being inconsistent can be its own problem. There are also many times for me where the local path is outside the repo because adding a path to a dep was more direct than adding a patch.

rnza0u commented 1 month ago

Part of this is coming down to intent. We don't know whether cargo add crate@x.y.z is meant to specify the minimum version requirement for an existing path or be used instead of the path.

Don't you think it would be better to not remove the path ?

If the path does not exist on the local machine, then it is simply ignored by Cargo anyway.

However, that is different with git and being inconsistent can be its own problem. There are also many times for me where the local path is outside the repo because adding a path to a dep was more direct than adding a patch.

I'm not sure if I understand this part. Are you talking about git dependencies ?

epage commented 1 month ago

If the path does not exist on the local machine, then it is simply ignored by Cargo anyway.

I'm not seeing that locally

#!/usr/bin/env nargo
---
[dependencies]
clap = { version = "4", path = "hello_world" }
---

fn main() {
}
$ ./cargo-14510.rs
   Compiling cargo v0.84.0 (/home/epage/src/personal/cargo)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 15.52s
     Running `/home/epage/src/personal/cargo/target/debug/cargo -Zscript -Zmsrv-policy ./cargo-14510.rs`
warning: `package.edition` is unspecified, defaulting to `2021`
error: failed to get `clap` as a dependency of package `cargo-14510 v0.0.0 (/home/epage/src/personal/dump)`

Caused by:
  failed to load source for dependency `clap`

Caused by:
  Unable to update /home/epage/src/personal/dump/hello_world

Caused by:
  failed to read `/home/epage/src/personal/dump/hello_world/Cargo.toml`

Caused by:
  No such file or directory (os error 2)