> On Oct. 24, 2014, 4:40 p.m., Timothy Chen wrote: > > src/tests/slave_tests.cpp, line 448 > > <https://reviews.apache.org/r/26622/diff/4/?file=731893#file731893line448> > > > > Just tried bin/ls on OSX and realize Mac's ls has no --author flag so > > this is going to fail on OSX. > > > > It's ok if we don't support it but then we need to wrap the test in > > #ifdef __linux__ > > R.B. Boyer wrote: > The part that actually breaks is any double-dash option arguments. If > the OSX version of ls shares any other double-dash arg options with linux > then just replace --author with that. > > Timothy Chen wrote: > unfortunately ls doesn't have any double dash options, and I just looked > at /usr/bin and none of the non-intrusive binaries has any double dash > options too. > I can however do a echo --author, that would have broke earlier right? > > R.B. Boyer wrote: > I don't have access to an OSX box, nor will I have time until mid next > week to revisit this ticket. > > If you can, please try swapping out "[echo, --author]" for my ls example, > reverting slave.cpp, and then: > > GTEST_FILTER=SlaveTest.MesosExecutorCommandTaskWithArgsList make check > > To just double check that "echo --author" killed mesos-executor on OSX > with the unpatched protobuf stuff. > > Timothy Chen wrote: > Ok I'll do that. > > R.B. Boyer wrote: > Is there anything else for me to do on this patch? > > Jie Yu wrote: > Tim, are you working on the patch? > > Timothy Chen wrote: > Ya sorry was busy with docker patches, will get to this when they're > pushed. > > Timothy Chen wrote: > Hi R.B., I can't apply your patch cleanly for some reason it can't find > your name/email. > Can you try to reupload it again? If I don't apply your patch I can't > commit it with your name/email. > > Jie Yu wrote: > There is no ship-it yet. Please post te latest patch please. > > Timothy Chen wrote: > I think it's because you don't have any email setup under reviewboard too. > > Timothy Chen wrote: > Ok I just merged this with manually putting your name and email in. > Please update reviewboard with your email next time. > > R.B. Boyer wrote: > Huh. I swear I've configured my profile in reviewboard. Does the profile > have to be public (mine is marked private right now)?
Why has this patch been committed? It does not look like the '/bin/ls -l --author' issue has been resolved. :-( - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26622/#review58287 ----------------------------------------------------------- On Oct. 24, 2014, 2:04 a.m., R.B. Boyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26622/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2014, 2:04 a.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-1873 > https://issues.apache.org/jira/browse/MESOS-1873 > > > Repository: mesos-git > > > Description > ------- > > Basically if you use "shell=false" with a non-empty argument list and the > Command Executor it is completely broken. > > When we clone the env vars to fork "mesos-executor" all of the original cmd > args are cloned as well (unintentionally) due to some protocol-buffer merge > shenanigans. > > Don't pass task-related arguments to mesos-executor. > > The description on the linked jira ticket goes into more detail. > > > Diffs > ----- > > src/slave/slave.cpp 55e5264 > src/tests/slave_tests.cpp a1bd00c > > Diff: https://reviews.apache.org/r/26622/diff/ > > > Testing > ------- > > make check > > added new test "SlaveTest.GetExecutorInfo" verifying the explicit desired > behavior. > > > Thanks, > > R.B. Boyer > >