Add cargo-add (from cargo-edit) to cargo proper #5586

killercup · 2018-05-28T11:53:28Z

Cargo as well as cargo-edit have gone a long way since #4 was opened. It might now make sense to move (parts of) cargo-edit into cargo itself. Basically, we now have a format-preserving TOML library that -- at least for adding a dependency line -- work quite well (you can test this with cargo install cargo-edit --vers 0.3.0-beta.1 -f).

Speaking with @matklad, I believe I now have a somewhat solid understanding of what needs to happen to add a new built-in cargo subcommand. One important realization was that cargo-install already contains some of the parts we need.

Here are some steps to get started:

Create a src/bin/cargo/commands/add.rs (as duplicate of install.rs at first)
In src/bin/cargo/commands/mod.rs, add add to builtin/builtin_exec, and to the list of mods
Create a src/bin/cargo/ops/cargo_add.rs (as duplicate of cargo_install.rs at first)

One thing we should aim for though, is to now just copy-paste the whole of cargo-edit but to re-use as much of cargo's infrastructure as possible. For example, we should try to use cargo's way of querying the registry as well as its CLI args handling and output formatting. We can, for example, also use args.workspace.current_manifest to get the manifest and deal with workspace.

The text was updated successfully, but these errors were encountered:

killercup · 2018-05-28T11:58:53Z

@rust-lang/cargo, is this something you'd be interested in being worked on right now? This might work as a mentored even if you can't spend much time to work on it ourselves.

alexcrichton · 2018-05-28T17:04:21Z

I'd personally be thrilled to see this enter Cargo itself! I would ideally prefer to avoid having two TOML parsers built in but if we need to have that for the time being I think it's not the end of the world. (I don't know much about the technical design of cargo-edit right now)

steveklabnik · 2018-05-29T08:34:14Z

This is the number one thing I want in Cargo; a simple 👍 is not enough. Tons and tons and tons of people are very excited for this 🎊

ibaryshnikov · 2018-05-29T08:39:20Z

I'd like to implement it, work in-progress is here
https://github.com/ibaryshnikov/cargo/tree/issue-5586-cargo-add

japaric · 2018-09-06T23:59:30Z

I know that lots of people (myself included) are plenty busy with stuff for the upcoming edition release and that we don't have much spare ~~human~~ Rustacean resources but I still want to say that it would be AMAZING to have cargo add in Cargo 2018. As mentioned by boats, cargo add fits perfectly with the module / path changes of the edition. And I fully agree, but to me the biggest win would be not having to cargo install cargo-edit, which involves compiling / linking to C code and can be a (very) frustrating experience.

@rust-lang/cargo @killercup @aturon could we make this happen, somehow? 🙏

steveklabnik · 2018-09-07T00:11:13Z

Huuuuuuuuuuuuge 👍 here

…

On Sep 6, 2018, at 7:59 PM, Jorge Aparicio ***@***.***> wrote: I know that lots of people (myself included) are plenty busy with stuff for the upcoming edition release and that we don't have much spare human Rustacean resources but I still want to say that it would be AMAZING to have cargo add in Cargo 2018. As mentioned by boats, cargo add fits perfectly with the module / path changes of the edition. And I fully agree, but to me the biggest win would be not having to cargo install cargo-edit, which involves compiling / linking to C code and can be a (very) frustrating experience. @rust-lang/cargo @killercup @aturon could we make this happen, somehow? 🙏 — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

joshtriplett · 2018-09-07T02:55:05Z

👍 here.

ibaryshnikov · 2018-09-07T07:40:58Z

I did some preparations in this pr like filling the boilerplate and updating errors from cargo-edit to fit more for cargo, but farther work requires some patches to toml parser which is used in cargo (in case we don't want to have two parsers), and currently I don't have enough time to do required updates in the parser on my own. If you find some of the code in this pr fits for the solution, feel free to use it.

thejpster · 2018-09-11T18:36:14Z

I just wanted to add that I think cargo-add is a great tool for people new to Rust, as is cargo-generate. Unfortunately if I recommend them in a guide or tutorial, I have to explain how to install them and that they need to first install GCC and cmake and libssl-dev, which is non-trivial.

withoutboats · 2018-09-11T21:02:18Z

Definitely would love to see this added to cargo, since my workflow setting up a new dev environment is:

Install Rust
Install ripgrep
cargo install cargo-edit

As @ibaryshnikov points out though, its nontrivial work since the TOML parser cargo uses right now doesn't preserve layout, whitespace, or comments. We need to migrate to using the same parser cargo-edit uses before we can upstream any of its features into cargo.

withoutboats · 2018-09-12T21:04:03Z

We discussed this in the cargo meeting. There was strong support for upstreaming the functionality of cargo add into cargo. We doubt it can be stable in time for the 2018 edition (since the final cargo master for the edition needs to be merged roughly 5 weeks from today) but we do think its good to start making progress on adding this functionality.

We are comfortable having two TOML parsers in cargo for now (with a view to eventually merging them) so long as they share a common lexer, to be confident they aren't interpreting the TOML significantly differently. @alexcrichton says he's left a comment on the repo for the parser cargo-edit is using to this effect.

sandangel · 2019-01-05T00:56:28Z

Hi everyone, may I know the status of this issue? It's 2019 now and Happy New Year. ^^

Eh2406 · 2019-01-05T01:40:43Z

From the discord Today at 5:48 PM:
"ehuss: (btw, my format-preserving toml parser just passed all tests 🎉 😄 )"

yhaskell · 2019-08-02T17:17:41Z

Hello! Does anyone know what is the status of this?

ordian · 2019-08-04T11:28:07Z

In #5611 (comment) a concern was raised that we should (partially) deduplicate toml parsing libraries (cargo currently uses toml-rs, but that's not a lossless toml parser, e.g. it will eat your comments, cargo-edit currently uses toml_edit) before using it in cargo. Unfortunately, I don't have time to work on it (link to issues), but would be happy to mentor if someone wants to make this happen (if you're interested, please leave a message in https://gitter.im/toml_edit/Lobby).

After the above issues are resolved, we can hopefully revive #5611.

tomharmon · 2019-11-03T17:24:39Z

@SamedhG and I have started to work on updating toml_edit to support toml v0.5 We wanted to hear the opinions of the rust team and see which is a better option, or if there is not a significant difference between the two:

Add lossless parsing to toml_rs and adapt cargo_edit to use that instead of toml_edit, then merge cargo-edit into cargo proper
Update toml_edit to support toml v0.5 and update cargo to use toml_edit, then merge cargo-edit into cargo proper

@killercup @alexcrichton @steveklabnik @ordian

ehuss · 2019-11-04T22:54:08Z

I'd be very wary of having a second TOML parser to Cargo. I think it was previously mentioned that if they could share a lexer, that might be a compromise. I'm still concerned about adding a large dependency that has doubt around how well it is maintained.

I did work on an experimental preserving API for toml-rs, but never finished it. I do have the intention of picking it back up, but probably not for a long while.

Creating a good, clean API that handles all the intricacies of toml is quite difficult, and it will require some moderately invasive changes to toml-rs. I unfortunately don't have a lot of time to help with this right now.

sylann · 2022-01-01T18:11:27Z

Hello there,

Apparently this feature has been awaited for quite some time. No doubt the toml writing part has played a role in that.

I was wondering if the idea of just "printing the line that would be added to Cargo.toml" has been considered.

Finding the latest version of a given crate without leaving the terminal, and have it pre-written in the relevant format should be a good first step that solves most of the developer's trouble.

And if I'm not mistaken, it seems waaay simpler than trying to handle all the possible shenanigans of how an existing file could have been written.

Or did I miss something?

epage · 2022-01-01T18:25:45Z

I think going that route depends on the answer to the questions like:

Can we change behavior away from that once toml_edit is ready? My guess is "no".
Will taking over the "cargo add" name with diminished functionality be acceptable by end users? My guess is "no"

With that said, I was considering the idea of creating a command for interacting with the registry that could possibly fill the role of what you mentioned. Right now, cargo has its own copy of a lot of logic but isn't very good for depending on for external tools. Some of the logic gets reused through cargo metadata command which is wrapped by the cargo_metadata crate. A lot of other functionality gets duplicated instead, like crates_index crate. What if we instead made a cargo command for updating the local index and querying it and wrapping that in a crate? My hope is this would be an acceptable way for cargo to expose the registry code for reuse. You could then use this command as well to look up crates.

Also, #10086 now passes tests. All that is left is to audit the user facing changes (and adjust as needed) and I need to do benchmarking.

Alexendoo · 2022-01-01T18:25:48Z

@romain20100 Hi!

You can use cargo search for this, the results it returns are valid to be put in a Cargo.toml

sylann · 2022-01-02T13:34:01Z

@Alexendoo Sure, it's relatively trivial to adapt the result. In fact I've done just that with a simple bash function (see below).
But it's not immediately usable.

My point was just that there may be an intermediate milestone that could be simpler to achieve.
Although it's possible we are now close to a full on solution. In that case it's ok.

function cargo-add {
    output=$(cargo search $1)
    tmp="${output//$1 = \"}"
    version="${tmp//\"*}"
    echo "$1 = { version = \"$version\" }"
}

lnicola · 2022-01-02T13:42:37Z

@romain20100 you can already copy-paste the output of cargo search to Cargo.toml. E.g. serde = "1.0" is enough, you don't have to write it as serde = { version = "1.0" }.

I don't think there's any need for an intermediate step here. cargo edit can and will be added to cargo proper when it's considered ready.

simonsan · 2022-01-02T14:55:08Z

serde = { version = "1.0" }

At least from cargo-smart-release I know from the limitations section, that the version needs to be inside a table though.

Edit: By which I mean that there is sometimes this requirement, which probably should be put behind an option flag or somethign? 🤔 Not sure.

CC: @Byron

epage · 2022-01-03T12:37:53Z

serde = { version = "1.0" }

At least from cargo-smart-release I know from the limitations section, that the version needs to be inside a table though.

Edit: By which I mean that there is sometimes this requirement, which probably should be put behind an option flag or somethign? thinking Not sure.

imo the focus should be on fixing that git-smart-release limitation. The code exists for it in cargo-release and in cargo-set-version (in this repo).

Byron · 2022-01-05T23:33:14Z

Thanks for bringing up the issue @simonsan , I totally agree with @epage and think smart-release should learn to deal with non-table versions.

Benefits: - A TOML 1.0 compliant parser - Unblock future work - Have `cargo init` add the current crate to the workspace, rather than error - rust-lang#5586: Upstream `cargo-add`

Port cargo from toml-rs to toml_edit Benefits: - A TOML 1.0 compliant parser - Unblock future work - Have `cargo init` add the current crate to the workspace, rather than error - #5586: Upstream `cargo-add` TODO - [x] Analyze performance and address regressions - [x] Identify and resolve incompatibiies - [x] Resolve remaining test failures, see https://github.com/ordian/toml_edit/labels/cargo - [x] ~~Switch the code from #10176 to only parse once~~ (this PR is being merged first)

epage · 2022-01-20T13:57:11Z

As an update, #10086 has been merged which was the major blocker for cargo-edit being upstreamed.

I am now preparing cargo-add for merging.

I've upgraded cargo-edit to the latest toml_edit, past some breaking changes that had a large effect on it
I am currently porting the tests to trycmd which, through its snapshot updating, will make it easier to update tests as we port cargo-add to use cargo's facilities
Next, I will fork cargo-add into a branch and will slowly refactor it into a state for merging
- Isolating all of the logic cargo-add needs from the cargo-edit [lib]
- Port parts of cargo-add from what was in cargo-edit [lib] to cargo
Then we'll be ready for a draft PR with the proposal to merge, like cargo tree
- I think an important part of this will be calling out the current CLI and re-examining parts of it (I think --upgrade in particular should be re-examined for how to expose that functionality)
- I feel like this issue has too much noise in it already to have the conversation. Maybe it could be had on internals?
We can then start prioritizing and preparing the next cargo-edit command

epage · 2022-01-25T20:34:09Z

FYI I just posted on internals looking for feedback before we restrict cargo-add's evolution by merging it in cargo.

nyurik · 2022-02-17T07:47:00Z

Is porting cargo upgrade part of this ticket, or would that be a separate one? I couldn't find any mentions of it besides this issue. Thx!

epage · 2022-02-17T14:56:57Z

The plan is to focus on cargo add as its seen as the highest priority. After that, I'll be looking to cargo rm because of the symmetry with add and it being a small lift. cargo upgrade will be after that and there are a lot of things we need to figure out (e.g. should it be kept as a separate command or expressed as flags on cargo update)

epage · 2022-02-17T21:57:34Z

I've started work on the port of cargo-add to cargo. The cargo-edit branch can be found at https://github.com/killercup/cargo-edit/tree/merge-add

If anyone wants to help out, steps we have include

Separate out an "op"
Port to cargo's internal metadata code
Port to cargo's registry code
Port to cargo's clap code
Look into how we should be handling user-reporting

Feel free to coordinate with me on Zulip, gitter, or Discord

epage · 2022-03-10T01:20:10Z

Once I merge this last PR, the merge-add branch is complete. Tomorrow, I expect to try to move the code over to cargo and make any adjustments needed for being in-tree. After that, I'll be making the PR that proposes merging cargo-add into cargo.

epage · 2022-03-10T20:05:52Z

Its live!

Import `cargo remove` into cargo ## What does this PR try to resolve? This PR merges `cargo remove` from [cargo-edit](https://github.com/killercup/cargo-edit) into cargo. ### Motivation - General approval from community, see #5586 and #10520. - Satisfying symmetry between add and remove. - Help users clean up their manifests (for example, when users forget to remove optional dependencies from feature lists). With #10472, cargo-add was added to cargo. As part of that discussion, it was also proposed that `cargo rm` (now `cargo remove`) eventually be added as well. ### Drawbacks - Additional code always opens the door for more bugs and features - The scope of this command is fairly small though - Known bugs and most known features were resolved before this merge proposal ### Behavior `cargo remove` operates on one or more dependencies from a manifest, removing them from a specified dependencies section (using the same flags as `cargo-add`) and from `[features]` activations if the dependency is optional. Feature lists themselves are not automatically removed when made empty. Like with cargo-add, the lock file is automatically updated. Note: like `cargo add`, `cargo remove` refers to dependency names, rather than crate names, which can be different with the presence of the `name` field. Note: `cargo rm` has been renamed to `cargo remove`, based on prior art and user feedback (see [discussion](#10520)). Although this renaming is arguably an improvement, adding an `rm` alias could make the switch easier for existing users of cargo-edit (at the cost of a naming conflict which would merit insta-stabilization). #### Help output <details> ```shell $ cargo run -- remove --help cargo-remove Remove dependencies from a Cargo.toml manifest file USAGE: cargo remove [OPTIONS] <DEP_ID>... ARGS: <DEP_ID>... Dependencies to be removed OPTIONS: -p, --package [<SPEC>...] Package to remove from -v, --verbose Use verbose output (-vv very verbose/build.rs output) --manifest-path <PATH> Path to Cargo.toml --offline Run without accessing the network -q, --quiet Do not print cargo log messages --dry-run Don't actually write the manifest -Z <FLAG> Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details -h, --help Print help information SECTION: --dev Remove as development dependency --build Remove as build dependency --target <TARGET> Remove as dependency from the given target platform ``` </details> #### Example usage ``` cargo remove serde cargo remove criterion httpmock --dev cargo remove winhttp --target x86_64-pc-windows-gnu cargo remove --package core toml ``` ## How should we test and review this PR? This is following the pattern from cargo-add which was implemented in three different PRs (implementation, documentation, and completions), in the interest of reducing the focusing discussions in each PR and allowing cargo-add's behavior to settle to avoid documentation churn. 1. #10472 2. #10578 3. #10577 The remaining changes (documentation and shell completions) will follow shortly after. Some work has already begun on this feature in #11059. Work on this feature was carried out on the [`merge-rm`](killercup/cargo-edit@master...merge-rm) branch of cargo-edit with PRs reviewed by `@epage.` If you are interested in seeing how this feature evolved to better match cargo's internals, you might find the commit history there to be helpful. As this PR is reviewed, changes will be made both here and on that branch, with the commit history being fully maintained on the latter. `cargo remove` is structured like most other subcommands: - `src/bin/cargo/commands/remove.rs` contains the cli handling and top-level execution. - `src/cargo/ops/cargo_remove.rs` contains the implementation of the feature itself. In order to support this feature, the `remove_from_table` util was added to `util::toml_mut::manifest::LocalManifest`. Tests are split out into a separate commit to make it easier to review the production code and tests. Tests have been implemented with `snapbox`, structured similarly to the tests of `cargo add`. ### Prior art - Python: [`poetry remove`](https://python-poetry.org/docs/cli/#remove) - Supports dry run - JavaScript: [`yarn remove`](https://yarnpkg.com/cli/remove) - Supports wildcards - JavaScript: [`pnpm remove`](https://pnpm.io/cli/remove) - Go: [`go get`](https://go.dev/ref/mod#go-get) - `go get foo@none` to remove - Julia: [`pkg rm`](https://docs.julialang.org/en/v1/stdlib/Pkg/) - Supports `--all` to remove all dependencies - Ruby: [`bundle remove`](https://bundler.io/v2.2/man/bundle-remove.1.html) - Dart: [`dart pub remove`](https://dart.dev/tools/pub/cmd/pub-remove) - Supports dry run - Lua: [`luarocks remove`](https://github.com/luarocks/luarocks/wiki/remove) - Supports force remove - .NET: [`Uninstall-Package`](https://docs.microsoft.com/en-us/nuget/reference/ps-reference/ps-ref-uninstall-package) - Supports dry run - Supports removal of dependencies - Supports force remove (disregards dependencies) - Haxe: [`haxelib remove`](https://lib.haxe.org/documentation/using-haxelib/#remove) - Racket: [`raco pkg remove`](https://docs.racket-lang.org/pkg/cmdline.html#%28part._raco-pkg-remove%29) - Supports dry run - Supports force remove (disregards dependencies) - Supports demotion to weak dependency (sort of a corollary of force remove) ### Insta-stabilization In the discussion of `cargo add`'s stabilization story ([Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/Stablizing.20cargo-add)), it was brought up that the feature might benefit from being insta-stabilized to avoid making the cargo-edit version of the binary hard to access. Since `cargo rm` (from cargo-edit) was renamed to `cargo remove` here, [such a conflict no longer exists](https://crates.io/search?q=cargo%20remove), so this is less of a concern. Since this feature is already has a had a long run of user testing in cargo-edit and doesn't have unsettled UI questions like cargo-add did, it might still be a candidate for insta-stabilization. ### Deferred work Necessary future work: - Add documentation. - Add shell completions. - Perform GC on workspace dependencies when they are no longer used (see #8415). - This is inspired by a feature from the RFC that was dropped (unused dependencies triggering a warning) - This was deferred out to avoid challenges with testing nightly features It was found in the review of `cargo add` that it was best to defer these first two items to focus the discussion and as there was still behavior churn during the review of cargo-add. ### Future Possibilities The following are features which we might want to add to `cargo remove` in the future: - Add a `cargo rm` alias to ease transition for current cargo-edit users - Automatically convert between dash and underscores in deps: killercup/cargo-edit#690 - Remove unused dependencies: killercup/cargo-edit#415 - Clean up caches: killercup/cargo-edit#647 ### Additional information Fixes #10520.

ibaryshnikov mentioned this issue Jun 5, 2018

[WIP]: Add cargo-add (from cargo-edit) to cargo proper #5611

Closed

Mark-Simulacrum added I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting T-cargo Team: Cargo labels Sep 7, 2018

nrc removed the I-nominated-to-discuss To be discussed during issue triage on the next Cargo team meeting label Sep 19, 2018

dwijnand mentioned this issue Oct 27, 2018

Subcommand to add a new dependency to Cargo.toml #2179

Closed

dwijnand mentioned this issue Dec 18, 2018

easier way to add crates in cargo #6457

Closed

jonas-schievink mentioned this issue Dec 20, 2018

Core? killercup/cargo-edit#252

Closed

carols10cents mentioned this issue Dec 26, 2018

Ability to cargo install <dependency>@<version> should update Cargo.toml #6488

Closed

ordian mentioned this issue May 8, 2019

Migrate to 2018 edition killercup/cargo-edit#292

Merged

This was referenced Jun 21, 2019

cargo install command #1917

Closed

cargo add <package> #4

Closed

cargo dependency subcommands #651

Closed

ehuss added the A-new-subcommand Area: new subcommand label Aug 22, 2019

epage mentioned this issue Mar 10, 2022

feat: Import cargo-add into cargo #10472

Merged

epage mentioned this issue Mar 22, 2022

A method to update manifest version requirements (like cargo-edit's cargo-upgrade) #10498

Open

epage mentioned this issue Mar 30, 2022

Add cargo-rm (from cargo-edit) to cargo proper #10520

Closed

bors closed this as completed in 6a4d98d Apr 18, 2022

This was referenced Sep 14, 2022

Merge cargo-rm into cargo killercup/cargo-edit#682

Open

Import cargo remove into cargo #11099

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cargo-add (from cargo-edit) to cargo proper #5586

Add cargo-add (from cargo-edit) to cargo proper #5586

killercup commented May 28, 2018 •

edited

Loading

killercup commented May 28, 2018

alexcrichton commented May 28, 2018

steveklabnik commented May 29, 2018

ibaryshnikov commented May 29, 2018 •

edited

Loading

japaric commented Sep 6, 2018

steveklabnik commented Sep 7, 2018 via email

joshtriplett commented Sep 7, 2018

ibaryshnikov commented Sep 7, 2018

thejpster commented Sep 11, 2018

withoutboats commented Sep 11, 2018

withoutboats commented Sep 12, 2018

sandangel commented Jan 5, 2019

Eh2406 commented Jan 5, 2019

yhaskell commented Aug 2, 2019

ordian commented Aug 4, 2019

tomharmon commented Nov 3, 2019 •

edited

Loading

ehuss commented Nov 4, 2019

sylann commented Jan 1, 2022

epage commented Jan 1, 2022

Alexendoo commented Jan 1, 2022

sylann commented Jan 2, 2022

lnicola commented Jan 2, 2022

simonsan commented Jan 2, 2022 •

edited

Loading

epage commented Jan 3, 2022

Byron commented Jan 5, 2022

epage commented Jan 20, 2022

epage commented Jan 25, 2022

nyurik commented Feb 17, 2022

epage commented Feb 17, 2022

epage commented Feb 17, 2022 •

edited

Loading

epage commented Mar 10, 2022

epage commented Mar 10, 2022

Add cargo-add (from cargo-edit) to cargo proper #5586

Add cargo-add (from cargo-edit) to cargo proper #5586

Comments

killercup commented May 28, 2018 • edited Loading

killercup commented May 28, 2018

alexcrichton commented May 28, 2018

steveklabnik commented May 29, 2018

ibaryshnikov commented May 29, 2018 • edited Loading

japaric commented Sep 6, 2018

steveklabnik commented Sep 7, 2018 via email

joshtriplett commented Sep 7, 2018

ibaryshnikov commented Sep 7, 2018

thejpster commented Sep 11, 2018

withoutboats commented Sep 11, 2018

withoutboats commented Sep 12, 2018

sandangel commented Jan 5, 2019

Eh2406 commented Jan 5, 2019

yhaskell commented Aug 2, 2019

ordian commented Aug 4, 2019

tomharmon commented Nov 3, 2019 • edited Loading

ehuss commented Nov 4, 2019

sylann commented Jan 1, 2022

epage commented Jan 1, 2022

Alexendoo commented Jan 1, 2022

sylann commented Jan 2, 2022

lnicola commented Jan 2, 2022

simonsan commented Jan 2, 2022 • edited Loading

epage commented Jan 3, 2022

Byron commented Jan 5, 2022

epage commented Jan 20, 2022

epage commented Jan 25, 2022

nyurik commented Feb 17, 2022

epage commented Feb 17, 2022

epage commented Feb 17, 2022 • edited Loading

epage commented Mar 10, 2022

epage commented Mar 10, 2022

killercup commented May 28, 2018 •

edited

Loading

ibaryshnikov commented May 29, 2018 •

edited

Loading

tomharmon commented Nov 3, 2019 •

edited

Loading

simonsan commented Jan 2, 2022 •

edited

Loading

epage commented Feb 17, 2022 •

edited

Loading