Support `cargo owner add` #11879

heisen-li · 2023-03-23T03:35:19Z

What does this PR try to resolve?

User confusion over how to invoke it, including

cargo owner doing nothing, rather than erroring and telling the user how to use it
Users assuming the interface uses subcommands, rather than action flags

This is done by deprecating the old interface and replacing it with subcommands.

To do this

cargo owner will now be synonymous with cargo owner --help except a bad error code will be returned
cargo owner --help includes a usage for each subcommand (Display help for all subcommands at the same time clap-rs/clap#1334 could do this for us)
--add, --list, and --remove are deprecated in favor of add, list, and remove
- Using a flag with a subcommand is a hard error
- The old flags are hidden in --help

This new interface loses the ability to do multiple operations at once but that seems like a premature optimization.

Additional information

Breaking change: cargo owner used to be a no-op (no actions specified) but will now be a hard error. This change is being made assuming the the use case for running cargo owner without any actions in a programmatic setting is small enough that this is acceptable

Fixes #4352

* upstream/master: docs: fix typos in `cargo_compile/mod.rs` docs(contrib): Replace architecture with redirects docs(contrub): Remove unused file docs(contrib): Move some lower resolver details from ops to core docs(contrib): Move higher level resolver docs into doc comments docs(contrib): Pull impl info out of architecture

rustbot · 2023-03-23T03:35:24Z

r? @weihanglo

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

* upstream/owner: (21 commits) Don't place side-effect expressions in assert! macros. Add `_MS` suffix to retry constants. Add some more docs and comments to `SleepTracker`. Add delays to network retries. Allow RegistryBuilder responder URLs to be a String Split the `cargo::util::network` module into submodules Add a note to `cargo logout` that it does not revoke the token. Sync external-tools JSON docs. Disable test_profile test on windows-gnu Drop derive feature from serde in cargo-platform a{n =>} benchmark target documented working directory behaviour of cargo-test, cargo-bench and cargo-run commands chore: Upgrade to clap v4.2 docs(contrib): Link to office hours doc doc(contrib): missing quotation mark Update changelog for 1.68.2 Add the old github keys as revoked Update proptest Added new GitHub RSA Host Key doc: Fix registries.name.index for sparse ...

heisen-li · 2023-04-02T09:23:55Z

So far, it seems that no one has pointed out any mistakes in the direction that I am taking with my current actions. Therefore, I completed the remaining tasks today.

The main modifications are:

1.Management of crate owners will be conducted using sub-commands instead of mandatory parameters, for example: cargo owner add/remove/list;
2.Executing cargo owner or cargo owner add/remove will prompt errors or return help information, which I believe will be helpful for users;
3.The previous functionality of using cargo owner --add someone1 --add someone2 has been retained. Now, you can use cargo owner add someone1, someone2 instead.
4.Test cases and corresponding documentation in cargo book have been modified.

Very welcome your suggestion. Thank you for your review.

src/bin/cargo/commands/owner.rs

* 'owner' of https://github.com/heisen-li/cargo: (360 commits) refactor(toml): Better abstract inheritance details Deduplicate the similar dependency. Update contrib docs on rustfix issue tracking. Add rustfix lib to autolabel triggers. Add a general introduction to the rustfix library docs. Ignore rustfix for semver-checks since it doesn't exist on the beta branch. typo: rusc -> rustc Handle $message_type in JSON diagnostics Fix --check-cfg invocation with zero features chore: bump cargo-credential-* crates as e58b84d broke stuff Remove copyright headers in tests. Fix tests to run on stable. Fix clippy warnings. contrib docs: Update now that credential crates are published. Add more resources to the contrib docs. Add rustfix to publish. Add optional flag to manifest for dependencies Add features to the default Cargo.toml file refactor(toml): Move accessor to be part of schema API fix(toml): Prevent workspace=false in API ...

src/etc/cargo.bashcomp.sh

epage · 2023-11-30T20:44:10Z

src/etc/_cargo

+                        '(add)'{add}'[specify name of a user or team to invite as an owner]:name' \
                        '--index=[specify registry index]:index' \
                        '(-l --list)'{-l,--list}'[list owners of a crate]' \
+                        '(list)'{list}'[list owners of a crate]' \
                        '(-r --remove)'{-r,--remove}'[specify name of a user or team to remove as an owner]:name' \
+                        '(remove)'{remove}'[specify name of a user or team to remove as an owner]:name' \


I suspect these will let us complete the subcommand names but not the arguments underneath? Or if they do, it'll report all the arguments underneath?

Unfortunately, it looks like zsh completions are missing cargo report future-incompat completions for us to copy from

Command completion for zsh is a bit difficult for me and I'm learning how to accomplish this.

I've tried to finish. In addition, whether cargo report future-incompat should be corrected in the new issue.

src/bin/cargo/commands/owner.rs

epage · 2023-11-30T20:51:56Z

src/bin/cargo/commands/owner.rs

+        _ => (
+            args.get_many::<String>("add")
+                .map(|xs| xs.cloned().collect::<Vec<String>>()),
+            args.get_many::<String>("remove")
+                .map(|xs| xs.cloned().collect()),
+            args.flag("list"),
+        ),


I assume this is meant to operate for None. We should separate out Some(_) and panic for it as we should only get the subcommands back we defined.

If there are no subcommands or any parameters(like：--add，..), an error message will be displayed instead of panic.

https://github.com/rust-lang/cargo/pull/11879/files#diff-981bc8fd9f47eff85de2c539a5be58b0002e2f060064a090218060c5ae3e5722R114-R122

bors · 2023-12-06T19:36:40Z

☔ The latest upstream changes (presumably #13117) made this pull request unmergeable. Please resolve the merge conflicts.

* 'owner' of https://github.com/heisen-li/cargo: resolve conflict resolve conflict

* 'owner' of https://github.com/heisen-li/cargo: fix failed tests

bors · 2024-01-02T17:10:55Z

☔ The latest upstream changes (presumably #13227) made this pull request unmergeable. Please resolve the merge conflicts.

heisen-li · 2024-01-08T08:26:30Z

@epage I'm sorry that this PR is taking too much of your effort, but I might have to ask, is there anything I need to revise further at the moment?

If the PR does not meet the community's acceptance rules, I can choose to close it. Please let me know if you need to modify it. All for the better cargo.

bors · 2024-02-20T19:56:49Z

☔ The latest upstream changes (presumably #13409) made this pull request unmergeable. Please resolve the merge conflicts.

weihanglo · 2024-10-04T02:40:47Z

I am going to close this as it has been stable for a while. The test infra in Cargo changed a lot during the period, so it is unavoidable to rework on it. Also, there were too many comments in this PR, making it hard to track the actual progress (Please stop folding everything GitHub!)

Feel free to discuss the design in #4352 and implementation details on Zulip t-cargo. Thank you!

heisen-li added 2 commits March 23, 2023 11:00

Support cargo owner add

786e0b6

rustbot assigned weihanglo Mar 23, 2023

rustbot added A-cli Area: Command-line interface, option parsing, etc. A-interacts-with-crates.io Area: interaction with registries Command-owner S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2023

heisen-li added 2 commits March 23, 2023 11:35

Delete res.txt

5862698

Update registry.rs

8914b1a

heisen-li changed the title ~~[WIP] Support cargo owner add~~ [WIP] Support cargo owner add Mar 23, 2023

heisen-li and others added 3 commits April 2, 2023 16:39

Merge branch 'rust-lang:master' into owner

6573a9e

Added test cases and modified document descriptions.

baf3020

rustbot added the A-documenting-cargo-itself Area: Cargo's documentation label Apr 2, 2023

fix doc

efaf17b

heisen-li force-pushed the owner branch from e39120a to efaf17b Compare April 2, 2023 09:01

heisen-li changed the title ~~[WIP] Support cargo owner add~~ Support cargo owner add Apr 2, 2023

Merge branch 'rust-lang:master' into owner

6037751