----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55037/#review162525 -----------------------------------------------------------
Ship it! The review description here is now out-of-date. Something like this would be more appropriate: ``` Used `os::host_default_path()` in several locations. Several of the codepaths that eventually launch a subprocess will check for a `PATH` environment variable. If this value is not set, it uses a default value. This commit replaces these hard-coded default values with the newly added stout function `host_default_path`. The return value of this function differs depending on the host (and the OS), meaning that the return value of `host_default_path` may or may not be portable. ``` I can remove the below before committing. src/slave/containerizer/mesos/launch.cpp (line 53) <https://reviews.apache.org/r/55037/#comment233833> This looks like an erroneous addition. - Joseph Wu On Jan. 18, 2017, 6:13 p.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55037/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2017, 6:13 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, > and Joseph Wu. > > > Bugs: MESOS-6839 > https://issues.apache.org/jira/browse/MESOS-6839 > > > Repository: mesos > > > Description > ------- > > Currently, when the containerizer launches a process with the POSIX > launcher, it will check if the `$PATH` environment variable (or `%PATH%` > in the case of Windows) is set in the `LaunchInfo`. If it is not, we > supply a default path. Unfortunately, this default path is specific to > POSIX. In many of our tests, this causes many of our tests to be unable > to find Windows-standard executables like `ping`, and subsequently fail. > > This commit will introduce a function, `defaultPath` that returns a > sensible default path for both POSIX and Windows. Since the Windows > implementation of this depends on the configuration of the host running > the containerizer (rather than, say, the one creating the `TaskInfo`), > we choose to implement this in `launch.cpp` instead of Stout, where a > user could mistakenly call it and expect the same output on all hosts. > > > Diffs > ----- > > src/slave/containerizer/docker.cpp 7a8a7271b54af0b4dcdae7a2aa8a90a8a7d05fd6 > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > ea91c71fdfac48a2fc1d31a0ee088a73244be367 > > src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp > ab4b88acddc7503e16e3730320df39a2f104539a > src/slave/containerizer/mesos/launch.cpp > e482ab8bdfc358f695b87cda72ca59fb64cd8c4d > > Diff: https://reviews.apache.org/r/55037/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
