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(shim): allow GUI applications to attach to the shell's console when launched using the GUI shim #5721

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

spider2048
Copy link
Contributor

@spider2048 spider2048 commented Nov 9, 2023

Description

Allows patched GUI shims to access the shell's console window.

Motivation and Context

Fixes issues described in #5559 (for the scoopcs shim type)

How Has This Been Tested?

The new shim code only tries to AttachConsole to the parent process, so that it gets the console handles (when it doesn't find a console window). Now, the target process which the shim starts can also request for the same console handles and it can write to the shell's terminal.

Validation steps:
Let's use alacritty.
We know that, alacritty.exe is a GUI application, but we have:

$ &(scoop which alacritty) -V
alacritty 0.12.3 (5efb069)

If we don't use this PR, we would not have this output, because the target process (alacritty) fails to get handles to the shell's console.

$ alacritty -h

But, if we use this PR:

$ scoop config shim scoopcs
$ scoop update -f alacritty
$ alacritty -V
alacritty 0.12.3 (5efb069)

This PR would restore the behaviour of GUI+CLI applications and avoids the need to implement new flags or adjust the manifests.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@spider2048 spider2048 marked this pull request as ready for review November 9, 2023 04:32
@spider2048 spider2048 changed the title fix(shim): allow scoopcs GUI apps to attach to the shell's console fix(shim): allow GUI applications to attach to the shell's console when launched using the GUI shim Nov 9, 2023
@niheaven niheaven merged commit 90766f9 into ScoopInstaller:develop Mar 20, 2024
2 checks passed
@goyalyashpal
Copy link

goyalyashpal commented Mar 24, 2024

is it released? if yes, then to which version or with what indication?

i am asking as i want to confirm if the non-confirmant thing i am seeing applies or not.

@spider2048
Copy link
Contributor Author

@goyalyashpal It is merged in to develop, will be merged to master in 0.4.0. You can track the progress of the release in #5424. And what do you mean by a non-confirmant thing?

@goyalyashpal
Copy link

what do you mean by a non-confirmant thing?

glad u asked 😄

quote

Validation steps: Let's use alacritty. We know that, alacritty.exe is a GUI application, but we have:

$ &(scoop which alacritty) -V
alacritty 0.12.3 (5efb069)

If we don't use this PR, we would not have this output, because the target process (alacritty) fails to get handles to the shell's console.

$ alacritty -h

@ spider2048 at #5721 (comment)

i guess i am seeing the behaviour which is shown as output of this pr already in v0.3 :

$ which alacritty
/d/UserFiles/scoop/shims/alacritty

$ alacritty -V
alacritty 0.13.1 (fe2a3c5)

$ $(scoop which alacritty) -V
alacritty 0.13.1 (fe2a3c5)

$ alacritty -h
A fast, cross-platform, OpenGL terminal emulator
...
$ scoop -V
Current Scoop version:
v0.3.1 - Released at 2022-11-15

@spider2048
Copy link
Contributor Author

spider2048 commented Mar 24, 2024

@goyalyashpal seems like you are on the master branch. If you don't have this problem, you would have the shim blocking your CLI.

@goyalyashpal
Copy link

goyalyashpal commented Mar 24, 2024

seems like you are on the master branch

how can i confirm that?

you would have the shim blocking your CLI.

but it does with some other GUI applications. i always used to think that this blocking behaviour is application dependent.

for examples (these all are scoop installed pplications:)

  • run alacritty -> launches alacritty GUI, blocks CLI
  • wezterm -> launches wezterm GUI, blocks
  • wezterm -V -> performs within CLI
  • notepad3 <file> -> launches GUI, blocks

non-scoop application:

  • codium <file> -> launches GUI, still non-blocking

the notepad3, and wezterm behaved same (blocking in GUI) when it was non-scoop application too.
and i'd bet that vscodium would stay the same too (non-blocking GUI) despite getting installed with scoop.

$ which notepad3 alacritty wezterm codium
/d/UserFiles/scoop/shims/notepad3
/d/UserFiles/scoop/shims/alacritty
/d/UserFiles/scoop/shims/wezterm
/c/Program Files/VSCodium/bin/codium

@spider2048
Copy link
Contributor Author

spider2048 commented Mar 24, 2024

@goyalyashpal

how can i confirm that?

By using scoop config

% scoop config
last_update           : 25-03-2024 01:31:18 AM
aria2-warning-enabled : False
shim                  : scoopcs
scoop_repo            : https://github.com/ScoopInstaller/Scoop
scoop_branch          : develop

You can switch to the develop branch by:

$ scoop config scoop_branch develop
$ scoop update

but it does with some other GUI applications. i always used to think that this blocking behaviour is application dependent.

It depends on the shim as we use the shim to launch applications. Now, the default shim in the master branch is a CLI shim, which would always have a terminal window. The nature of the application is characterized by the Subsystem flag in the PE File Format.

for examples (these all are scoop installed pplications:) ...

Let's look at your examples:

  1. alacritty is a GUI binary - it should not be supposed to have a CLI window
  2. wezterm is a CLI binary, you are supposed to use wezterm-gui if it should not block
  3. wezterm -V should perform within CLI as it is a CLI binary
  4. notepad3 is a GUI binary, and should not be supposed to block

To fix apps which should not be blocking, my PRs would correct the Subsystem field in the shim binary. So, ideally, if you are using the develop branch, you should be having only the CLI binaries to block and GUI binaries to detach. #5559

Just in case if a GUI app wants to write to a console using

AttachConsole(-1);

It won't be able to obtain handles to our shell which we used to launch. So, this PR would solve this issue in the scoopcs shim type, as we don't have direct access to the kiennq shim type.

I would advise you to avoid the kiennq shim, as it just had memory issues while parsing a 100+ character long string in the arguments. (comments in #1606)

Coming to codium or code, they are shell scripts:

$ which code
D:\Tools\Sceoop\apps\vscode\current\bin\code.cmd

So they would always have a console window until they start the actual application.

Finally, move to the develop branch. The version which you are using seems to be too old and as scoop has not yet issued a release.

@goyalyashpal
Copy link

goyalyashpal commented Mar 24, 2024

seems like you are on the master branch
By using scoop config

on point 👍 😃

$ scoop config | yq .scoop_branch,.scoop_repo
master
https://github.com/ScoopInstaller/Scoop

4. notepad3 is a GUI binary, and should not be supposed to block
To fix apps which should not be blocking, my PRs would correct the Subsystem field in the shim binary. So, ideally, if you are using the develop branch, you should be having only the CLI binaries to block and GUI binaries to detach. #5559

ohkayh. awesome.

I would advise you to avoid the kiennq shim

i am very novice to scoop and shims. so, i don't even know what these types are or how to configure anything about it. but i'll keep in mind when i gain more xp with scoop.

Finally, move to the develop branch. The version which you are using seems to be too old and as scoop has not yet issued a release.

thanks for rec. i will in some days 😄


ps:

we both should be sleeping deep at this odd hour of night lol. hopefully big fest today day 😆💦🔫🌊🎈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants