Feedback on `cargo-add` before its merged

[epage]

In preparation for merging cargo-add into cargo, I want to solicit input on the CLI before we restrict its evolution by shipping it with cargo.

As a reminder, our options are

• Remove features (for now or forever)
• Make features unstable (require -Z to use)
• Re-work features

Background

cargo add allows you to add or modify dependencies in your Cargo.toml file.

Example commands

cargo add regex
cargo add regex serde
cargo add regex@1 --dev
cargo add regex@~1.0 --build
cargo add regex --vers 1.0 --optional
cargo add regex --upgrade minor  # for some reason this is the policy name for `^`
cargo add https://github.com/rust-lang/regex
cargo add regex --git https://github.com/rust-lang/regex
cargo add ../dependency
cargo add dep --path ../dependency

For full --help output, see the README

Particular points

• Effectively there are two modes
  • Fill in any relevant field for one package
  • Add multiple packages, erroring for fields that are package-specific (--vers, --features, etc)
• We infer if the dependencies table is sorted and preserve that sorting when adding a new dependency
• Adding a workspace dependency
  • dev-dependencies always use path
  • all other dependencies use version + path
• Re-add to modify an existing entry
  • No --rename will remove a prior --rename
  • --optional is never removed (cargo-edit#298)
  • Only explicit --features removes a prior one
  • --vers, --path, and --git overwrite each other

Known Points of Concern

1. This is feature rife with minutia that could become a maintenance burden for the cargo team

There are many different users with different needs and different formatting styles. We get a lot of feedback about trying to maintain the existing style or changing a default here or there.

Examples:

• "cargo add" should use dotted keys if already used in Cargo.toml #521
• cargo add: use predominant string style #217
• cargo add : include only MAJOR.MINOR #126

Proposal: We adopt a couple of guiding principles

• cargo-add is meant to help users on the default path; users always can edit Cargo.toml
• cargo-add should be opinionated in style but not get in the way of a user communicating intent
  • rustfmt is a prime example of this. A lot of syntax minutia is handled for you but you can still organize your code to clarify intent (e.g. whitespace, ordering, etc).
• Impact:
  • Use a consistent style run-to-run independent of the existing Cargo.toml (e.g. string and table styles)
  • Have a default version requirement style that users explicitly override per call (part of "default" path)
  • Sorting is something we will infer and preserve since that is tied more into the organization than in the minutia
  • Close out the above issues as wont-fix
  • We will not re-format existing entries but leave that to a future rustfmt for Cargo.toml (this means removing --sort)

2. --upgrade <policy> isn't quite clear

The policy names don't map well to what Rust developers are used to (operator characters) and the policy name for ^ isn't correct, calling it minor.

Proposal: Remove --upgrade <policy>

• Most people use or should use the default policy
• Redundant with --vers <req> and <name>@<req> except when no version is specified and cargo-add looks up the latest. If some does want to specify it, they are most likely to specify = which means they probably have a reason and a specific version in mind rather than "latest".

3. Is our story around pre-release strong enough to have --allow-preleease in the default path?

See Changing Cargo semver compatibility for pre-releases for one case where we need to improve on pre-release. With clap3, this was a frustration point.

Proposal: Remove the flag (or make it unstable)

• Pre-releases are uncommon enough that requiring people to explicitly set the version rather than "pick latest" seems sufficient
• Pre-releases can break from one to the next, so people probably should be explicit about what they are intending to use

4. The sourcing / constraining parts of the CLI don't seem quite polished

• --vers is redundant with @ and is a non-obvious name
  • Due to --version / -V flags though --version will go away when this is upstreamed into cargo
• We overload the positional argument between name, path, and git url
  • Have to disambiguate them
  • git url limits future evolution because we can't tell it apart from anything else
  • This leaves --path and --git as redundant
• <name>@<version-req> is common in external prior art but cargo tends to use : for pkg ids, maybe we should do the same

Proposal:

• As positional, accept zero or more <name>[:<version-req>] or <path>
• Accept --git <url>[#<type>=<ref>] where type is one of branch , tag, or rev
  • If possible, we auto detect the type from the remote and allow #<ref> (maybe only do this?)
  • I chose to use a fragment since its client side, it won't conflict with any requirements from the server
  • Will get name from remote if needed
  • Alternative, replace --git <url> with git+<url> positional like Poetry
• Remove --branch (now handled through fragment)
• --vers (specified as part of <name>[:<version-req>] or extracted from <path>. Chance of needing it in other cases is low

Prior Art

• (Python) poetry add
  • git+ is needed to specify git dependencies
  • Has dry-run, which we lack
  • Doesn't have separate --vers and --git flags
  • git branch is specified via a URL fragment, instead of a --branch
  • No --upgrade <policy> argument to control the version requirement operator
  • No --sort to force sorting of dependencies
• (Javascript) yarn add
  • name@data where data can be version, git (with fragment for branch), etc
  • -E / --exact , -T / --tilde , -C / --caret to control version requirement operator instead of --upgrade <policy> (also controlled through defaultSemverRangePrefix in config)
  • --cached for using the lock file ( cargo add : Be clever and infer version from Cargo.lock #41)
  • In addition to --dev , it has --prefer-dev which will only add the dependency if it doesn't already exist in dependencies as well as dev-dependencies
  • --mode update-lockfile will ensure the lock file gets updated as well
• (Javascript) npm doesn't have a native solution
• (Javascript) pnpm-add
  • Specify version with @<version>
  • Also overloads <name>[@<version>] with path and repo
    • Supports a git host-specific protocol for shorthand, like github:user/repo
    • Uses fragment for git ref, seems to have some kind of special semver syntax for tags?
  • Only supports --save-exact / -E for operators outside of the default
• (Go) go get
  • Specify version with @<version>
  • Remove dependency with @none
• (Haskell) stack doesn't seem to have a native solution
• (Julia) pkg Add
• (Ruby) bundle add
  • Uses --version / -v instead of --vers (we use --vers because of --version / -V )
  • --source instead of path ( path correlates to manifest field)
  • Uses --git / --branch like cargo-add
• (Dart) pub add
  • Uses --git-url instead of --git
  • Uses --git-ref instead of --branch , --tag , --rev

Status Update

(As of Feb 3, 2022)

You can check out the -h output or install it via cargo install --git https://github.com/killercup/cargo-edit cargo-edit

Summary of UI changes:

• --sort has been removed as part of decoupling reformatting from adding
• --upgrade <policy> has been removed due to the unclear interface and low probability of need
• --allow-prerelease has been removed awaiting further work on our pre-release story in cargo. It wasn't marked unstable because it is a relatively small feature that seems to have limited application, so it didn't seem worth it
• Git URLs as positional arguments has been removed as detecting them is ambiguous and limits future evolution (any other kind of URLs we might want to accept)
• <name>@<req> has switched to <name>:<req> as a parallel with the pkgid syntax This was reverted, we'll instead request cargo to add support for @ to pkgids
• --vers was removed in favor of <name>:<req> due to being redundant and reduces the difference between the "multiple add" vs "single add" modes, making it easier to explain
• --git <url> is now unstable and has been put behind -Z git feature flag as we work out how we want to expose git support
• Added `-F` short for `--features and we'll request other cargo commands to update accordingly
• Unstable support for inline feature activation (e.g. cargo add serde +derive serde_json)
  • An alternative not-implemented proposal is for position-sensitive options
• Overwrite mode is polished (calling cargo add multiple times on same dep)
  • Toggle default features on/off
  • Features get appended
  • Toggling of optional
  • And preserving of unspecified fields (1 , 2 , 3 ).
• Reuse version reqs from other tables

[josh]

for removing --upgrade, removing --vers in favor of name@version, and removing --allow-prerelease or making it unstable.

[epage]

I'm wondering if we should put the "modify" case behind an unstable flag.

I've been considering the case:

$ cargo add foo
...
$ cargo add foo --features regex
...
$ cargo add foo --features serde

The intent is to change the features but

• The version will change
• The version requirement operator will be set back to default
• We don't report what we changed from, leaving it to the user to diff the manifest to realize they thought it was going to append serde and it instead overwrote it, removing regex

The user can compensate for these but the extra work to make it not change the version and to discover and deal with the feature behavior negates the benefit of cargo add

I can easily adjust this but I worry there might be more like this. The modify feature seems like it could be useful or else I'd say remove it.

[djc]

@epage

cargo add regex
cargo add regex serde
cargo add regex@1 --dev
cargo add regex@~1.0 --build
cargo add regex --vers 1.0 --optional
cargo add regex --upgrade minor  # for some reason this is the policy name for `^`
cargo add https://github.com/rust-lang/regex
cargo add regex --git https://github.com/rust-lang/regex
cargo add ../dependency
cargo add dep --path ../dependency

In order to review this, it would be really helpful if there was a short example for each of these (like a one-line diff?) on what they did? Like, if I write cargo add regex, what does it use for the version?

My intuition here is that this is a lot to add in one go. Are we concerned about keeping CLI compatibility with the existing externally installed cargo add command? It would seem attractive to me to start from scratch with a minimal feature set and figure out the right syntax for a bunch of the edge cases later on. (For example, I would probably not add the cargo add https://github.com/rust-lang/regex interface in favor of the version with --git.)

[epage]

In order to review this, it would be really helpful if there was a short example for each of these (like a one-line diff?) on what they did?

Good call. I was mostly thinking in terms of people already using it but this is also important context for others to catch up on this

$ cargo add regex  # Adds `regex = "1.5.4"`
cargo add regex serde  # The above with `serde = "1.0.136"`
cargo add regex@1 --dev  # Adds `regex = "1" to `dev-dependencies`
cargo add regex@~1.0 --build  #  Adds `regex = "~1.0" to `build-dependencies`
cargo add regex --vers 1.0 --optional  # Adds `regex = { version = "1.0", optional = true}` 
cargo add regex --upgrade minor # Adds `regex = "^1.5.4"`
cargo add https://github.com/rust-lang/regex  # Adds `regex = { git = "https://github.com/rust-lang/regex" }`
cargo add regex --git https://github.com/rust-lang/regex   # Adds `regex = { git = "https://github.com/rust-lang/regex" }`
cargo add ../dependency  # Adds `dep = { version = "1.0.3", path = "../dependency" }`

My intuition here is that this is a lot to add in one go. Are we concerned about keeping CLI compatibility with the existing externally installed cargo add command? It would seem attractive to me to start from scratch with a minimal feature set and figure out the right syntax for a bunch of the edge cases later on.

We can break things from cargo-edit but I feel we should limit the scope of impact in removing functionality, erring towards unstable features (-Z), if the impact is too much. The built-in version will take precedence so if we reduce the feature set too much, so that people are wanting the cargo-edit version, then we'll have to rename it and people will have to go through hoops to use it.

(For example, I would probably not add the cargo add https://github.com/rust-lang/regex interface in favor of the version with --git .)

Same. I feel we should either only support --git or have s special indicator of a git path, like poetry does (git+<url>). The part that is appealing to me about the latter (combined with some of my other proposals like dropping --path) is we have the benefits of a flag but we are also able to consistently process all positional arguments as dependencies to add and only use the flags for modifying how we add them. While it complicates the syntax for specifying a dependency, it simplifies teaching how the args interact and simplifies the arg validation implementation.

We could also put both kinds of git support behind a -Z flag to see how they work and hold off on any decision.

[josh]

I get the impression that cargo add is used more commonly for crate names (to avoid having to look up version numbers) than for git URLs, so making that bit unstable seems fine.

Modification, though, seems too useful to leave unstable for long, for two reasons:

• It's helpful to be able to cargo add x without knowing if you already depend on x, and that shouldn't explode with a "that's nightly only" message.
• It's helpful to be able to add features, whether or not you have the dependency already.

[djc]

@epage

Good call. I was mostly thinking in terms of people already using it but this is also important context for others to catch up on this

$ cargo add regex  # Adds `regex = "1.5.4"`
cargo add regex serde  # The above with `serde = "1.0.136"`
cargo add regex@1 --dev  # Adds `regex = "1" to `dev-dependencies`
cargo add regex@~1.0 --build  #  Adds `regex = "~1.0" to `build-dependencies`
cargo add regex --vers 1.0 --optional  # Adds `regex = { version = "1.0", optional = true}` 
cargo add regex --upgrade minor # Adds `regex = "^1.5.4"`
cargo add https://github.com/rust-lang/regex  # Adds `regex = { git = "https://github.com/rust-lang/regex" }`
cargo add regex --git https://github.com/rust-lang/regex   # Adds `regex = { git = "https://github.com/rust-lang/regex" }`
cargo add ../dependency  # Adds `dep = { version = "1.0.3", path = "../dependency" }`

Some feedback:

• I don't think the @ syntax for versions fits very well, I've only seen it in the JavaScript ecosystem. I'd prefer an explicit switch like --vers though I also find it a little jarring. Do we use --vers in other places in the cargo CLI?
• The --dev, --build and --optional flags make intuitive sense to me.
• The --upgrade minor flag definitely doesn't. Maybe leave that out for now? I think a next step could be to work on an integration of cargo upgrade which could do that better.

@epage

We can break things from cargo-edit but I feel we should limit the scope of impact in removing functionality, erring towards unstable features ( -Z ), if the impact is too much. The built-in version will take precedence so if we reduce the feature set too much, so that people are wanting the cargo-edit version, then we'll have to rename it and people will have to go through hoops to use it.

Yeah, so the trick is to strike a good balance.

@epage

Same. I feel we should either only support --git or have s special indicator of a git path, like poetry does ( git+<url> ). The part that is appealing to me about the latter (combined with some of my other proposals like dropping --path ) is we have the benefits of a flag but we are also able to consistently process all positional arguments as dependencies to add and only use the flags for modifying how we add them. While it complicates the syntax for specifying a dependency, it simplifies teaching how the args interact and simplifies the arg validation implementation.

We could also put both kinds of git support behind a -Z flag to see how they work and hold off on any decision.

I don't like the git+<url> option that much because it's just different from what the TOML does. I think the --git version is fairly obvious/intuitive, so I wouldn't mind making that stable.

[Nemo157]

Probably doesn't matter since this looks backwards compatible to fix, but I just stumbled upon the fact that --manifest-path and --package can't be combined in the existing cargo-edit, while this should be valid to select a workspace manifest and then a package within that workspace to modify.

I also agree that @ looks really weird for the version constraint and I never knew it was supported, always using --vers. There seems to be a variety of operators used for this in third-party tools, I know that cargo-download writes this <name>=<version-request> (so tokio==1.0.0 to download specifically 1.0.0).

Similarly I never knew this supported upgrading, that seems quite an overlap with cargo-upgrade also from cargo-edit

[kornel]

I think it'd be fine to hide all advanced features that aren't rock-solid. The main use-case is cargo add crate_name, and it's better to have that standardized ASAP than to wait for other improvements.

[kornel]

I'd keep --vers, because cargo yank uses --vers. Not having it would create inconsistency within cargo.

[epage]

@Nemo157

also agree that @ looks really weird for the version constraint and I never knew it was supported, always using --vers . There seems to be a variety of operators used for this in third-party tools,

What are your thoughts on : which is what cargo uses?

[sanxiyn]

I like : instead of @.

[CAD97]

@epage

I've been considering the case:

$ cargo add foo
...
$ cargo add foo --features regex
...
$ cargo add foo --features serde

I definitely think that cargo add should (strive to) be semantically append-only — that is, doing cargo add [anything] will not remove any dependency, only add them. Cargo features are strictly additive, adding new dependencies is strictly additive, and theoretically (barring version conflicts and assuming semver-correctness), adding new "I use this" metadata into the manifest (the domain of cargo add) should never cause a compiling workspace to stop compiling.

My suggestion would be to -Zunstable gate or remove any functionality which doesn't meet the ideal of "semantically append-only."

[Nemo157]

Cargo uses : to separate package name and version in package ids, I don't know that it has ever used a shorthand notation for version requirements. Maybe using the same symbol for both would make sense, foobar:=1.2.3 is probably understandable.

Trying to trigger an error message containing a version requirement just now I used --rename for the first time. I would consider its behavior as implemented in cargo-edit buggy and don't think that should be stabilized in cargo; cargo add foo@1 && cargo add foo@2 --rename foo2 will result in the second add replacing the existing foo dependency with a renamed foo2 dependency instead of having two dependencies for the different versions.

[Kixunil]

I only use cargo add dep so I guess I'm fine with most things but would like to mention two features I'm missing to hopefully avoid decisions that could conflict with adding those features in the future:

• MSRV-aware adding - if I set MSRV of my project then cargo add should pick latest version of the dependency that obeys MSRV or has unknown MSRV. In case of unknown a warning could be printed.
• Adding whole group at once. A typical example is Json - one often adds serde, serde-derive and serde_json together. I'd like to configure a group called json in a config file (~/.config/cargo/crate_groups or so) and refer to it by some symbol. E.g. cargo add %json could add all crates from the group. Features would be very helpful too because e.g. adding rt to tokio is common.

[steffahn]

@Kixunil

one often adds serde , serde-derive

I would usually use just serde, but with the "derive" feature enabled, since it re-exports the derive macros.

[Kixunil]

Sure, it's not the only example... (hyper implying tokio comes to mind) But maybe this supports allowing features. One quite big annoyance about it is having to add { version = and } when specifying features. (Edit at two places.)

[epage]

@Kixunil

MSRV-aware adding - if I set MSRV of my project then cargo add should pick latest version of the dependency that obeys MSRV or has unknown MSRV. In case of unknown a warning could be printed.

This would be a great feature. I feel like we wouldn't do it though unless the data is in the Index which it unfortunately isn't.

[197g]

As an experienced user, a mere mapping of all fields a crate can have into command line arguments would not make a particularly convincing tool to me. I'm better served opening Cargo.toml in my favorite editor and adding them to the section. This has advantages:

• Ensures a consistent style, as for example dependencies that use any features including default ones are always scrutinized and I tend to add summaries justifying their use. I would have to do this process anyways, and the tool just makes it harder to implement such policy of well-documented code (and Cargo.toml is code in a machine readable sense).
• Editor integration, copy&paste for multiple, user-defined templates, …

I'd be much more convinced by some process that brings cargo-specific information into the regular editor workflow. Maybe not unlike git commit. That is, have the tool fill in fields it can automatically determine such as the most recent release (for an msrv) or necessary renames due to conflict. Maybe with an explicit --edit flag or the inverse, with my bias that I've always been content with git's default of requiring an explicit --no-edit in most places. Closing the editor directly without changes isn't that much of a deal.

Another question would be if it a 'transactional' property in the sense of sudoedit could bring anything to the table. Maybe the opened file to edit could be a copy and only when it parses according to cargo's rules, and dependency resolution works, the original file is replaced.

[Kixunil]

Crates can set it already which is the initial step towards solving it.