Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dtolnay opened this issue May 12, 2022 · 12 comments
Open

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

dtolnay opened this issue May 12, 2022 · 12 comments
Labels
A-custom-subcommands Area: custom 3rd party subcommand plugins C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-list S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@dtolnay
Copy link
Member

dtolnay commented May 12, 2022

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 dtolnay added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label May 12, 2022
@dtolnay
Copy link
Member Author

dtolnay commented May 12, 2022

As proof of concept, I have released a version of the cargo-expand crate containing a suitable ELF note (dtolnay/cargo-expand#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
Copy link
Contributor

epage commented May 12, 2022

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
Copy link
Contributor

ehuss commented May 18, 2023

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

  • There were some concerns that the proposed solution seemed to be ELF-specific. Although this could be extended for other formats, like Windows, it seems risky to try to support that across all of Cargo's platforms. Additionally, some people use shell scripts or other kinds of executables that can't embed metadata.
  • Would it be possible to use the description from Cargo.toml instead? I think there was some concern that what people put in description might be slightly different from what they would use for cargo --list, but I suspect in most cases it should be sufficient. If so, then one option that may be simpler is to embed the metadata in cargo's "install" JSON tracker.

@ehuss ehuss added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label May 18, 2023
@timmyjose
Copy link

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
Copy link

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
Copy link
Contributor

epage commented May 3, 2024

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

@dtolnay
Copy link
Member Author

dtolnay commented May 3, 2024

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 (cargo-bins/cargo-binstall#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
Copy link
Contributor

epage commented May 3, 2024

#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
Copy link

Subcommands installed using cargo binstall (cargo-bins/cargo-binstall#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
Copy link
Member

weihanglo commented May 7, 2024

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
Copy link

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:
    • For each subcommand, check if the relevant description field exists in .crates2.json
    • 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
    • Show the description to the user

@epage
Copy link
Contributor

epage commented May 8, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-custom-subcommands Area: custom 3rd party subcommand plugins C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-list S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

6 participants