mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 55037: Added default PATH value in `launch.cpp` for Windows case.
Date Thu, 12 Jan 2017 01:23:07 GMT


> On Dec. 27, 2016, 6:12 a.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?

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 :)


- Joseph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55037/#review160161
-----------------------------------------------------------


On Dec. 26, 2016, 1: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, 1: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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message