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

CMD.exe's Terminate Batch Script Prompt is Swallowed When Using a Wait Bar While Running a bat Script #811

Closed
rmartin16 opened this issue Aug 5, 2022 · 7 comments
Labels
bug A crash or error in behavior.

Comments

@rmartin16
Copy link
Member

rmartin16 commented Aug 5, 2022

This originated from and is made significantly worse by:

However, all Python versions are affected by the aspect described here.

Describe the bug
CMD.exe emits a Terminate batch job (Y/N)? prompt to the user when a CTRL+C is sent during execution of a batch script. AFAICT, this is to allow the execution of the rest of batch script while canceling its current action (perhaps similar to bash's default behavior of continuing on error without set -e). This behavior will be ubiquitous regardless of the shell the user started briefcase in; all shells in Windows (e.g. powershell, terminal, msys2) will all defer batch script interpretation to cmd.

Many of the Android tools are invoked from bat scripts provided from Google. For instance, sending a CTRL+C while building an Android APK results in:

  • cmd emitting its terminate prompt
  • the prompt is swallowed by Rich's Wait Bar
  • the user can't tell anything really happened since the Wait Bar is still pulsing
  • the whole process remains stagnate until the user sends another CTRL+C
  • (On Python 3.7/3.8, this also leads to Python hanging entirely, see # 809)

To Reproduce
Steps to reproduce the behavior:

  1. Run briefcase build android gradle
  2. Once the Building... Wait Bar starts, send CTRL+C
  3. Witness issue

Expected behavior
CTRL+C should allow the user to exit or provide clear instructions (such as the terminate batch prompt) on how to exit.

Environment:

  • Operating System: Windows 10
  • Python version: 3.7, 3.8, 3.9, 3.10
  • Software versions:
@rmartin16 rmartin16 added the bug A crash or error in behavior. label Aug 5, 2022
@rmartin16 rmartin16 changed the title CMD.exe's Terminate Batch Script Prompt is Swallowed When Using a Wait Bar While Running a bat Script CMD.EXE's Terminate Batch Script Prompt is Swallowed When Using a Wait Bar While Running a bat Script Aug 5, 2022
@rmartin16 rmartin16 changed the title CMD.EXE's Terminate Batch Script Prompt is Swallowed When Using a Wait Bar While Running a bat Script CMD.exe's Terminate Batch Script Prompt is Swallowed When Using a Wait Bar While Running a bat Script Aug 5, 2022
@rmartin16
Copy link
Member Author

Summarizing previous discussion in #809:

@rmartin16
As for the Terminate batch job (Y/N)? prompt, this discussion seems like a decent summary of the situation. Given this user experience of CTRL+C is pretty bad, we should probably avoid wrapping bat files in Wait Bars

@freakboy3742
my general philosophy is that when a platform makes something difficult to do, you should stop fighting the platform. That would seem to suggest disabling wait bars on Windows for those tools that are batch files would be worthwhile. Would it be plausible to stop/suspend the waitbar when invoking subprocess.* if the first arg ends in .bat?

@rmartin16

        if str(args[0]).endswith(".bat") and self.command.input._active_wait_bar:
            self.command.input._active_wait_bar.stop()
            self.command.input.is_output_controlled = False

Naively putting that at the top of Subprocess.run() and running briefcase build android does allow for a much more graceful handling of cmd's terminate batch prompt.....so, I think it may be plausible.

That said, we were depending on the Wait Bars as a proxy mechanism to get spawned process output in to the log.....so, a lot in the air I guess.....this is a card close to the bottom of the house in a way...

@mhsmith
If getting rid of the "Terminate batch job?" message means we lose the process output from the log file, then I think we'll just have to live with the message. As long as the user can enter Y and it actually terminates, it's not a major problem. Or the discussion you linked says you can just press Ctrl-C a second time, which is even easier.

@rmartin16
Copy link
Member Author

rmartin16 commented Aug 5, 2022

To make sure it is clear, there is apparently no known msft-endorsed method to default a choice for the Terminate batch job (Y/N)? prompt. So, it will likely be necessary to be accommodate this prompt on Windows for the foreseeable future (even after cmd is long in the rearview mirror of msft since they seemingly have a complex with backwards compat but I digress...)

The most straightforward solution right now is not to wrap bat scripts with the Wait Bar. However, this has two consequences:

  1. The user experience becomes visually quite different for these scripts and the user will have no indication why
  2. The output of these bat files (and its spawned processes) will not be included in the briefcase log file

No 2 starts to get more at the root of the problem since it implies a lot of briefcase's logging infrastructure needs to be disabled for bat scripts; while the Wait Bar manifests the problem, the root of the problem is briefcase depends on piping spawned process output back up through subprocess to write that output both to console and the log file. When cmd (rudely) interjects its own prompt superseding everything, this undermines the starting assumptions that 1) briefcase is in control of the console and 2) the user will not be prompted at that time.

To demonstrate the issue without the Wait Bar: remove the Wait Bar and add stream_output=True to the subprocess.run() call to start the APK build.....the same thing happens.

It may be possible to still circumvent all this on Windows and still get logging but more experimentation is necessary.

@freakboy3742
Copy link
Member

Thanks for that summary, and for splitting this issue out from #809.

Clarifying some of my positions from past discussions:

  • I'm not overly concerned about the existence of the Terminate batch job prompt; that prompt should be reasonably familiar to Windows users, so won't be overly surprising if it occurs. It's only a problem if that prompt causes a lockup.
  • Having a different user experience isn't overly concerning to me either; most users will only ever experience Briefcase on a single platform - it's only people who are developing briefcase that will really notice the platform discrepancies.
  • Losing the log output would be a big loss - especially on Windows, because the tools provided by Windows for capturing console output are so inadequate. If there's any way to avoid losing the log output - especially on a build/compile step - that would be preferable.

@rmartin16
Copy link
Member Author

@mhsmith, we fixed the deadlock, but are you still seeing the Terminate batch job (Y/N)? message hold Briefcase up from exiting?

I was trying to replicate this again but Briefcase is able to wrap up and exit despite cmd's Terminate message. In #809, we both reported Briefcase just appeared to hang until another CTRL+C was sent...

@mhsmith
Copy link
Member

mhsmith commented Aug 22, 2022

I think there may still be some issues here, but I won't have time to test it today. Let me get back to you tomorrow.

@mhsmith
Copy link
Member

mhsmith commented Aug 23, 2022

The Terminate batch job message still sometimes appears, but it normally doesn't stop Briefcase from exiting after the first Ctrl-C. With the current HEAD (d013638) Python 3.8, and the cmd shell, I can interrupt all of the following subprocesses reliably:

  • Gradle
  • adb install
  • Logcat

The exception is sdkmanager --list_installed, which since #832 is now only run when creating a log file. If I press Ctrl-C while that's running, Briefcase hangs. However, I can escape by pressing Ctrl-C an additional 1 or 2 times, so this isn't as bad as #809.

@rmartin16
Copy link
Member Author

All right, thank you for going through all those, @mhsmith.

I'll put together a PR to stop the Wait Bar if it's running when subprocess.run is invoked for .bat file.

rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
rmartin16 added a commit to rmartin16/briefcase that referenced this issue Sep 27, 2022
freakboy3742 added a commit that referenced this issue Sep 28, 2022
Disable Wait Bar for Windows batch scripts (Fixes #811)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior.
Projects
None yet
Development

No branches or pull requests

3 participants