----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55037/#review160161 -----------------------------------------------------------
Fix it, then Ship it! While the below might never be implemented for Windows I think we should still unify them and switch all PATH defaults to the new function. https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L1408 https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L1118 https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L1502 src/slave/containerizer/mesos/launch.cpp (line 141) <https://reviews.apache.org/r/55037/#comment231209> s/off/of/ ? src/slave/containerizer/mesos/launch.cpp (line 148) <https://reviews.apache.org/r/55037/#comment231208> By not adding `syswow64` we are excluding 32bit runnables, is this intentional and documented? Is this still a thing on windows? - Till Toenshoff On Dec. 26, 2016, 9:53 a.m., Alex Clemmer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55037/ > ----------------------------------------------------------- > > (Updated Dec. 26, 2016, 9:53 a.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, `default_path` 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/mesos/launch.cpp > e482ab8bdfc358f695b87cda72ca59fb64cd8c4d > > Diff: https://reviews.apache.org/r/55037/diff/ > > > Testing > ------- > > > Thanks, > > Alex Clemmer > >
