-
Notifications
You must be signed in to change notification settings - Fork 892
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
Windows Terminal Glitch #3430
Comments
From my testing it seems like the following need to be in place for it to happen:
|
I have experienced both people that cancelled the rustup install while the dev tools where being installed (because they take so long) then restarted it, and people that already had visual studio installed. |
I have made a issue on the Windows Terminal repository microsoft/terminal#15811. |
After some testing with modified |
Only the first 3 bullet points are true for me, but I can still reproduce. The only workarounds I have found are changing the default terminal application to conhost, and changing the default launch size to something more than 30 rows. |
Yeah, I was surprised when I wasn't able to get it to happen with just a clean install of Windows 11, but I am not surprised its not actually required for it to happen. |
if cfg!(target_os = "windows") {
println!("Loading...");
std::thread::sleep(std::time::Duration::from_secs_f64(0.5));
} Inserted in |
Here is how that would look to the user. 2023-08-08.23-01-48.mp4 |
This is microsoft/terminal#14512. The workaround on the rustup side is to not fill up the console with text on startup. My suggestion would be to show the first part of the welcome message followed by a "press enter to continue" prompt. This naturally introduces a pause without seeming too artificial. |
@ChrisDenton that bug says fixed in v1.18 - that fixes it for Rustup too right? If so, can rustup detect older versions being used readily? If yes, we could detect that and emit a warning. If no, we could pin this bug or one of the older ones with a note that people need to upgrade. |
I guess we could lookup the parent process, find the file name and read the file version. But that strikes me as brittle and overly complicated for an issue that should go away with time. Also note that v1.18 hasn't had a stable release yet so it'd be a bit early to suggest updating. |
I can confirm that the problem is reproducible with Terminal (v1.17), but it goes away with Terminal Preview (v1.18). @konnorandrews suggestion for a slight delay on Windows makes the most sense to me, at least until we know that v1.18 has had a stable release. A blanket approach has practically no downsides, and we don't gain much from detecting the version of Terminal at runtime. In the meantime, I am going to investigate how short we can make the delay without causing the so-called terminal glitch. |
This problem will self resolve itself when the terminal is fixed. Rustup proxy execution time is critical path for building rust code, I don't think we should add a delay on startup. Yes, we could add it just to the installer, but if we then find proxies exhibiting the same fault, we couldn't use the same approach to 'fix it'. How long until ms terminal releases? |
I don't think the suggestion was to add an unconditional delay for every proxy binary, just the installer. The bug can be triggered only under very narrow circumstances: specifically, a Win11 user launching the installer from a desktop session (i.e. double-clicking on the binary), bringing up a new Terminal tab.
Why not? I agree it is inelegant but I don't see why it couldn't be done |
Because we'd dramatically slow down compilation times on Windows for everyone, and I judge that tradeoff to not be worth it.
A build of a crate with 100 transitive dependencies runs rustc at least 100 times, and rustup proxy performance has been measured as significantly affecting build times; we're working towards taking the proxy out of the critical path for builds by having As the workaround is really simple, I think documenting that (install via a different terminal) is sufficient. |
@rbtcollins I don't see how This issue is specific to the initial install menu, since that is where it creates the most confusion. This install menu is only shown during a manual installation, since automated installations will pass flags to skip this step. Therefore there is no impact on build times or automated installations of rust caused by adding a sleep before printing this menu. I would suggest adding a (very small) sleep before printing the installation instructions on windows. This can be further targeted by checking for the |
@Diggsey I was answering the quoted question and trying to give more context. w.r.t WT_SESSION that appears to be unreliable according to this discussion. One of the corner cases mentioned is running within vscode, which I think is probably quite likely as a combined interaction here. You say this is causing 'most confusion' - how much confusion is it causing? I haven't seen it show up in e.g. reddit discussions. Looking at the Windows Terminal release history, it seems like they are quite overdue for a release at this point - presumably northern hemisphere holidays has impacted them. I recognise that adding a sleep to rustup's install codepath is pragmatic, but it really feels bad - its the accumulation of such workarounds that makes increases cognitive load over time. Regardless, I don't have time to crank the release handle for probably a couple of weeks at this point, so if we do get to a consensus that we should add such a workaround, it will still be a while before we can look at shipping it. |
@rbtcollins some of your questions were answered by the folks over on the issue filed for Terminal, I'll summarize below.
I didn't get a positive response on this so it seems unlikely, we are awaiting clarification.
About a month from now. There is a chance they may service this bugfix back to 1.17, but I don't know if that would beat the 1.18 release. |
Windows Terminal 1.18 is available today. Do you think the issue can be safely closed now, @konnorandrews? cc @ryanavella |
As there are currently no further actions that can be taken from our side with respect to this issue, I'm closing it now as stale. Please feel free to reopen if you have more information to provide. Have a nice day :) |
Problem
Continuation of #3314. I have been able to find a sequence of steps to reliably reproduce the issue. The issue in question is that rustup doesn't show it's install menu in the command line when run under a very particular circumstance. I am currently unsure if this is something rustup is actually at fault for or if its all because of Windows Terminal.
The issue manifests as the following output to the user.
I have had to help multiple people with this issue (the fix for anyone looking is to press 1 and then enter as if the menu was there). However, it effects a very small number of people as I will outline below. It does cause a large amount of confusion though because users believe rustup has finished it's install when it really hasn't.
Steps
These steps are documented from a clean installation of Windows 11 in a VM, but it can be reproduced on an existing bare metal install.
Win11_22H2_English_x64v2.iso
)If this step doesn't happen then a different behavior happens where the menu is just duplicated not missing.
rustup
(or use a locally built copy)rustup
install exerustup
has continued its installation.rustup
at the menu.rustup
install exe (to continue the installation)The
rustup
install terminal should now be incorrectly displaying duplicated text about the environment variables.Possible Solution(s)
I am currently unsure of the root cause. I am filling this issue because it has caused serious issues for the people I have helped. In some cases multiple hours of trying to figure out why it won't install anything. This has caused a bad first impression for multiple people.
Notes
I haven't yet found an example of this type of behavior in Windows Terminal that isn't
rustup
, but I also don't use Windows 11 enough to have a good sample set. I have also not heard of anyone experiencing issues after this one menu ofrustup
.Rustup version
Seems to affect all versions including the latest.
Installed toolchains
None
The text was updated successfully, but these errors were encountered: