recommended-bin-packages field in Cargo.toml by not-my-profile · Pull Request #3383 · rust-lang/rfcs (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation26 Commits6 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This RFC proposes the addition of an optional recommended-bin-packages field to the [package] section of Cargo.toml, to enable package authors to point out related binary packages in the error message Cargo users get when attempting to cargo install a package without binaries.

Rendered
FCP

(Opening an RFC for this since the idea has gotten positive feedback on the Rust Internals forum).

lebensterben, steffahn, typecasto, Aloso, ranile, schneiderfelipe, zachs18, artob, hkratz, mav8557, and 3 more reacted with thumbs up emoji steffahn, ChayimFriedman2, Gankra, Tiwalun, estebank, artob, and ameknite reacted with heart emoji

Clippy could gain a lint to check that the referenced crates actually exist, have not been yanked and are actually binary crates.

I feel like this might be something that cargo publish should could/should check, at least partially, though I'd be fine to still keep it only as a future possibility.

Right, I did also think about that. The issue with that is that there's somewhat of a chicken-and-egg problem when first publishing crates since the crates referenced via this new field are likely to depend on the crate containing the field.

But if you have not published a foobar-cli crate yet, publishing foobar library crate with a recommendation for foobar-cli is bad, too. Anyone could publish a crate called foobar-cli at that point, before you publish it yourself. It seems highly advisable to me to go about this in a different order, even if doing so would involve pre-publishing a crate in a dummy state to reserve the name. Either that, or have the initial version of foobar not come with a recommended-bin-crates field. And lastly, it’s always possible to disable checks done locally on your machine by cargo publish (e.g. with --no-verify), if you know what you’re doing and you are willing to risk the race-condition and the possibility to make a mistake.

ehuss added the T-cargo

Relevant to the Cargo team, which will review and decide on the RFC.

label

Feb 4, 2023

So, I personally feel like this feature is almost too restrictive for binary crates, and would make more sense as a general recommendation mechanism. For example, serde-json is often used with serde, and the serde crate could vouch for that crate being recommended.

Except this almost approaches the point where it's maybe not a good thing to include in the cargo manifest, since that is pinned to versions and for example, installing an old version of serde would give old recommendations.

Just a few thoughts on this; not super strong on these ideas, just kind of my gut reaction to the RFC.

The proposed field has to be specific to binary crates so that cargo install can point out these crates when an installation fails because the crate doesn't contain any binaries. Improving that error message is very much the primary goal of this RFC.

I don't really see the need for a more general recommendation mechanism, I think documenting related libraries in the README works well enough, for instance the second link of the serde README is Data formats supported by Serde.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when this field is specified in a package with bins?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I'd say the field is specifically meant for crates without binaries. Specifying it for a crate with binaries wouldn't have any effect unless websites for crates would decide to display the field regardless.

I'll clarify the Guide-level explanation accordingly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside: #3243 could offer another way to link these crates together in the UI

Comment on lines 156 to 157

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least as of today, clippy does not lint Cargo.toml. We are cautious of adding warnings to cargo itself as we don't (yet) have lint controls.

Another problem is whatever does the linting would need to update and check the index.

Also, is the "actually exist" check just to move the cargo publish check earlier?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are mistaken about clippy currently not linting Cargo.toml. Clippy has the clippy::cargo category which currently contains 5 lints (you can filter for the Cargo lint group on the Clippy Lints website) ... it's just that the category isn't enabled by default.

You are right about the indexing part ... I can add a respective note to the RFC. Since this is the Future possibilities section, I think it's fine that some things would still need to be figured out.

My reasoning for suggesting such a clippy lint is that the referenced packages may change after the referencing package has been published. E.g. if we publish a lib-only crate with a valid reference and the referenced crate gets yanked after, that wouldn't be picked up until we do a new cargo publish ... so the lint would be particularly helpful for lib-only crates that don't have regular releases.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some clippy lints but the team seems hesitant about each new cargo clippy lint.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an update, cargo's linting system is moving forward, see rust-lang/cargo#12235

The main question at this time is always-run lints (like rustc) vs on-demand lints (like clippy)

not-my-profile changed the titlerecommended-bin-crates field in Cargo.toml recommended-bin-packages field in Cargo.toml

Feb 15, 2023

I saw other people using cargo install this way,

cargo install foo-cli alias dora='foo-cli' foo --help

so I thought I could add a feature: if a user wants to install a foo binary, cargo can install foo-cli directly and the user can run it as foo

if a user wants to install a foo binary, cargo can install foo-cli directly and the user can run it as foo

The latter is already possible, the binary name does not have to match the package name (a package may also include multiple binaries), either by putting the main at src/bin/foo/main.rs and relying on auto-detection, or manually configuring the name separately:

[package] name = "foo-cli"

[[bin]] name = "foo" path = "src/main.rs"

We discussed this in today's Cargo team meeting as part of our RFC triage. We had previously discussed the trade offs between putting everything into a package vs using a workspace. We are overall in favor of improving the workflow for people.

The main concern is over what schema to use (which different designs can lead to minor tweaks to the semantics). We discussed the possibility of a [diagnostics] table (inspired by the recently stabilized diagnostics attribute) and lean towards it after exploring several potential use cases for it. We'd recommend exploring what the experience would be like to create a [diagnostics] table, what kind of fields make sense within that and what lessons we should learn from the diagnostic attribute (CC @estebank for any insight you can provide).

@rfcbot merge

@rfcbot concern schema

@epage (off the cuff) one thing that came up with #[diagnostic] is the need for it to be very permissive without causing errors: whenever you add a feature you would never be able to remove it without pain, and if you add a new feature to one version and people want to use it, you're forcing crate authors to bump their MSRV sorely for better errors. You'll also need to see how you handle these for transitive dependencies, whether crate authors can customize errors not just for people who have added them as direct dependencies, or for everyone that has them as a dependency in their tree.

@estebank any lessons on the reporting side? We're wondering whether to expose just a string or go all out like diagnostics or have special built fields so we have semantic information.

Rather than treating this purely as diagnostic, what if we treated it as directly actionable metadata? If someone tries to install a crate, and it isn't installable because it's a library crate, and it has this field, directly install the mentioned crate instead.

We can decide that exact behavior later, but if we might want that behavior, then it doesn't seem like this belongs under a diagnostic namespace.

@joshtriplett I'd have to dig through our notes to fully remember why we were leaning towards diagnostics but one concern with this RFC as a whole is that we need to match the behavior with expectations in a way that avoids the expectations that would lead to creating a circular dependency between the library and the bin.

@epage Not following how this would go circular. It would only lead cargo from attempting to install the lib to actually installing the bin, and installing the bin would likely depend on the lib, but that's not circular. What's the scenario of concern?

(Not looking to retread a discussion that has already taken place, if this already has. I don't recall this aspect of the topic arising, though.)