> On Dec. 27, 2016, 2:12 p.m., Till Toenshoff wrote: > > src/slave/containerizer/mesos/launch.cpp, line 148 > > <https://reviews.apache.org/r/55037/diff/1/?file=1592009#file1592009line148> > > > > By not adding `syswow64` we are excluding 32bit runnables, is this > > intentional and documented? Is this still a thing on windows? > > Joseph Wu wrote: > Actually, in the current implementation of subprocess on Windows, any > user-specified environment variables that conflict with system environment > variables will be overridden at subprocess launch. This means you can > specify a `PATH=nowhere` and we'll just ignore it. > > At some point, we may loosen this behavior, as we only did this because > the test coverage of the Windows code at the time was nil, and a bunch of > other changes kept breaking the environment variables on Windows :)
Actually, the issue is that I'm not sure how to get that folder name without depending on the Shell API. (Not a Windows expert, let me know if you have a good way to do this.) We are very careful not to depend on the Shell because it's not available on Server Nano. I'll drop it for now, but I'd be quite happy to add this if there was a suggestion for doing it that worked around this constraint. - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55037/#review160161 ----------------------------------------------------------- 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 > >