rust-lang / cargo

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

Make `cargo --list` print description for custom subcommands #10662

Open dtolnay opened 2 years ago

dtolnay commented 2 years ago

Problem

Currently cargo --list prints descriptions only for the builtin subcommands. For third-party subcommands like cargo expand it does not print a description.

$ cargo --list
Installed Commands:
    …
    doc                  Build a package's documentation
    expand
    fetch                Fetch dependencies of a package from the network
    …

Proposed Solution

First of all, I don't want cargo --list to run any of the binaries (in order to, for example, run --help on them and parse the output, or some other handshake to cause the binary to print out a description of itself). Arbitrary binaries in the PATH are not necessarily safe to run. An arbitrary binary might completely ignore --help or any other handshake being passed by Cargo, and immediately do unwanted stuff instead, which would be unexpected and undesirable to the person running cargo --list.

Instead, Cargo subcommands should be able to embed a description into their compiled binary, which can then be directly retrieved from the binary by Cargo in a way that does not involve running it.

The way this is exposed to subcommands can be as simple as:

// subcommand's main.rs

cargo_subcommand_metadata::description!("…");

fn main() {
    /* … */
}

The underlying implementation would need to be platform-specific based on the file format used for executables on the platform. On platforms that use ELF executables, I would propose that this uses a Note. On those platforms the macro shown above would expand to:

#[used]
#[link_section = ".note.cargo.subcommand"]
static ELF_NOTE: ElfNote = {
    …
}

This note can then be retrieved efficiently from the ELF executable during cargo --list by using the object crate to parse the executable's program header.

From testing on my machine with the cargo-expand crate, cargo build --release produces a 13 MB executable, and retrieving the subcommand description ELF note from it using the object crate takes 0.03 milliseconds, so this should be plenty fast enough for cargo --list.

We do not necessarily need to implement support for a large number of platforms immediately up front. Supporting just ELF would already benefit a large fraction of Cargo users.

Notes

Yes, exactly.

dtolnay commented 2 years ago

As proof of concept, I have released a version of the cargo-expand crate containing a suitable ELF note (https://github.com/dtolnay/cargo-expand/pull/142) and created a draft PR #10663 which makes cargo --list retrieve the subcommand description from that ELF note.

Screenshot from 2022-05-11 23-37-10

epage commented 2 years ago

Currently, cargo hard codes the descriptions for first-party cargo external subcommands like cargo-clippy and cargo-fmt. This would also allow us to remove those hard coded descriptions and scale up better for adding more first-part external subcommands. I've been toying with the idea of splitting some of cargo's built-in subcommands out as external subcommands. This has the downside increasing the distribution size though it means we could possibly remove them in the minimal rustup profile. On the other hand, modularization like this could help with a more approachable code base that is easier to review, dogfooding of cargo's APIs, and faster compile/test times for whichever section of cargo a developer is in. This is blocked on our need to scale up our processes via workspaces which is blocked on the fact that we are part of the rust-lang/rust workspace and can't have our own.

ehuss commented 1 year ago

The Cargo Team discussed this a bit, and here are some questions or thoughts from that discussion:

timmyjose commented 1 year ago

I was thinking that I'd missed something in the documentation, but I see that this issue is still open. Hopefully it should be resolved sometime in the near future.

Oakchris1955 commented 5 months ago

Perhaps we could try and embed a subcommand's description into the executable as discussed, at least for the platforms which support doing this, and as a fallback for other platforms or executables without a description embedded, try to fetch it from another source (perhaps from Cargo.toml as already suggested or by querying the crates.io API). Nevertheless, we also need to account for backwards compatibility, hence the fallback proposal.

epage commented 5 months ago

Instead of doing a web request on every cargo --list, what if we stored the description inside of crates2.json?

dtolnay commented 5 months ago

Compared to putting descriptions into the executable, putting descriptions in .crates2.json has the disadvantage that it would only work for subcommands installed using cargo install. Subcommands installed using cargo binstall (https://github.com/cargo-bins/cargo-binstall/issues/1552), or just downloading a published release, would not be able to have descriptions in cargo --list.

As a fallback for the cases where the executable does not contain a description, using descriptions from .crates2.json seems good to me.

epage commented 5 months ago

13847 was proposing downloading descriptions from crates.io for plugins installed via cargo install, so definitely an improvement over that.

But yes, it doesn't cover other install cases. Its also a lot lower barrier for adoption than accepting the embedded description scheme.

Oakchris1955 commented 5 months ago

Subcommands installed using cargo binstall (https://github.com/cargo-bins/cargo-binstall/issues/1552), or just downloading a published release, would not be able to have descriptions in cargo --list.

I think this is out of scope. But then again, one could alert the devs of cargo-binstall about the incoming change and have them implement a solution similar to ours

weihanglo commented 5 months ago

Web requests might lead to wrong information. People can have a script called cargo-expand under $PATH, but has nothing to do with cargo-expand on crates.io.

To me, embedded description is more accurate an approach, and then description-in-crates2.json. Despite that, it depends on how accurate cargo --list need to be.

Oakchris1955 commented 5 months ago

Perhaps the following logic could be implemented to solve most of the aforementioned issues:

1) When a user installs a subcommand, save the description in .crates2.json 2) When cargo --list is run:

epage commented 5 months ago

If not (because the subcommand would have installed before this issue got resolved/implemented), fetch the description from crates.io (if it was installed from there) and save it in .crates2.json

For myself, I don't see enough benefit to doing this.