-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Added documentation for the configuration discovery of cargo install
to the man pages
#11763
Conversation
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've written something pleasing to read! Thank you! However, this is probably a breaking change for someone depending on old cargo uninstall
behaviour. Could we split this PR into two, so we can approve cargo install
part and discuss the other half afterward?
BTW, I think we may want to first warn the user who depends on the behaviour of implicit config discovery in cargo uninstall
. After a transition period, we can then turn the warning into an error.
Of course. Never contributed to a project that can't make a breaking change, release a new version and call it a day 😅. I'll split the breaking changes and make a new PR. Maybe we could use that PR to also tackle the fact that cargo/src/bin/cargo/commands/install.rs Lines 93 to 94 in efd3733
Can you point me to a location in the source where cargo already has such a warning? Then I could create a PR for the warning as well (of course only if that is something you'd want). |
You'll find them by searching “hard error” or “transition period” in this repository, such as: cargo/src/cargo/ops/cargo_package.rs Lines 299 to 306 in 65cab34
|
cargo uninstall
should ignore local configuration chain and added documentation for that behaviour to cargo install
and cargo uninstall
.cargo install
to the man pages
cargo install
to the man pagescargo install
to the man pages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good! Thanks for the update :)
@bors r+ |
Added documentation for the configuration discovery of `cargo install` to the man pages Fixes #11660.
BTW, @jofas could you squash these commits so that we won't have things added and removed making the history confusing? No problem if you're unable to do that. |
Sure, I can squash the commits into a single one. Can you stop bors from merging? |
@bors r- |
BTW, if the bot detects more commits being pushed, it will cancel the merge automatically. |
Thanks, good to know. I squashed the commits. |
Thanks a lot! @bors r+ |
☀️ Test successful - checks-actions |
10 commits in 9d5b32f503fc099c4064298465add14d4bce11e6..9880b408a3af50c08fab3dbf4aa2a972df71e951 2023-02-22 23:04:16 +0000 to 2023-02-28 19:39:39 +0000 - bump jobserver to respect `--jobserver-auth=fifo:PATH` (rust-lang/cargo#11767) - Addition of support for -F as an alias for --features (rust-lang/cargo#11774) - Added documentation for the configuration discovery of `cargo install` to the man pages (rust-lang/cargo#11763) - Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml (rust-lang/cargo#11756) - Fix warning with tempfile (rust-lang/cargo#11771) - Error message for transitive artifact dependencies with targets the package doesn't directly interact with (rust-lang/cargo#11643) - Fix tests with nondeterministic ordering (rust-lang/cargo#11766) - Make some blocking tests non-blocking (rust-lang/cargo#11650) - Suggest cargo add when installing library crate (rust-lang/cargo#11410) - chore: bump is-terminal to 0.4.4 (rust-lang/cargo#11759)
Update cargo 10 commits in 9d5b32f503fc099c4064298465add14d4bce11e6..9880b408a3af50c08fab3dbf4aa2a972df71e951 2023-02-22 23:04:16 +0000 to 2023-02-28 19:39:39 +0000 - bump jobserver to respect `--jobserver-auth=fifo:PATH` (rust-lang/cargo#11767) - Addition of support for -F as an alias for --features (rust-lang/cargo#11774) - Added documentation for the configuration discovery of `cargo install` to the man pages (rust-lang/cargo#11763) - Fix Cargo removing the sparse+ prefix from sparse URLs in .crates.toml (rust-lang/cargo#11756) - Fix warning with tempfile (rust-lang/cargo#11771) - Error message for transitive artifact dependencies with targets the package doesn't directly interact with (rust-lang/cargo#11643) - Fix tests with nondeterministic ordering (rust-lang/cargo#11766) - Make some blocking tests non-blocking (rust-lang/cargo#11650) - Suggest cargo add when installing library crate (rust-lang/cargo#11410) - chore: bump is-terminal to 0.4.4 (rust-lang/cargo#11759) r? `@ghost`
Fixes #11660.