rust-lang / crates.io

The Rust package registry
https://crates.io
Apache License 2.0
2.9k stars 592 forks source link

Display `cargo install crate-name` rather than `cargo add crate-name` for binary crates #5882

Closed Aloso closed 1 month ago

Aloso commented 1 year ago

Current Behavior

crates.io has an "Install" sections with copy-paste snippets to install the crate. It includes these two snippets:

cargo add crate-name
crate-name = "version"

For binary crates (e.g. ripgrep) this is not useful.

Expected Behavior

For crates that have a binary target, but not a library target, crates.io should show instead:

cargo install crate-name

For crates that have both a binary target and a library target, crates.io should all three:

cargo add crate-name
crate-name = "version"
cargo install crate-name

Steps To Reproduce

  1. Go to https://crates.io/crates/ripgrep
  2. Look at the page

Environment

Anything else?

No response

epage commented 1 year ago

At the moment, crates.io has no knowledge of whether a crate has a [[bin]] or a [lib] component. You can see what cargo publish tells crates.io here.

Adjusting crates.io's backend to take that information and cargo to report it would be the first step. We'd then need to wait a period of time for publishes to happen to get the new data. In theory, we could tear apart the crates and update the metadata but I suspect this is a more more complex of a case (e.g. different lib and main auto-detection rules) that we would just wait for new publishes. Then the webpage could start sporting the new information.

I feel like there were an issue or two in cargo's backlog where having this information would be helpful but I can't find them at the moment.

epage commented 1 year ago

I remembered what cargo's case is.

If we are already putting whether a crate has a [[bin]] and/or a [lib] in the index, it could be useful for us to enumerate what [[bin]]s are present.

In cargo today, if you type cargo foo without the cargo-foo bin being installed, we could check if a cargo-foo crate exists and suggest cargo install cargo-foo. The more advanced solution is if we can walk the entire index, checking for what crate includes the cargo-foo bin, we can then suggest that crate name. For example, the cargo upgrade bin is provided by cargo-edit crate.

See rust-lang/cargo#4682 for more details.

jonasbb commented 1 year ago

[...] it could be useful for us to enumerate what [[bin]]s are present.

That could also be a first step in helping cargo avoid needless reinstalls of crates, since cargo currently gets confused when bin targets are only available conditionally, e.g., https://github.com/rust-lang/cargo/issues/8513 and https://github.com/rust-lang/cargo/issues/8703.

ccqpein commented 6 months ago

I found the same thing on crates.io days ago and I found this issue. I checked the ref issues/PRs linked with this issue and source code. If I understand correct, we need to update db by adding the enum crates types, index it, then change the source code to parsing the [lib] [[bin]] etc., right?

Another rookie question, I code Rust for years but never has chance to contribute to community, so what's the workflow if we want to implement this? Like we give purpose first or RFC? Do we already has this issue's RFC? Thanks.

epage commented 6 months ago

crates.io is using a library to parse Cargo.toml that might or might not support target discovery. Implementing it can be a bit messy because you are operating on a .crate so you need to implement a FS trait. However, they should be switching to cargo-util-schema as that is maintained by the cargo team and used in cargo. That doesn't do target discovery. However, I have been wanting to change cargo to "normalize" the manifests to include all targets to be discovered within the manifest when its published. This gets a bit messy so not sure how good it is for a first issue but willing to help out if you want to go that route. I'm available on zulip and have office hours.

If crates.io team wants to go with the FS trait until then, I leave that to them.

Once its in the manifest, it becomes easier. In the code, you would check the manifest for what it contains and then set a field in the database that you'd add (maybe a "has bin" and "bin names"?). You'd then have the frontend change things based on that. I can't speak to all of the best practices and desires of the crates.io team towards the specifics of this.

ccqpein commented 6 months ago

@epage Thanks!

I have been wanting to change cargo to "normalize" the manifests to include all targets

Is this one? https://github.com/rust-lang/rfcs/pull/3371

Turbo87 commented 6 months ago

the normalized manifests would certainly make things easier in the future. unfortunately they won't help if we want to backfill the information for existing crate files though.

crates.io is using a library to parse Cargo.toml that might or might not support target discovery.

https://github.com/LukeMathWalker/cargo-manifest/blob/v0.12.1/src/lib.rs#L239-L248 is what Ed was talking about. the library that we're currently using supports it, but we're not using that part of the library yet since it would slow down the publish step even more. we could however introduce another background worker job, that downloads the newly published crate file again and does a slightly deeper analysis including the library-or-application check.

one open question about this is how we deal with crate files that have both an application and a library in them. but that can still be decided and changed on the frontend, once we have all the necessary data in the database and exposed on the API.

epage commented 6 months ago

Is this one? https://github.com/rust-lang/rfcs/pull/3371

That is just about target/. What I'm talking about doesn't need an RFC but it does touch a lot of places in cargo (I have a branch where I started on it).

finnbear commented 1 month ago

I have a library crate with a few binaries for my own use during development. They are not for end users of the crate. Is there a way to disable the cargo install instructions? (or, preferably, a way to prevent cargo install from doing anything?)

image

epage commented 1 month ago

Could you exclude them from being published? Or move them to another package in the workspace?

finnbear commented 1 month ago

Could you exclude them from being published?

Thanks for the suggestion, but I thought exclude was for excluding files not disabling binaries. The binaries are mentioned in my Cargo.toml file, and I don't know how cargo or crates.io will react if binary files are simply missing.

Or move them to another package in the workspace?

Yes, as a last resort :)

epage commented 1 month ago

Thanks for the suggestion, but I thought exclude was for excluding files not disabling binaries. The binaries are mentioned in my Cargo.toml file, and I don't know how cargo or crates.io will react if binary files are simply missing.

If you wait a couple of weeks, Cargo 1.80 will strip [[bin]] entries that are excluded, see https://github.com/rust-lang/cargo/pull/13713

finnbear commented 1 month ago

Thanks, that solves my problem! :heart:

Turbo87 commented 1 month ago

the has_lib and bin_names columns in the database have been backfilled today (see https://rust-lang.zulipchat.com/#narrow/stream/318791-t-crates-io/topic/lib.2Fbin.20detection). https://crates.io/crates/ripgrep finally shows cargo install now, so I guess we can finally close this issue as resolved 🎉