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.

Sign up for GitHub

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

Jump to bottom

fix(update): Make -p more convenient by being positional #12545

Merged
merged 2 commits into from
Aug 29, 2023
Merged

fix(update): Make -p more convenient by being positional #12545

merged 2 commits into from
Aug 29, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Aug 23, 2023

Generally, cargo avoids positional arguments. Mostly for the commands that might forward arguments to another command, like cargo test. It also allows some flexibility in turning flags into options.

For cargo add and cargo remove, we decided to accept positionals because the motivations didn't seem to apply as much (similar to cargo install).

This applies the pattern to cargo update as well which is in the same category of commands as cargo add and cargo remove.

As for --help formatting, I'm mixed on whether [SPEC]... should be at the top like other positionals or should be relegated to "Package selection". I went with the latter mostly to make it easier to visualize the less common choice.

Switching to a positional for cargo update (while keeping -p for backwards compatibility) was referenced in #12425.

@epage
fix(update): Make -p more convenient by being positional
4bf1af0 
Generally, cargo avoids positional arguments.  Mostly for the commands
that might forward arguments to another command, like `cargo test`.
It also allows some flexibility in turning flags into options.

For `cargo add` and `cargo remove`, we decided to accept positionals
because the motivations didn't seem to apply as much (similar to `cargo
install`).

This applies the pattern to `cargo update` as well which is in the same
category of commands as `cargo add` and `cargo remove`.

As for `--help` formatting, I'm mixed on whether `[SPEC]...` should be at the top like
other positionals or should be relegated to "Package selection".  I went
with the latter mostly to make it easier to visualize the less common
choice.

Switching to a positional for `cargo update` (while keeping `-p` for
backwards compatibility) was referenced in rust-lang#12425.
@rustbot
Copy link
Collaborator

rustbot commented Aug 23, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-completions Area: shell completions A-documenting-cargo-itself Area: Cargo's documentation A-registries Area: registries Command-update S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2023
@epage epage force-pushed the positional branch 2 times, most recently from 4ca5b9b to 99b0ecb Compare August 23, 2023 19:10
@epage
fix(update): Improve error on bad argument order
dc37222 
This is inspired by `cargo install`
@epage epage added the T-cargo Team: Cargo label Aug 24, 2023
@epage
Copy link
Contributor Author

epage commented Aug 24, 2023

This was mentioned in #12425 but because it was more of an aside, I want to do a straw poll to make sure people are on board specifically with this before merging as it is a one-way door.

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Aug 24, 2023 •
edited by joshtriplett
Loading

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Aug 24, 2023
@epage
Copy link
Contributor Author

epage commented Aug 24, 2023

when running cargo tree gix-actor, I realized we should probably do the same thing there

@rfcbot rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Aug 24, 2023
@rfcbot
Copy link
Collaborator

rfcbot commented Aug 24, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@weihanglo
Copy link
Member

when running cargo tree gix-actor, I realized we should probably do the same thing there

I may hold off this now. cargo tree has -i and -d flags, which make things a bit complicated.

@epage epage marked this pull request as ready for review August 25, 2023 01:16
@epage
Copy link
Contributor Author

epage commented Aug 25, 2023

I may hold off this now. cargo tree has -i and -d flags, which make things a bit complicated.

How so?

@weihanglo
Copy link
Member

Just checked what we have today in cargo-tree. It is already confusing.

$ cargo tree -p resolver-tests -i tracing
tracing v0.1.37
├── cargo v0.75.0 (/projects/cargo)
│   └── resolver-tests v0.0.0 (/projects/cargo/crates/resolver-tests)
├── cargo-util v0.2.7 (/projects/cargo/crates/cargo-util)
│   ├── cargo v0.75.0 (/projects/cargo) (*)
│   └── resolver-tests v0.0.0 (/projects/cargo/crates/resolver-tests)
└── tracing-subscriber v0.3.17
    └── cargo v0.75.0 (/projects/cargo) (*)

This command prints an inverted tree with tracing as the root, and… without looking into code I can't even understand how to interpret the effect of --package in the graph.

They are completely different issues, so not a blocker :)

@epage
Copy link
Contributor Author

epage commented Aug 25, 2023

Oh man, I forgot -i could accept a package or use the -p package. Yeah, thats a no-go

weihanglo
weihanglo reviewed Aug 28, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

Looks good. Just one trailing backslash is missing 😆.

src/etc/_cargo Outdated Show resolved Hide resolved
@epage
Copy link
Contributor Author

epage commented Aug 29, 2023

@bors r=weihanglo

@bors
Copy link
Contributor

bors commented Aug 29, 2023

📌 Commit dc37222 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit dc37222 with merge 94770a5...

@bors
Copy link
Contributor

bors commented Aug 29, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 94770a5 to master...

@bors bors merged commit 94770a5 into rust-lang:master Aug 29, 2023
14 of 20 checks passed
@epage epage deleted the positional branch August 29, 2023 02:45
@bors bors mentioned this pull request Aug 29, 2023
Merged
epage added a commit to epage/cargo that referenced this pull request Aug 29, 2023
@epage
fix(update): Remove references to -p in help
66f0b13 
Missed these in rust-lang#12545
@epage epage mentioned this pull request Aug 29, 2023
Merged
bors added a commit that referenced this pull request Aug 29, 2023
@bors
Auto merge of #12586 - epage:positional, r=weihanglo
87a14ed 
fix(update): Remove references to -p in help

Missed these in #12545
@weihanglo weihanglo mentioned this pull request Aug 29, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2023
@bors
Auto merge of rust-lang#115365 - weihanglo:update-cargo, r=weihanglo
6e8f677 
Update cargo

18 commits in 925280f028db3a322935e040719a0754703947cf..96fe1c9e1aecd8f57063e3753969bb6418fd2fd5
2023-08-25 21:16:44 +0000 to 2023-08-29 20:10:34 +0000
- fix(lints): Fail when overriding inherited lints (rust-lang/cargo#12584)
- cargo install: suggest --git when package name is url (rust-lang/cargo#12575)
- chore: remove unstable-options for logout (rust-lang/cargo#12588)
- Improve logout message for asymmetric tokens (rust-lang/cargo#12587)
- fix(update): Remove references to -p in help (rust-lang/cargo#12586)
- fix(update): Make `-p` more convenient by being positional (rust-lang/cargo#12545)
- Set tracing target for networking messages. (rust-lang/cargo#12582)
- Retry docs (rust-lang/cargo#12583)
- feat(resolver): **Very** preliminary MSRV resolver support (rust-lang/cargo#12560)
- Update git2 (rust-lang/cargo#12580)
- Explain how `version` works for `git` dependencies (rust-lang/cargo#12270)
- Improve deserialization errors of untagged enums (rust-lang/cargo#12574)
- Add support for `target.'cfg(..)'.linker` (rust-lang/cargo#12535)
- Improve resolver version mismatch warning (rust-lang/cargo#12573)
- Stabilize `--keep-going` (rust-lang/cargo#12568)
- Define {{command}} for use in src/doc/man/includes (rust-lang/cargo#12570)
- Update serde (rust-lang/cargo#12569)
- chore: add missing `windows-sys` features back (rust-lang/cargo#12563)

r? ghost
@rfcbot rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Sep 3, 2023
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 8, 2024
@he32
rust: update to version 1.74.1.
1fca108 
Pkgsrc changes:

 * Remove NetBSD-8 support (embedded LLVm requires newer C++
   than what is in -8; it's conceivable that this could still
   build with an external LLVM)
 * undo powerpc 9.0 file naming tweak, since we no longer support -8.
 * Remove patch to LLVM for powerpc now included by upstream.
 * Minor adjustments, checksum changes etc.


Upstream changes:

Version 1.74.1 (2023-12-07)
===========================

- [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM]
  (rust-lang/rust#118464)
- [Clarify guarantees for std::mem::discriminant]
  (rust-lang/rust#118006)
- [Fix some subtyping-related regressions]
  (rust-lang/rust#116415)

Version 1.74.0 (2023-11-16)
==========================

Language
--------

- [Codify that `std::mem::Discriminant<T>` does not depend on any
  lifetimes in T]
  (rust-lang/rust#104299)
- [Replace `private_in_public` lint with `private_interfaces` and
  `private_bounds` per RFC 2145]
  (rust-lang/rust#113126)
  Read more in
  [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html).
- [Allow explicit `#[repr(Rust)]`]
  (rust-lang/rust#114201)
- [closure field capturing: don't depend on alignment of packed fields]
  (rust-lang/rust#115315)
- [Enable MIR-based drop-tracking for `async` blocks]
  (rust-lang/rust#107421)

Compiler
--------

- [stabilize combining +bundle and +whole-archive link modifiers]
  (rust-lang/rust#113301)
- [Stabilize `PATH` option for `--print KIND=PATH`]
  (rust-lang/rust#114183)
- [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`]
  (rust-lang/rust#115644)
- [Promote loongarch64-unknown-none* to Tier 2]
  (rust-lang/rust#115368)
- [Add `i686-pc-windows-gnullvm` as a tier 3 target]
  (rust-lang/rust#115687)

Libraries
---------

- [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err]
  (rust-lang/rust#98704)
- [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`]
  (rust-lang/rust#111278)
- [impl Step for IP addresses]
  (rust-lang/rust#113748)
- [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`]
  (rust-lang/rust#114041)
- [`impl TryFrom<char> for u16`]
  (rust-lang/rust#114065)
- [Stabilize `io_error_other` feature]
  (rust-lang/rust#115453)
- [Stabilize the `Saturating` type]
  (rust-lang/rust#115477)
- [Stabilize const_transmute_copy]
  (rust-lang/rust#115520)

Stabilized APIs
---------------

- [`core::num::Saturating`]
  (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html)
- [`impl From<io::Stdout> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio)
- [`impl From<io::Stderr> for std::process::Stdio`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`]
  (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio)
- [`std::ffi::OsString::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsString::into_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes)
- [`std::ffi::OsStr::from_encoded_bytes_unchecked`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked)
- [`std::ffi::OsStr::as_encoded_bytes`]
  (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes)
- [`std::io::Error::other`]
  (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other)
- [`impl TryFrom<char> for u16`]
  (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16)
- [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`]
  (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E)
- [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E)

These APIs are now stable in const contexts:

- [`core::mem::transmute_copy`]
  (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html)
- [`str::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii)
- [`[u8]::is_ascii`]
  (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii)

Cargo
-----

- [fix: Set MSRV for internal packages]
  (rust-lang/cargo#12381)
- [config: merge lists in precedence order]
  (rust-lang/cargo#12515)
- [fix(update): Clarify meaning of --aggressive as --recursive]
  (rust-lang/cargo#12544)
- [fix(update): Make `-p` more convenient by being positional]
  (rust-lang/cargo#12545)
- [feat(help): Add styling to help output ]
  (rust-lang/cargo#12578)
- [feat(pkgid): Allow incomplete versions when unambigious]
  (rust-lang/cargo#12614)
- [feat: stabilize credential-process and registry-auth]
  (rust-lang/cargo#12649)
- [feat(cli): Add '-n' to dry-run]
  (rust-lang/cargo#12660)
- [Add support for `target.'cfg(..)'.linker`]
  (rust-lang/cargo#12535)
- [Stabilize `--keep-going`]
  (rust-lang/cargo#12568)
- [feat: Stabilize lints]
  (rust-lang/cargo#12648)

Rustdoc
-------

- [Add warning block support in rustdoc]
  (rust-lang/rust#106561)
- [Accept additional user-defined syntax classes in fenced code blocks]
  (rust-lang/rust#110800)
- [rustdoc-search: add support for type parameters]
  (rust-lang/rust#112725)
- [rustdoc: show inner enum and struct in type definition for concrete type]
  (rust-lang/rust#114855)

Compatibility Notes
-------------------

- [Raise minimum supported Apple OS versions]
  (rust-lang/rust#104385)
- [make Cell::swap panic if the Cells partially overlap]
  (rust-lang/rust#114795)
- [Reject invalid crate names in `--extern`]
  (rust-lang/rust#116001)
- [Don't resolve generic impls that may be shadowed by dyn built-in impls]
  (rust-lang/rust#114941)

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

None this cycle.
@epage epage mentioned this pull request Feb 9, 2024
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo left review comments

Assignees

@ehuss ehuss

Labels
A-cli Area: Command-line interface, option parsing, etc. A-cli-help Area: built-in command-line help A-completions Area: shell completions A-documenting-cargo-itself Area: Cargo's documentation A-registries Area: registries Command-update disposition-merge FCP with intent to merge finished-final-comment-period FCP complete S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-cargo Team: Cargo to-announce
Projects
None yet
Milestone
1.74.0
Development

Successfully merging this pull request may close these issues.
6 participants
@epage @rustbot @rfcbot @weihanglo @bors @ehuss