> On Jan. 10, 2018, 4:09 a.m., Alexander Rukletsov wrote: > > 3rdparty/stout/include/stout/windows.hpp > > Lines 343-348 (original), 343-348 (patched) > > <https://reviews.apache.org/r/63859/diff/5/?file=1931096#file1931096line343> > > > > Could you please clarify, what the fix is here? At least in the patch > > description, but feel free to update the comment above.
The clarification is in the previous comments on the review: these were literally incorrect values. They're now correct. I don't know how to link you to the comment, but I'll reproduce it here: Me: > :/ I'd really like to fix whatever code is using these signals for logic. I > feel like the defining the signals for Windows was originally a band-aid, and > understand this patch didn't add them. > The funny thing is that, since these values aren't defined on Windows, they > could be any number so long as only the symbol is used in the rest of the > code base. I think this is why this worked anyway. > Akash, what bug did you run into that required correcting these? > (And indeed, they now appear correct. SIGKILL is 9, SIGCONT is 18 in decimal, > and SIGSTOP is 19 in decimal). Akash: > I think they worked before because they were used only within stout. One of > the docker executor method signature is docker->kill(ID, SIGNAL), which > eventually calls docker kill -s SIGNAL ID. Docker (and go standard library) > defines the Linux signal values on Windows, so it's expecting docker kill -s > 9. If you want, I can fix the docker executor to ignore the signal field on > Windows and just send docker kill without the -s. Me: > Gotcha! That makes sense. I could go either way on your proposition. It > obviously works and is supported by Docker on Windows, so it makes sense > (now) to leave it. OTOH if removing it would also let us remove these > definitions, I like that too. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63859/#review195129 ----------------------------------------------------------- On Jan. 4, 2018, 4:28 p.m., Akash Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63859/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2018, 4:28 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer and John Kordich. > > > Bugs: MESOS-7342 > https://issues.apache.org/jira/browse/MESOS-7342 > > > Repository: mesos > > > Description > ------- > > Also fixed the WEXITSTATUS macro to cast the exit code instead of > bit-masking it, since Windows exit codes are 32 bit unsigned ints. > > > Diffs > ----- > > 3rdparty/stout/include/stout/windows.hpp > 7aa0ba72c4cc3b688ce6374b3308945ea8cb7572 > > > Diff: https://reviews.apache.org/r/63859/diff/5/ > > > Testing > ------- > > See https://reviews.apache.org/r/63862/ for test results. > > > Thanks, > > Akash Gupta > >
