-
-
Notifications
You must be signed in to change notification settings - Fork 742
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
ICU-21172 fix install race #2966
Conversation
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
Regarding the CLA, Arm has a corporate CLA signed. |
Now that you have a corporate CLA agreement in place, you now need to personally click the "CLA not signed yet" button to authorize yourself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks.
tested locally LGTM,. why is CI stuck? |
I was thinking maybe there is a problem between an old commit and newer changes to the CI system, and tried to "squash" this commit via the UI, but then I got a permission-denied error message. @rossburton would you mind rebasing your commit to trigger a fresh CI round? |
The generic recursive target calls target-local so also adding it to the dependency list results in races due to install-local being executed twice in parallel. For example, install-manx can fail if the two install processes race and one process tries to chown a file that the other process has just deleted. Also install-manx should be a phony target, and for clarity use $^ instead of $? in the install command. Signed-off-by: Ross Burton <[email protected]>
Notice: the branch changed across the force-push!
~ Your Friendly Jira-GitHub PR Checker Bot |
The generic recursive target calls target-local so also adding it to the dependency list results in races due to install-local being executed twice in parallel. For example, install-manx can fail if the two install processes race and one process tries to chown a file that the other process has just deleted.
Also install-manx should be a phony target, and for clarity use
$^
instead of$?
in the install command.Checklist