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

Issue 4172: handle processes names ending with .exe #453

Conversation

jeanmichel-nwsb
Copy link
Contributor

Signed-off-by: Jean-Michel Mercier [jeanmichel.mercier@newsbridge.io](mailto:jeanmichel.mercier@newsbridge.io

This PR fix the detection of running executable as on current version of Windows, the Process.Name may ends with .exe which make the comprison fails, and the processes are not marked to be terminated.

This fixes git-for-windows/git#4172

Signed-off-by: Jean-Michel Mercier <jeanmichel.mercier@newsbridge.io>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the Issue-4172-handle-process-name-terminated-with-.exe branch from e54bf0c to c6b8a55 Compare December 18, 2022 20:18
@dscho
Copy link
Member

dscho commented Dec 18, 2022

@jeanmichel-nwsb I edited the commit message and the patch a bit, could you please test to confirm that it still fixes the reported problem?

@jeanmichel-nwsb
Copy link
Contributor Author

Hi @dscho
I've seen your change
Will test it later today
Thanks, much compact 👍

@jeanmichel-nwsb
Copy link
Contributor Author

Hi @dscho
So I have tested and it works ...
But just wanted to be sure with you if it is the inteded behavior:

  • it first display the ProcessesPage,
  • and then if I confirm "Install", it offer to stop the process

From the code I was under the impression it would try to terminate the processes before displaying this page but I'm probably wrong with understanding the logic of InnoSetup pages

So for me all good 👍
Thanks

image
image

@dscho
Copy link
Member

dscho commented Dec 19, 2022

But just wanted to be sure with you if it is the inteded behavior:

* it first display the ProcessesPage,

* and then if I confirm "Install", it offer to stop the process

Yes, it is intended. It says "will be terminated" to notify the user that this is going to happen instead of silently stopping the processes.

@dscho dscho merged commit 7d9f3ad into git-for-windows:main Dec 19, 2022
github-actions bot pushed a commit that referenced this pull request Dec 19, 2022
The installer is expected to stop GPG agents automatically, but there

prevented that from working, which [has been
fixed](#453).

Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
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.

2 participants