-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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 mach spawn fail interrupt #20073
Conversation
@test success(pipeline(`perl -le '$|=1; for(0..2){ print; sleep 1 }'`, | ||
prefixer("X",3) & prefixer("Y",3) & prefixer("Z",3), | ||
prefixer("A",2) & prefixer("B",2))) | ||
prefixer(prefix, sleep) = `sh -c "while read; do echo '$prefix ' \$REPLY; sleep $sleep; done"` |
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.
is REPLY going to work in all posix shells?
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.
hm, looks like specifying it explicitly might be more compatible (with dash and ash)
@@ -102,7 +97,7 @@ script: | |||
# capture the log, but only print it if `make deps` fails | |||
# try to show the end of the log first, because this log might be very long (> 4MB) | |||
# and thus be truncated by travis | |||
- moreutils/mispipe "make $BUILDOPTS NO_GIT=1 -C deps 2> deps-err.log" "$BAR" > deps.log || | |||
- moreutils/mispipe "make \$BUILDOPTS NO_GIT=1 -C deps 2> deps-err.log" "$BAR" > deps.log || |
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.
Ok I'm at least convinced that this is fine on linux - not working yet on mac though. Comment out the linux entries of the matrix while debugging that?
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.
Trying to figure out why mac wouldn't build this PR is why I started down the path of trying to see if the failure was caused by this mis-setting of BUILDOPTS.
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.
And I've been saying, the only difference this backslash makes is in the spawn bit that doesn't matter for the deps
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.
It slightly changes how the command get rewritten and parsed. But anyways, this commit didn't use to pass on it's own with the same failure.
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.
has this ever passed on mac travis? the libuv changes have always caused it to freeze as far as I can tell
4cc5904
to
2a85dfd
Compare
Would this test failure be an example of this?: https://gist.github.com/iamed2/e0a7fda816a5f8bb3d65f0124c70bb78 |
yes. |
this was a bug in our libuv fork triggering a bug in the mach kernel code must be careful to never use sigprocmask, setjmp, longjmp, or similar such thread-unsafe functions to avoid this documented unspecified behavior
2a85dfd
to
0f4c22b
Compare
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.
umpteenth time's the charm. lgtm unless the currently running win64 appveyor job shows anything surprising
yeah, it's no fun running into mach bugs trying to avoid other ones :/ |
travis seemed to be having issues with the cache in the previous PR (#19590), so attempting a new one.
edit by tkelman:
should close #17626sadly that was over-optimistic