Skip to content

[icu] Prevent stale MSYS gpg-agent.exe process blocking command control#6407

Merged
vicroms merged 2 commits intomicrosoft:masterfrom
heydojo:patch-1
May 16, 2019
Merged

[icu] Prevent stale MSYS gpg-agent.exe process blocking command control#6407
vicroms merged 2 commits intomicrosoft:masterfrom
heydojo:patch-1

Conversation

@heydojo
Copy link
Contributor

@heydojo heydojo commented May 11, 2019

This commit fixes:
#5476

The issue is that CI environments such as Appveyor's VS2017 image will wait for all processes to complete. If a stale process resides as a result, builds will hang.
There does not appear to be any good reason for gpg-agent.exe to be running once the build of icu has completed.

Without this patch builds of icu4c using CI systems will very likely hang and not in an obvious way.

Is this the right solution to this problem? Probably not but it is one solution. And it degrades gracefully in that the build will not fail if gpg-agent.exe is not running. The gpg-agent.exe will not run again once MSYS has been configured, so to test this patch, a fresh install of vcpkg is required. Open the task manager and before the icu build completes, look for gpg-agent.exe just sitting there for no reason.
Might I suggest that the issue is fixed in vcpkg MSYS instead or as well?

Please don't request further from this commit.

This commit fixes:
#5476

The issue is that CI environments such as Appveyor's VS2017 image will wait for all processes to complete. If a stale process resides as a result, builds will hang.
There does not appear to be any good reason for gpg-agent.exe to be running once the build of icu has completed.

Without this patch builds of icu4c using CI systems will very likely hang and not in an obvious way.

Is this the _right_ solution to this problem? Probably not but it is one solution. And it degrades gracefully in that the build will not fail if gpg-agent.exe is not running. The gpg-agent.exe will not run again once MSYS has been configured, so to test this patch, a fresh install of vcpkg is required. Open the task manager and before the icu build completes, look for gpg-agent.exe just sitting there for no reason.
Might I suggest that the issue is fixed in vcpkg MSYS instead or as well?

Please don't request further from this commit.
@heydojo
Copy link
Contributor Author

heydojo commented May 11, 2019

No way I'm logging into another account just to look at an Azure pipeline log.
Especially as I am already logged into the Microsoft owned Github.

Please feel free to reject, refactor and or resubmit this commit. I'll leave the fork up for about a month.
The offer of this merge is a one time deal. I'm trying to migrate away from vcpkg if I can in the future and so, have little interest in further patches. Thanks.

@NancyLi1013
Copy link
Contributor

Hi @heydojo, thanks for your new PR. I see some regressions from the current CI system:

x64-osx master test notes
icu Pass Fail Regression
x64-linux master test notes
icu Pass Fail Regression

Please let us know if you need any help.
Also, could you please bump the version in CONTROL file?

@cenit
Copy link
Contributor

cenit commented May 13, 2019

@NancyLi1013 unfortunately @heydojo said "Please don't request further from this commit."... whoever has writing permissions to this PR should continue the work and finish it. It should be enough to wrap the added lines

vcpkg_execute_required_process(`
	COMMAND TASKKILL /F /IM gpg-agent.exe /fi "memusage gt 2"
	WORKING_DIRECTORY ${SOURCE_PATH}
)

inside a if(NOT VCPKG_CMAKE_SYSTEM_NAME OR VCPKG_CMAKE_SYSTEM_NAME STREQUAL "WindowsStore"), since osx and linux do not require it

@msftclas
Copy link

msftclas commented May 15, 2019

CLA assistant check
All CLA requirements met.

@vicroms vicroms changed the title Prevent stale MSYS gpg-agent.exe process blocking command control [icu] Prevent stale MSYS gpg-agent.exe process blocking command control May 15, 2019
@vicroms vicroms self-assigned this May 16, 2019
@vicroms
Copy link
Member

vicroms commented May 16, 2019

Thanks for your contribution @heydojo !

I'm sorry that the project didn't meet your expectations. If you're willing to provide feedback regarding your decision to move away from vcpkg we would like to hear it, send us an email to vcpkg@microsoft.com or leave a comment here.

@vicroms vicroms merged commit 58c7cfa into microsoft:master May 16, 2019
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.

5 participants