----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60280/#review179745 -----------------------------------------------------------
This will break the windows build. os::which is only in posix header. I would ask in the #windows channel see if we can port os::which to be on both windows and posix (sounds very doable). src/slave/containerizer/mesos/launch.cpp Lines 799-814 (patched) <https://reviews.apache.org/r/60280/#comment254583> I will do the following: ``` // Search executable in the current working directory as well. // `execvpe` and `execvp` will only search executable from the // current working directory if environment variable `PATH` is // not set. if (!path::absolute(executable) && launchInfo.has_working_directory()) { Option<string> which = os::which( executable, launchInfo.working_directory()); if (which.isSome()) { executable = which.get(); } } ``` src/slave/containerizer/mesos/launch.cpp Lines 808 (patched) <https://reviews.apache.org/r/60280/#comment254582> This line is longger than 80 chars: ``` Jies-MacBook-Pro:mesos jie$ support/apply-review.sh -r 60280 2017-07-05 19:39:04 URL:https://reviews.apache.org/r/60280/diff/raw/ [1397/1397] -> "60280.patch" [1] Checking 1 C++ file src/slave/containerizer/mesos/launch.cpp:803: Lines should be <= 80 characters long [whitespace/line_length] [2] src/slave/containerizer/mesos/launch.cpp:808: Lines should be <= 80 characters long [whitespace/line_length] [2] Total errors found: 1 No Python files to lint ``` ALso, we need to test if working_directory is set or not. src/slave/containerizer/mesos/launch.cpp Lines 810 (patched) <https://reviews.apache.org/r/60280/#comment254584> indentation here should be 2 - Jie Yu On July 5, 2017, 9:03 p.m., Aaron Wood wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60280/ > ----------------------------------------------------------- > > (Updated July 5, 2017, 9:03 p.m.) > > > Review request for mesos, Jie Yu, James Peach, and Zhitao Li. > > > Bugs: MESOS-7703 > https://issues.apache.org/jira/browse/MESOS-7703 > > > Repository: mesos > > > Description > ------- > > If a framework specifies use of its own executor and sets shell to false the > executor is never found. This provides the full path to the executor so that > the `execvp` or `execvpe` is successful. > > > Diffs > ----- > > src/slave/containerizer/mesos/launch.cpp 162ca1c2e > > > Diff: https://reviews.apache.org/r/60280/diff/7/ > > > Testing > ------- > > `cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make -j4` > Also spun up a master and agent, connected and sent a task using the UCR > (both with and without the use of an OCI image) via our own framework, and > checked the sandbox to verify that things went accordingly. > > > Thanks, > > Aaron Wood > >
