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.

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

Fix #9350 (cargo build -Z help is missing options) #9369

Closed
wants to merge 15 commits into from

Conversation

PicoJr
Copy link

@PicoJr PicoJr commented Apr 17, 2021

Do not merge yet, some options are still undocumented.

Fix #9350 (cargo build -Z help is missing options)

Add a procedural macro to declare CliUnstable struct and provide help messages instead of hard-coding help in src/bin/cargo/cli.rs

Flags documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html

Feedback welcome

* Add automatic padding when displaying options.
* Update test.
* run cargo fmt.
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 17, 2021
tests/testsuite/help.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
@ehuss
Copy link
Contributor

ehuss commented Apr 18, 2021

r? ehuss

Looking good! A few things:

  • The underscores need to be converted to dashes when displayed in the help.
  • Some of these options should be hidden. I think it would be fine to have an empty string mean hidden, and to maybe use a const HIDDEN to make that clearer. I think the ones that should be hidden are:
    • print_im_a_teapot
    • advanced_env
    • features
    • separate_nightlies
  • Can you write small help strings for the entries that are empty? There is some documentation at https://doc.rust-lang.org/nightly/cargo/reference/unstable.html that should cover most of them (unfortunately some are missing). I can help with any that aren't clear.

@ehuss ehuss assigned ehuss and unassigned Eh2406 Apr 18, 2021
PicoJr and others added 2 commits April 18, 2021 20:41
* add HIDDEN constant, do not display hidden options
* write small help strings for empty entries when possible (else write TODO)
* convert snake_case to kebab-case when displaying options
* update test
@PicoJr
Copy link
Author

PicoJr commented Apr 18, 2021

Entries not meant to be hidden and without a help message contain "TODO" for now.

https://doc.rust-lang.org/nightly/cargo/reference/unstable.html does not provide a documentation for those entries.

I'm not confident with the help messages for:

  • binary-dep-depinfo
  • panic-abort-tests

@ehuss thanks for the review :)

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

I think this uncovered an issue where the CLI option -Z future-incompat-report doesn't match the field name enable_future_incompat_feature. I would probably prefer to just rename the field to match. I think its best if all the options match the actual cli name anyways.

src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
src/cargo/core/features.rs Outdated Show resolved Hide resolved
@PicoJr
Copy link
Author

PicoJr commented Apr 19, 2021

Thanks!

I think this uncovered an issue where the CLI option -Z future-incompat-report doesn't match the field name enable_future_incompat_feature. I would probably prefer to just rename the field to match. I think its best if all the options match the actual cli name anyways.

I'll rename this field to match the option name.

Rename the field so that it has the same name as the corresponding
unstable feature.
@ehuss
Copy link
Contributor

ehuss commented Apr 19, 2021

Sorry, one last thing that I noticed. Can the first few lines be moved into the macro definition itself? Similar to the features! macro in the same file, the struct itself can be defined in the macro. That should simplify it some so it doesn't need to match on all those elements (this macro won't be reused, so it's fine to embed the struct name inside it). I would also probably remove the pub prefixes, since every field is pub there isn't a need to repeat that in the macro call, and just force every field to be pub in the definition.

@ehuss
Copy link
Contributor

ehuss commented Apr 20, 2021

Sweet, thank you very much!

@bors r+ squash

@bors
Copy link
Collaborator

bors commented Apr 20, 2021

📌 Commit 4f8f80d has been approved by ehuss

@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 Apr 20, 2021
@bors
Copy link
Collaborator

bors commented Apr 20, 2021

⌛ Testing commit 4f8f80d with merge c51e3338e0a515425891a211e55e6632bec64b0f...

@bors
Copy link
Collaborator

bors commented Apr 20, 2021

💔 Test failed - checks-actions

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

ehuss commented Apr 20, 2021

Oops, sorry, forgot that the docs should be updated. I pushed a small fix for that.

@bors r+ squash

@bors
Copy link
Collaborator

bors commented Apr 20, 2021

📌 Commit 1cbfced has been approved by ehuss

@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 Apr 20, 2021
@bors
Copy link
Collaborator

bors commented Apr 20, 2021

⌛ Testing commit 1cbfced with merge a18936b...

bors added a commit that referenced this pull request Apr 20, 2021
Fix #9350 (cargo build -Z help is missing options)

> Do not merge yet, some options are still undocumented.

Fix #9350 (cargo build -Z help is missing options)

Add a procedural macro to declare `CliUnstable` struct and provide help messages instead of hard-coding help in `src/bin/cargo/cli.rs`

> Flags documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html

Feedback welcome
@bors
Copy link
Collaborator

bors commented Apr 20, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing a18936b to master...

@ehuss
Copy link
Contributor

ehuss commented Apr 20, 2021

Closing since bors didn't auto-close (see rust-lang/homu#136).

@ehuss ehuss closed this Apr 20, 2021
@PicoJr PicoJr deleted the fix-9350 branch April 21, 2021 09:43
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 24, 2021
Update cargo, rls

## cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000
- Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397)
- Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392)
- Update changelog for 1.52 beta changes. (rust-lang/cargo#9396)
- Fix build-std updating the index on every build. (rust-lang/cargo#9393)
- Fix typo in profile docs (rust-lang/cargo#9386)
- Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384)
- Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365)
- Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369)
- an struct -> a struct (rust-lang/cargo#9379)
- Handle man pages better on Windows. (rust-lang/cargo#9378)
- fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368)
- Fix typo in book (rust-lang/cargo#9376)
- Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348)
- doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372)
- refactor: remove `CargoResultExt` (rust-lang/cargo#9367)
- Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363)
- Update clippy lint allow set. (rust-lang/cargo#9356)
- Fix 'suport' typo in documentation (rust-lang/cargo#9338)

## rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000
- Bump default integration test message timeout to 30s (rust-lang/rls#1731)
- itertools = 0.9, fst = 0.4 (rust-lang/rls#1729)
- Update cargo (rust-lang/rls#1728)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 24, 2021
Update cargo, rls

## cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000
- Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397)
- Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392)
- Update changelog for 1.52 beta changes. (rust-lang/cargo#9396)
- Fix build-std updating the index on every build. (rust-lang/cargo#9393)
- Fix typo in profile docs (rust-lang/cargo#9386)
- Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384)
- Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365)
- Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369)
- an struct -> a struct (rust-lang/cargo#9379)
- Handle man pages better on Windows. (rust-lang/cargo#9378)
- fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368)
- Fix typo in book (rust-lang/cargo#9376)
- Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348)
- doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372)
- refactor: remove `CargoResultExt` (rust-lang/cargo#9367)
- Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363)
- Update clippy lint allow set. (rust-lang/cargo#9356)
- Fix 'suport' typo in documentation (rust-lang/cargo#9338)

## rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000
- Bump default integration test message timeout to 30s (rust-lang/rls#1731)
- itertools = 0.9, fst = 0.4 (rust-lang/rls#1729)
- Update cargo (rust-lang/rls#1728)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 24, 2021
Update cargo, rls

## cargo

18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1
2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000
- Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397)
- Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392)
- Update changelog for 1.52 beta changes. (rust-lang/cargo#9396)
- Fix build-std updating the index on every build. (rust-lang/cargo#9393)
- Fix typo in profile docs (rust-lang/cargo#9386)
- Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384)
- Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365)
- Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369)
- an struct -> a struct (rust-lang/cargo#9379)
- Handle man pages better on Windows. (rust-lang/cargo#9378)
- fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368)
- Fix typo in book (rust-lang/cargo#9376)
- Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348)
- doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372)
- refactor: remove `CargoResultExt` (rust-lang/cargo#9367)
- Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363)
- Update clippy lint allow set. (rust-lang/cargo#9356)
- Fix 'suport' typo in documentation (rust-lang/cargo#9338)

## rls

3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d
2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000
- Bump default integration test message timeout to 30s (rust-lang/rls#1731)
- itertools = 0.9, fst = 0.4 (rust-lang/rls#1729)
- Update cargo (rust-lang/rls#1728)
@ehuss ehuss added this to the 1.53.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo build -Z help is missing options
5 participants