mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Clemmer <clemmer.alexan...@gmail.com>
Subject Re: Review Request 55037: Added default PATH value in `launch.cpp` for Windows case.
Date Sun, 15 Jan 2017 08:57:34 GMT


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


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