feat(cargo-add): warn when adding binary-only packages - #17265
feat(cargo-add): warn when adding binary-only packages#17265raushan728 wants to merge 2 commits into
Conversation
|
r? @epage rustbot has assigned @epage. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Note that the linked Issue is a "needs mentor". In #11803 (comment), the right way to do this is said to be from the Index with a "It could be worked around" by downloading the package. What I would expect for a PR like this is a comment in the PR description (actually, it really should be in the Issue before making a PR) about why to go ahead with the discouraged route. |
Yes, I was aware of that.
I followed the suggested workaround. The registry index does not contain target information, so
I've updated the PR description. Specifically, it notes that the index lacks the information needed for this check, so downloading the resolver-selected package is necessary to determine whether a library target is present. |
|
Also, all tests pass locally for me, including the full Cargo testsuite. The CI failures I'm seeing are only snapshot differences where the output changes from My local toolchain is: 1.97.0 I'm not sure whether these should be treated as expected snapshot differences due to the toolchain version, or if there's something else I should investigate. |
This comment has been minimized.
This comment has been minimized.
Reuse the PackageId selected by Cargo's resolver instead of performing a second version lookup. This keeps the warning aligned with the resolver's MSRV and publish-age decisions while avoiding duplicatedversion selection logic and reducing the chance of warning about a different package version.
2c13a17 to
bb347b9
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| <tspan x="10px" y="118px"> | ||
| <tspan x="10px" y="118px"><tspan class="fg-bright-green bold"> Adding</tspan><tspan> my-package v99999.0.0 to dependencies</tspan> | ||
| </tspan> | ||
| <tspan x="10px" y="136px"><tspan class="fg-bright-green bold"> Locking</tspan><tspan> 1 package to highest Rust 1.97.1 compatible version</tspan> |
There was a problem hiding this comment.
Redactions for rust versions weren't preserved, causing test failures
|
I'm going to close to redirect back to the issue, particularly because I didn't go back up in the thread to the see the status of this and started looking into the details when that isn't the stage this is at. We can always re-open this if we decide on this direction. |
Emit a warning when
cargo addadds a package that does not contain a library target. The dependency is still added toCargo.toml, but Cargo warns that the package is unlikely to work as a dependency.Rationale
Users sometimes use
cargo addfor binary-only crates when they intended to install a CLI tool. Keeping this as a warning preserves the existing behavior while providing immediate feedback about a likely mistake.The registry index does not include target information, so
cargo addcannot determine whether a package exposes a library target from the index alone. Instead of waiting until a later build, this implementation follows the approach suggested in #11803 (comment) by downloading the resolver-selected package (when network access is available) and inspecting its manifest to determine whether it provides a library target.Closes #11803