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

Generate shell completions for bootstrap with Clap #111388

Merged
merged 1 commit into from
May 14, 2023

Conversation

clubby789
Copy link
Contributor

Now that #110693 has been merged, we can look at generating shell completions for x.py with clap_complete. Leaving this as draft for now as I'm not sure of the best way to integration the completion generator. Additionally, the generated completions for zsh are completely broken (will need to be resolved upstream, it doesn't seem to handle subcommands + global arguments well).
I don't have Fish installed and would be interested to know how well completions work there.

Alternative to #107827

@rustbot
Copy link
Collaborator

rustbot commented May 9, 2023

r? @Mark-Simulacrum

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels May 9, 2023
@jyn514
Copy link
Member

jyn514 commented May 9, 2023

Can we support powershell here too?

I saw you're only completing subcommands - now that this is part of bootstrap proper, can we support paths as well?

also, I think rather than an env variable we should commit this somewhere in etc/ and have a check in x test tidy that makes sure it's up to date.

@clubby789
Copy link
Contributor Author

Paths, as well as the -- type flags are completed, at least testing under bash. I can add a branch for powershell too since that's supported. As this just hints that arguments may be of type 'file' to bash, completions won't work for, e.g. completing paths from the root while in a subdirectory

@jyn514
Copy link
Member

jyn514 commented May 9, 2023

Sorry, "paths" is the wrong word - I meant using the should_run impls, so that e.g. x dist l completes to x dist llvm-tools.

@clubby789
Copy link
Contributor Author

Ah, I see - we could potentially do some post-processing on the output but that seems like it could be a bit brittle. It might be possible to tweaks the Flags structure a bit so that it can suggest files or those paths

@jyn514
Copy link
Member

jyn514 commented May 9, 2023

👍 if it it's tricky i'm also happy to land this as it, it already seems like a big improvement :)

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label May 9, 2023
@clubby789 clubby789 marked this pull request as ready for review May 9, 2023 14:13
@clubby789 clubby789 changed the title WIP: Generate shell completions for bootstrap with Clap Generate shell completions for bootstrap with Clap May 9, 2023
@clubby789
Copy link
Contributor Author

Added a new x run step for this and check in tidy that they're unchanged. Disabled zsh for now

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

if let Some(comp) = get_completion(shells::PowerShell, &powershell) {
std::fs::write(&powershell, comp).expect("writing powershell completion");
}
// FIXME: zsh generated completions don't seem to work well with subcommands
Copy link
Member

Choose a reason for hiding this comment

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

Can we report this upstream to clap_complete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look at properly diagnosing and reporting the issue today

Copy link
Contributor Author

@clubby789 clubby789 May 10, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even after fixing this error, flags don't seem to be being suggested after the subcommand and the path completion isn't working at all 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merged now, I'll investigate the remaining issues with Zsh soon. I'll open a follow-up bumping the clap_complete version when I have a fix

src/etc/completions/x.sh Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@clubby789 clubby789 force-pushed the clap-complete branch 2 times, most recently from e73ea97 to 4a3e651 Compare May 9, 2023 19:58
src/bootstrap/test.rs Outdated Show resolved Hide resolved
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

this is awesome, thank you!

@clubby789 clubby789 force-pushed the clap-complete branch 2 times, most recently from 9f5ebec to 307f057 Compare May 10, 2023 19:01
@jyn514
Copy link
Member

jyn514 commented May 10, 2023

@bors r+

This already seems like a big improvement, we can iterate in follow-up PRs :)

Could you also make a PR documenting in this in the dev-guide, maybe around https://rustc-dev-guide.rust-lang.org/building/suggested.html ?

@bors
Copy link
Contributor

bors commented May 10, 2023

📌 Commit 307f0576129a463533a65ec3f8fa35b9da53b62f has been approved by jyn514

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 10, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@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 May 10, 2023
@clubby789
Copy link
Contributor Author

I've just updated the filenames to x.py.{sh,fish,ps1}, as this is more conventional (the completions are registed to x.py, not x).
It may also be useful to duplicate the completions for x and x.py, but using x.py for now as it's the common entrypoint

@jyn514
Copy link
Member

jyn514 commented May 10, 2023

@bors r- r+

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 10, 2023
@bors
Copy link
Contributor

bors commented May 10, 2023

📌 Commit a348f8a has been approved by jyn514

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 10, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 10, 2023
@bors
Copy link
Contributor

bors commented May 13, 2023

⌛ Testing commit a348f8a with merge db5d56a3714ee1602c47c6303186fadc5b23b268...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-mingw-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [rustdoc] tests\rustdoc\inline_cross\impl-inline-without-trait.rs stdout ----

error: rustdoc failed!
status: exit code: 0xc00000fd
command: PATH="C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0-bootstrap-tools\x86_64-pc-windows-gnu\release\deps;C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0\bin;C:\a\rust\rust\ninja;C:\a\rust\rust\mingw64\bin;C:\hostedtoolcache\windows\Python\3.11.3\x64\Scripts;C:\hostedtoolcache\windows\Python\3.11.3\x64;C:\msys64\usr\bin;C:\a\rust\rust\sccache;C:\Program Files\MongoDB\Server\5.0\bin;C:\aliyun-cli;C:\vcpkg;C:\cf-cli;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\2.9.3\x64;C:\cabal\bin;C:\ghcup\bin;C:\Program Files\dotnet;C:\mysql\bin;C:\Program Files\R\R-4.3.0\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.20.3\x64\bin;C:\hostedtoolcache\windows\Python\3.7.9\x64\Scripts;C:\hostedtoolcache\windows\Python\3.7.9\x64;C:\hostedtoolcache\windows\Ruby\2.5.9\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.372-7\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\Program Files\Eclipse Foundation\jdk-8.0.302.8-hotspot\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\Chocolatey\bin;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\150\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\TortoiseSVN\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.8.7\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\SeleniumWebDrivers\ChromeDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files (x86)\Microsoft BizTalk Server;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustdoc.exe" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\lib\\rustlib\\x86_64-pc-windows-gnu\\lib" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\inline_cross\\impl-inline-without-trait\\auxiliary\\impl-inline-without-trait\\auxiliary" "-o" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\rustdoc\\inline_cross\\impl-inline-without-trait" "--deny" "warnings" "C:\\a\\rust\\rust\\tests\\rustdoc\\inline_cross\\auxiliary\\impl-inline-without-trait.rs"
stdout: none
thread 'main' has overflowed its stack
------------------------------------------


---
test result: FAILED. 614 passed; 1 failed; 6 ignored; 0 measured; 0 filtered out; finished in 107.85s

Some tests failed in compiletest suite=rustdoc mode=rustdoc host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
Build completed unsuccessfully in 0:49:11
make: *** [Makefile:78: ci-mingw-subset-1] Error 1

@bors
Copy link
Contributor

bors commented May 13, 2023

💔 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 May 13, 2023
@clubby789
Copy link
Contributor Author

@bors retry

@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 May 13, 2023
@bors
Copy link
Contributor

bors commented May 14, 2023

⌛ Testing commit a348f8a with merge 2e18605...

@bors
Copy link
Contributor

bors commented May 14, 2023

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 2e18605 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 14, 2023
@bors bors merged commit 2e18605 into rust-lang:master May 14, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 14, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2e18605): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.4% [-1.5%, -1.3%] 2
Improvements ✅
(secondary)
-2.3% [-2.3%, -2.3%] 1
All ❌✅ (primary) -1.4% [-1.5%, -1.3%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
6.3% [1.7%, 11.3%] 6
Improvements ✅
(primary)
-1.1% [-1.1%, -1.1%] 1
Improvements ✅
(secondary)
-9.3% [-9.7%, -9.0%] 3
All ❌✅ (primary) -1.1% [-1.1%, -1.1%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 660.195s -> 659.769s (-0.06%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants