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

Installer freezes when testing custom Atom editor command while Atom is already open #3155

Closed
1 task done
schtandard opened this issue Mar 30, 2021 · 4 comments
Closed
1 task done
Milestone

Comments

@schtandard
Copy link

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

    $ git --version --build-options
    git version 2.31.1.windows.1
    cpu: x86_64
    built from commit: c5f0be26a7e3846e3b6268d1c6c4800d838c6bbb
    sizeof-long: 4
    sizeof-size_t: 8
    shell-path: /bin/sh
    feature: fsmonitor--daemon
    
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

    $ cmd.exe /c ver
    
    Microsoft Windows [Version 10.0.19042.868]
    
  • What options did you set as part of the installation? Or did you choose the
    defaults?

    $ cat /etc/install-options.txt
    
    Editor Option: Atom
    Custom Editor Path:
    Default Branch Option:
    Path Option: Cmd
    SSH Option: OpenSSH
    Tortoise Option: false
    CURL Option: OpenSSL
    CRLF Option: CRLFAlways
    Bash Terminal Option: MinTTY
    Git Pull Behavior Option: Merge
    Use Credential Manager: Core
    Performance Tweaks FSCache: Enabled
    Enable Symlinks: Disabled
    Enable Pseudo Console Support: Disabled
    
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

    Nope.

Details

  • How did I produce the error?

    • Open Atom.
    • Run the Git for Windows installer
    • In the step Choosing the default editor used by Git, select Select other editor as Git's default editor.
    • In the text field below, enter atom --wait --new-window.
    • Click Test Custom Editor.
  • What happened?

    • A new atom window opened, as expected.
    • After closing it, the installer is frozen.
    • After closing the originally opened Atom window, the installer is still frozen.
    • The only way I could find to get rid of the installer window was to terminate its process by force.
@dscho
Copy link
Member

dscho commented Mar 30, 2021

Oh... would you have a chance to test out something? I have the hunch that calling the ShellExec() function is at fault here (because Atom is installed user-wide, we should execute it as the original user, not as administrator, i.e. use ShellExecAsOriginalUser().

This is how you can help by testing it:

  1. install Git for Windows' SDK,
  2. sdk cd installer,
  3. edit install.iss to replace ShellExec() as indicated above
  4. build a new installer via sdk build installer
  5. verify that that installer works as expected?
  6. open a PR?

@schtandard
Copy link
Author

That seems to do the trick and I created a pull request.

I assume this won't lead to problems with editors that are installed system-wide? I have no experience here.

dscho added a commit to git-for-windows/build-extra that referenced this issue Mar 31, 2021
installer: use ShellExecAsOriginalUser() instead of ShellExec() for custom editor tests

This is the right thing to do: if the editor in question wants to
communicate via IPC to a user-specific instance, it will _have_ to be
run as original user. Atom would be such an editor, and it was reported
in git-for-windows/git#3155 that this is
indeed required to Make Things Work.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Mar 31, 2021

Fixed via git-for-windows/build-extra#334

@dscho dscho closed this as completed Mar 31, 2021
@dscho dscho added this to the Next release milestone Mar 31, 2021
dscho added a commit to git-for-windows/build-extra that referenced this issue Mar 31, 2021
When testing a custom editor in the installer, [we now spawn it in
non-elevated mode](git-for-windows/git#3155),
fixing e.g. Atom when an instance is already running.

Signed-off-by: Johannes Schindelin <[email protected]>
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

No branches or pull requests

2 participants