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

rustup-init leaves user with colored terminal #2590

Open
Vbbab opened this issue Dec 1, 2020 · 29 comments · May be fixed by #2669
Open

rustup-init leaves user with colored terminal #2590

Vbbab opened this issue Dec 1, 2020 · 29 comments · May be fixed by #2669
Labels
Milestone

Comments

@Vbbab
Copy link

Vbbab commented Dec 1, 2020

(If this doesn't belong here, please tell me which repo to move it to, thanks!)

Problem

rustup's setup script, rustup-init, leaves the user with a colored terminal when exiting interrupted.

Steps

  1. Execute rustup-init
  2. At the first prompt, interrupt by Ctrl-C
  3. User's shell is now red.

Possible Solution(s)
Check the interrupted-exit code maybe? Make sure that the color is reset before exiting (Since ^C is displayed in red)

Notes

OS: Win 10 (1909)
Output of rustup --version: rustup 1.23.0 (00924c9ba 2020-11-27)
Output of rustup-init --version: rustup-init 1.22.1 (b01adbbc3 2020-07-08)
Output of rustup show:

Default host: x86_64-pc-windows-msvc
rustup home:  C:\Users\kangj\.rustup

installed toolchains
--------------------

stable-x86_64-pc-windows-msvc
nightly-x86_64-pc-windows-msvc (default)

active toolchain
----------------

nightly-x86_64-pc-windows-msvc (default)
rustc 1.48.0-nightly (6af1bdda5 2020-09-15)
@Vbbab Vbbab added the bug label Dec 1, 2020
@kinnison
Copy link
Contributor

kinnison commented Dec 1, 2020

Thank you for your report, I shall try and reproduce this tonight. If anyone else reads this and knows what might be going on, please comment on the issue, I'm not a Windows developer by default.

@kinnison kinnison self-assigned this Dec 1, 2020
@kinnison kinnison added this to the 1.24.0 milestone Dec 1, 2020
@toothbrush7777777
Copy link

toothbrush7777777 commented Dec 7, 2020

I've run into this before. It happens because conhost doesn't reset the terminal colours after a program is terminated.
I think bash and most other terminals & shells resets the colour automatically.

@kinnison
Copy link
Contributor

kinnison commented Dec 8, 2020

What's odd is that there's no red in the rustup output at that point. So I wonder what we've done to the terminal that is causing that.

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

Perhaps the Interrupt is colored (^C)?

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

Oh and it appears that the bug only reproduces if you wait a few seconds at the first prompt. In that case the interrupt is red.

Waiting up to 4 seconds before interrupting will actually show the beginning of an error, which could be why it's red...

@kinnison
Copy link
Contributor

What error is it showing at that point? can you attach a screenshot?

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

If I interrupt it at exactly 4 seconds, this error shows up, but is blank:
image

@kinnison
Copy link
Contributor

Oh interesting, I wonder if the control+C managed to give it some input before it killed it.

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

but if I interrupt at different time intervals, things happen:

  • Immediately: Regular white ^C, user shell is normal
  • At 4 secs: error, red ^C, user shell is red
  • Between 1-4 secs, and more than 4 secs: no error, red ^C, user shell is red

@kinnison
Copy link
Contributor

That is extremely odd.

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

Could this be a bug with some sort of input-handling loop? Or maybe rustup-init is loading something within the initial 4 secs?

@kinnison
Copy link
Contributor

I guess possibly. I'm not entirely sure how the input loop behaves on Windows.

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

OK... Is there any way that I can see rustup-init's code? (Or is that closed-source?) All I see on this repo is a rustup-init.sh

@kinnison
Copy link
Contributor

the src/ tree is rustup and rustup-init - it's all open source. the code in question will be in src/cli/self_update.rs most likely

@Vbbab
Copy link
Author

Vbbab commented Dec 10, 2020

OK, thanks!

@kinnison
Copy link
Contributor

kinnison commented Jan 9, 2021

I'm not able to reproduce this, which is also odd. Are you able to investigate at all @Vbbab ?

@kinnison kinnison removed their assignment Jan 9, 2021
@Vbbab
Copy link
Author

Vbbab commented Jan 9, 2021

Hmm, I could try, but I'm not really familiar with Rust, sorry...

Perhaps someone else with Windows and experience with Rust?

@chansuke
Copy link
Contributor

I have a WIndows machine and would like to try to reproduce the error tomorrow.

@kinnison
Copy link
Contributor

If you can reliably reproduce this then we stand a chance of diagnosing it, yes please @chansuke

@chansuke
Copy link
Contributor

chansuke commented Jan 25, 2021

I tried to reproduce the error (rustup 1.23.1 (3df2264a9 2020-11-30)) but couldn't succeed...

@Vbbab
Copy link
Author

Vbbab commented Jan 25, 2021

An update: There's some error message on my end now. Very rare chance of this actually occurring.

The regular way is by hitting ctrl-c as soon as you arrive at the initial prompt, at which point a red ^C would display and leave your shell red.

Oh interesting, I wonder if the control+C managed to give it some input before it killed it.

I do believe so. Take a look:
image

@Vbbab
Copy link
Author

Vbbab commented Jan 25, 2021

Another screenshot of repro:
image

@toothbrush7777777
Copy link

I think it was easier to reproduce with certain anti-viruses. I can't remember which but I think McAfee was one.

@chansuke
Copy link
Contributor

@toothbrush7777777 Ah...thank you for the detailed information!! I will take a look again.

@guilt
Copy link

guilt commented Feb 16, 2021

I think you'd typically have to set the reset sequence (explictly print) on die.

let _ = writeln!(t);

Like, at the end of these kinda functions + general execution, good to add a
let _ = t.reset();

I mean, they could always die without reset-ting (and is a common issue with most commands in Unix as well, nothing new). They usually can handle that atexit / signal handler.

@guilt guilt linked a pull request Feb 17, 2021 that will close this issue
@Vbbab
Copy link
Author

Vbbab commented Feb 17, 2021

BTW, another way to repro:

  • Navigate to the directory of rustup-init.
  • Type rustup-init, but don't hit enter.
  • Now, follow enter key immediately with ctrl - c.
  • Repeat until the bug is produced.

@guilt
Copy link

guilt commented Feb 17, 2021

We need to check where all this can happen; What if there's a fault when printing something, or SIGKILL; If this is a signal handler, the at_exit patch I've posted can be easily re-written; Will figure out how to test it.

My question to all is that is at_exit even a standard at this point? beta/stable compilers don't expose it.

@kinnison
Copy link
Contributor

I don't think we're going to get to a stable-compatible solution to this problem in the 1.24.0 cycle so I'm going to defer this particular piece of work for 1.25.0 - if we fix it before 1.24.0 is branched then that's great, but I'm not going to hold up the release for it.

I think we're approaching an idea of the fix, but with the need for nightly right now, we're not able to use it yet.

@kinnison kinnison modified the milestones: 1.24.0, 1.25.0 Feb 20, 2021
@workingjubilee
Copy link
Member

@rustbot label: +O-windows

@rustbot rustbot added the O-windows Windows related label Apr 29, 2021
@rami3l rami3l modified the milestones: 1.25.0, 1.28.0 Jan 17, 2024
@rami3l rami3l modified the milestones: 1.28.0, 1.28.1 Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants