Re: Review Request 68532: Windows: Ported `OsTest.Children, KillTree, ProcessExists`.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On Aug. 27, 2018, 9:29 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68532/
> ---
> 
> (Updated Aug. 27, 2018, 9:29 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were (partially) ported by implementing logic similar to
> the what's used on POSIX with `os::Fork` and `os::Exec` (which will
> not be implemented on Windows).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os_tests.cpp b80c34e8e84cab68de7c843c7eafefbd84c3328c 
> 
> 
> Diff: https://reviews.apache.org/r/68532/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68529: Windows: Added notes for permanently disabled tests.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On Aug. 27, 2018, 9:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68529/
> ---
> 
> (Updated Aug. 27, 2018, 9:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added notes for permanently disabled tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 09d0a40a74a293dcf6eecde2a443281e4b8d9fe8 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 3ca4099300c6288eb453ef04eb97d299adc5880d 
>   3rdparty/stout/tests/os/strerror_tests.cpp 
> 6a69d52a5b9ae93f3393ab17a3282ecb30ad90b0 
>   3rdparty/stout/tests/os_tests.cpp b80c34e8e84cab68de7c843c7eafefbd84c3328c 
> 
> 
> Diff: https://reviews.apache.org/r/68529/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68532: Windows: Ported `OsTest.Children, KillTree, ProcessExists`.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On Aug. 27, 2018, 9:15 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68532/
> ---
> 
> (Updated Aug. 27, 2018, 9:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were (partially) ported by implementing logic similar to
> the what's used on POSIX with `os::Fork` and `os::Exec` (which will
> not be implemented on Windows).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os_tests.cpp b80c34e8e84cab68de7c843c7eafefbd84c3328c 
> 
> 
> Diff: https://reviews.apache.org/r/68532/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68531: Windows: Ported `OsTest.Environment`.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On Aug. 27, 2018, 9:15 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68531/
> ---
> 
> (Updated Aug. 27, 2018, 9:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This required replacing `os::raw::environment()` inline as it does not
> exist on Windows. Furthermore, it was also discovered that the
> documentation is incorrect, and that sometimes environment variables
> start with an equal sign. The fix is to simply look for the first
> equal sign excluding the first character, otherwise we end up with
> empty keys.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/environment.hpp 
> 40eee59ac0b73ebe71b120e540ba7eace672ff4c 
>   3rdparty/stout/tests/os_tests.cpp b80c34e8e84cab68de7c843c7eafefbd84c3328c 
> 
> 
> Diff: https://reviews.apache.org/r/68531/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 68530: Windows: Enabled `RemoveWithContinueOnError` test.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On Aug. 27, 2018, 9:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68530/
> ---
> 
> (Updated Aug. 27, 2018, 9:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test "just worked" after previous fixes.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 3ca4099300c6288eb453ef04eb97d299adc5880d 
> 
> 
> Diff: https://reviews.apache.org/r/68530/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67977: Fixed `test-linkee` logic in `ProcessRemoteLinkTest::SetUp()`.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On July 19, 2018, 8:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67977/
> ---
> 
> (Updated July 19, 2018, 8:44 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> Liangyu Zhao, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5941 and MESOS-9097
> https://issues.apache.org/jira/browse/MESOS-5941
> https://issues.apache.org/jira/browse/MESOS-9097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Several problems existed on Windows here:
> 
> * The `BUILD_DIR` path has forward slashes (probably fine, but wrong).
> * The executable must be named correctly, `.exe` and all.
> * We should assert that `test-linkee` exists.
> * The shell should not be used, otherwise `'(62)'` will resolve to an
>   unknown UPID inside `test-linkee` because of the single quotes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 8baf60d8bbb675e26fea5e76c825ef73fedbc629 
> 
> 
> Diff: https://reviews.apache.org/r/67977/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67951: Added optional `path_separator` parameter to `Path` constructor.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On July 17, 2018, 10:27 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67951/
> ---
> 
> (Updated July 17, 2018, 10:27 p.m.)
> 
> 
> Review request for mesos, Eric Mumau, John Kordich, Joseph Wu, Liangyu Zhao, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-5904 and MESOS-8564
> https://issues.apache.org/jira/browse/MESOS-5904
> https://issues.apache.org/jira/browse/MESOS-8564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defaults to `os::PATH_SEPARATOR` and so by default retains the
> previous behavior. However, now `Path` can be arbitrarily used with,
> e.g., URLs on Windows by providing `/` as the separator.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 27438d31617b3b78bf3d4deffd25c93340610e8d 
> 
> 
> Diff: https://reviews.apache.org/r/67951/diff/1/
> 
> 
> Testing
> ---
> 
> `stout-tests` and `libprocess-tests` passed on Windows and Ubuntu
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67494: Add notes against disabled tests and enbale RemoveWithContinueOnError.

[Radhika Jandhyala via Review Board]

> On June 7, 2018, 11:07 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/os_tests.cpp
> > Lines 83-90 (original), 83-92 (patched)
> > 
> >
> > We could enable this test by using `GetEnvironmentStrings` manually in 
> > place of `os::raw::environment()`, and writing a second (but very similar) 
> > loop that deals with the wstring to string conversions.

I did look into that. The authors were against that: 
//   itt is ungainly to implement the `char**` return using the Win32 APIs;
//   since `GetEnvironmentStrings` returns a completely different type,
//   we'd have to allocate a char** environment statically, or manage
//   allocation semantics of the `char**` differently across Unix and
//   Windows.
//

If you think it is needed, I can use this
https://stackoverflow.com/questions/6321756/environment-variables-are-in-a-char-how-to-get-it-to-a-stdstring?utm_medium=organic_source=google_rich_qa_campaign=google_rich_qa


> On June 7, 2018, 11:07 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/tests/os_tests.cpp
> > Line 805 (original), 816-818 (patched)
> > 
> >
> > If this is a `TODO`, we should open and reference an accompanying issue.

I don't see a JIRA issue tracking chown on Windows. So will drop


- Radhika


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


On June 7, 2018, 9:41 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67494/
> ---
> 
> (Updated June 7, 2018, 9:41 p.m.)
> 
> 
> Review request for Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-5813
> https://issues.apache.org/jira/browse/MESOS-5813
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add notes against disabled tests and enbale RemoveWithContinueOnError.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/strerror_tests.cpp 
> 6a69d52a5b9ae93f3393ab17a3282ecb30ad90b0 
>   3rdparty/stout/tests/os_tests.cpp 419879a47eb14813b8c6cac8f4a3a3bb0b9e2bed 
>   3rdparty/stout/tests/proc_tests.cpp 
> cc698b46319f418d4f609379e772bc8553346b2c 
> 
> 
> Diff: https://reviews.apache.org/r/67494/diff/1/
> 
> 
> Testing
> ---
> 
> Added notes against tests that are disabled in Windows. We can enable some of 
> these tests in the future if we do enable the required functionality on 
> Windows.
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated June 13, 2018, 6:37 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Changes
---

Rebase


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8afcaa6dee8a4216e0ae54c6d94209c001046b10 


Diff: https://reviews.apache.org/r/67286/diff/5/

Changes: https://reviews.apache.org/r/67286/diff/4-5/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

(Updated June 13, 2018, 6:36 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Changes
---

Rebase


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/5/

Changes: https://reviews.apache.org/r/67287/diff/4-5/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated June 13, 2018, 6:36 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li Li.


Changes
---

Rebase


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
17a1a3762b2012ff875e4da9c9d4622514e48051 
  src/slave/containerizer/mesos/launcher.hpp 
4b5ae730026fbb6054b390d79bb79a9cbad64485 
  src/slave/containerizer/mesos/launcher.cpp 
9df05f3f8b3c989989e9421e90d4ca6571fc819f 
  src/slave/containerizer/mesos/linux_launcher.hpp 
4c5cdd126e6916130f40d12ccc7a4c51ad6b56ab 
  src/slave/containerizer/mesos/linux_launcher.cpp 
9ce5fbf9cbf961eece12c5d336d356727b251290 
  src/tests/containerizer/launcher.hpp 776cb35a2eb05a0b65bdded44cd071038d30f882 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
362d9fb522817332da1fdf5ea4afd6bc2402005c 


Diff: https://reviews.apache.org/r/67394/diff/4/

Changes: https://reviews.apache.org/r/67394/diff/3-4/


Testing
---

All mesos tests on windows


Thanks,

Radhika Jandhyala

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 10:56 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67286/diff/4/

Changes: https://reviews.apache.org/r/67286/diff/3-4/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 10:56 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/4/

Changes: https://reviews.apache.org/r/67287/diff/3-4/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 10:56 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li Li.


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67394/diff/3/

Changes: https://reviews.apache.org/r/67394/diff/2-3/


Testing
---

All mesos tests on windows


Thanks,

Radhika Jandhyala

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 9:07 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/3/

Changes: https://reviews.apache.org/r/67287/diff/2-3/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 9:08 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67286/diff/3/

Changes: https://reviews.apache.org/r/67286/diff/2-3/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated June 12, 2018, 9:07 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li Li.


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67394/diff/2/

Changes: https://reviews.apache.org/r/67394/diff/1-2/


Testing
---

All mesos tests on windows


Thanks,

Radhika Jandhyala

Re: Review Request 67390: Windows: Integrated libwinio with libprocess code.

[Radhika Jandhyala via Review Board]

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




3rdparty/libprocess/src/libwinio_io.cpp
Lines 47 (patched)


I am not sure what the rules are, but since this is retruning WindowsError 
and making calls to the windows namespace, should this be in stout?


- Radhika Jandhyala


On May 30, 2018, 6:46 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67390/
> ---
> 
> (Updated May 30, 2018, 6:46 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668, MESOS-8671 and MESOS-8672
> https://issues.apache.org/jira/browse/MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8671
> https://issues.apache.org/jira/browse/MESOS-8672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The libprocess eventloops are used to implement the `EventLoop`,
> SocketImpl` and `io::read/write` interfaces. Thus, by providing those
> implementations using libwinio, libprocess now supports the libwinio
> backend.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libwinio.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_eventloop.cpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_io.cpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_socket.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67390/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated May 31, 2018, 10:50 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li Li.


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67394/diff/1/


Testing (updated)
---

All mesos tests on windows


Thanks,

Radhika Jandhyala

Re: Review Request 67389: Windows: Implemented Windows IOCP async backend.

[Radhika Jandhyala via Review Board]

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




3rdparty/libprocess/src/libwinio_impl.cpp
Lines 38 (patched)


$NCPUs sounds good.


- Radhika Jandhyala


On May 30, 2018, 6:54 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67389/
> ---
> 
> (Updated May 30, 2018, 6:54 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8668, MESOS-8671 and MESOS-8672
> https://issues.apache.org/jira/browse/MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8671
> https://issues.apache.org/jira/browse/MESOS-8672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows IOCP async backend, called libwinio, provides a single
> threaded event loop implementation that allows for async IO and timer
> events. libwinio wraps the native Win32 async functions using
> libprocess's primitives, which makes it easier to use and more type
> safe.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> cf443dffd0663ecf02b7efd6f7094175b94aae19 
>   3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION 
>   3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67389/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Review Request 67394: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li Li.


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67394/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67288: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated May 30, 2018, 7:04 a.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Summary (updated)
-

White list fds that child processes can inherit in stout.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

Review: https://reviews.apache.org/r/67286

White list fds that child processes can inherit in libprocess.

Review: https://reviews.apache.org/r/67287

Whitelist fds in mesos containerizer.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67288/diff/3/

Changes: https://reviews.apache.org/r/67288/diff/2-3/


Testing
---


Thanks,

Radhika Jandhyala

Review Request 67377: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

Review request for mesos.


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 
  src/tests/containerizer/launcher.hpp a8e436f164b67d937ebcff35e084d3ca755c003c 
  src/tests/containerizer/launcher.cpp a92d9890f0931425d69ef8ce0896d081b8722079 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
01f2b38cfa67b144298c361e92170322864ac201 


Diff: https://reviews.apache.org/r/67377/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67288: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

> On May 24, 2018, 10:32 p.m., Andrew Schwartzmeyer wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1878 (patched)
> > 
> >
> > Oh, also, we only need to send `pipes[0]`; as the child process only 
> > needs the `read` pipe. If I understand this correctly, this control pipe is 
> > for the parent to signal the child; that is, that child waits in a `read` 
> > on the read end of the pipe, for the parent to `write` to the write end.
> > 
> > So `std::vector whitelist_fds{pipes[0]};`

In launch.cpp, it currently closes pipes[1]. Since we are sending both pipes 
via the environment to the child, we should whitelist both, I think
 Try close = os::close(pipe[1]);
if (close.isError()) {
  cerr << "Failed to close pipe[1]: " << close.error() << endl;
  exitWithStatus(EXIT_FAILURE);
}


- Radhika


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


On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67288/
> ---
> 
> (Updated May 24, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
>   src/slave/containerizer/mesos/launcher.hpp 
> f69d934d2e1a129e10df8c7f5c78723e832adc7d 
>   src/slave/containerizer/mesos/launcher.cpp 
> 2fe47d368cb82a46328e1f636baa836272db244c 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 80e444501e429c1e1ae354abcd51f86430316ada 
> 
> 
> Diff: https://reviews.apache.org/r/67288/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

> On May 24, 2018, 6:42 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> > Lines 23 (patched)
> > 
> >
> > Do you know if this header might include `windows.h`? If it does, we 
> > should move this `#include` to `stout/windows.hpp` and then include that 
> > here to preserve Windows header ordering.

It does not include windows.h


- Radhika


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


On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67286/
> ---
> 
> (Updated May 24, 2018, 10:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
> Yu, Li Li, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> White list fds that child processes can inherit in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
> 
> 
> Diff: https://reviews.apache.org/r/67286/diff/2/
> 
> 
> Testing
> ---
> 
> All Mesos-tests
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 10:47 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

White list fds that child processes can inherit in stout.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67286/diff/2/

Changes: https://reviews.apache.org/r/67286/diff/1-2/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 10:47 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

White list fds that child processes can inherit in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/2/

Changes: https://reviews.apache.org/r/67287/diff/1-2/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67288: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 10:47 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

White list fds that child processes can inherit in mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 


Diff: https://reviews.apache.org/r/67288/diff/2/

Changes: https://reviews.apache.org/r/67288/diff/1-2/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67288: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 6:43 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

Mesos containerizer.: White list fds that child processes can inherit


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 


Diff: https://reviews.apache.org/r/67288/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 6:44 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

Stout: White list fds that child processes can inherit.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67286/diff/1/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Re: Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

(Updated May 24, 2018, 6:43 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie 
Yu, Li Li, and Radhika Jandhyala.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description (updated)
---

Libprocess: White list fds that child processes can inherit.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/1/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Review Request 67286: White list fds that child processes can inherit in stout.

[Radhika Jandhyala via Review Board]

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

Review request for mesos, Akash Gupta and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in stout.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67286/diff/1/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Review Request 67287: White list fds that child processes can inherit in libprocess.

[Radhika Jandhyala via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in libprocess.


Diffs
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 


Diff: https://reviews.apache.org/r/67287/diff/1/


Testing
---

All Mesos-tests


Thanks,

Radhika Jandhyala

Review Request 67288: White list fds that child processes can inherit in mesos containerizer.

[Radhika Jandhyala via Review Board]

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

Review request for mesos, Akash Gupta and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

White list fds that child processes can inherit in mesos containerizer.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 


Diff: https://reviews.apache.org/r/67288/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 23, 2018, 11:14 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 
  src/slave/containerizer/mesos/linux_launcher.cpp 
80e444501e429c1e1ae354abcd51f86430316ada 


Diff: https://reviews.apache.org/r/67201/diff/10/

Changes: https://reviews.apache.org/r/67201/diff/9-10/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 23, 2018, 4:54 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 


Diff: https://reviews.apache.org/r/67201/diff/9/

Changes: https://reviews.apache.org/r/67201/diff/8-9/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 23, 2018, 4:40 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 


Diff: https://reviews.apache.org/r/67201/diff/8/

Changes: https://reviews.apache.org/r/67201/diff/7-8/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 22, 2018, 8:42 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Changes
---

MesosContainerizerProcess::_launch puts read write pipes into the environment 
which is used to create process. These handles need to be whitelisted as well, 
so pass them down the stack as an option


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
6a1262340c333b617402637e648c12769827ffc4 
  3rdparty/libprocess/src/subprocess.cpp 
d7a725363251f9c54072cd7551f5598696938308 
  3rdparty/libprocess/src/subprocess_windows.hpp 
c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f 
  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/launcher.hpp 
f69d934d2e1a129e10df8c7f5c78723e832adc7d 
  src/slave/containerizer/mesos/launcher.cpp 
2fe47d368cb82a46328e1f636baa836272db244c 
  src/slave/containerizer/mesos/linux_launcher.hpp 
0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 


Diff: https://reviews.apache.org/r/67201/diff/7/

Changes: https://reviews.apache.org/r/67201/diff/6-7/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 19, 2018, 4:42 a.m.)


Review request for mesos and Andrew Schwartzmeyer.


Changes
---

The following statements should be within pipes.isSome() block
startup_info_ex.StartupInfo.hStdInput = std::get<0>(pipes.get());


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/6/

Changes: https://reviews.apache.org/r/67201/diff/5-6/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 18, 2018, 10:48 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Changes
---

Fixed formatting


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/5/

Changes: https://reviews.apache.org/r/67201/diff/4-5/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 18, 2018, 10:46 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/4/

Changes: https://reviews.apache.org/r/67201/diff/3-4/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 18, 2018, 10:32 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/3/

Changes: https://reviews.apache.org/r/67201/diff/2-3/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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




3rdparty/stout/include/stout/internal/windows/inherit.hpp
Lines 33 (patched)


Please note that the data allocated in handles needs to be alive for the 
lifetime of AttributeList


- Radhika Jandhyala


On May 18, 2018, 9:59 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67201/
> ---
> 
> (Updated May 18, 2018, 9:59 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whitelist handles that can be inherited by a child process.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
> 
> 
> Diff: https://reviews.apache.org/r/67201/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

(Updated May 18, 2018, 9:59 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/2/

Changes: https://reviews.apache.org/r/67201/diff/1-2/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> > Lines 34 (patched)
> > 
> >
> > What do the second and third arguments represent?

Second argument is the number of attributes you want to update. The third one 
is reserved(windows... sigh)


> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> > Lines 41-43 (patched)
> > 
> >
> > We should probably use a `std::unique_ptr` 
> > here for RAII.

Defined an std::vector since _PROC_THREAD_ATTRIBUTE_LIST is not defined in a 
public header


> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> > Lines 50 (patched)
> > 
> >
> > RAII takes care of this ;)
> > 
> > I noticed below we also called `DeleteProcThreadAttributeList`. Under 
> > what conditions do we `DeleteProcThreadAttributeList` and `free`, and when 
> > do we just `free`?

DeleteProcThreadAttributeList is used to release memory when 
proc_thread_attribute_list is initialized successfully. You have to call free 
to deallocate it(due to the malloc)


> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/os/windows/shell.hpp
> > Line 269 (original), 270 (patched)
> > 
> >
> > Is there a `W` version of this, or is that redundant...? I don't know.

If UNICODE is defined(and it is), the function names are automatically suffixed 
wth W. I changed to W version to be consistent for now.


> On May 17, 2018, 11:21 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/os/windows/shell.hpp
> > Lines 279-291 (original), 283-306 (patched)
> > 
> >
> > (Old comment: Since the size is `constexpr` (it's always 3), I don't 
> > think we need to malloc on the heap. We can just use `std::array > 3>`.)
> > 
> > Actually, I don't think you even need `handle_array` at all. `pipes` is 
> > already a `std::array` (and `int_fd` in this case is `HANDLE`). 
> > You can use the 
> > [data()](http://en.cppreference.com/w/cpp/container/array/data) member of 
> > `std::array` to get access to the underlying `HANDLE[]`.
> 
> Andrew Schwartzmeyer wrote:
> Oops, I was rushing. `int_fd` is castable to `HANDLE` but is an 
> abstracted data type that has another field. So you'd need 
> `std::vector handles(pipes->cbegin(), pipes->cend())` and then pass 
> `handles.data()`. But still, allocated automatically with no need to rfree :)

Thanks.


- Radhika


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


On May 17, 2018, 10:18 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67201/
> ---
> 
> (Updated May 17, 2018, 10:18 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whitelist handles that can be inherited by a child process.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c 
> 
> 
> Diff: https://reviews.apache.org/r/67201/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Review Request 67201: Whitelist handles that can be inherited by a child process.

[Radhika Jandhyala via Review Board]

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

Review request for mesos and Andrew Schwartzmeyer.


Bugs: MESOS-8926
https://issues.apache.org/jira/browse/MESOS-8926


Repository: mesos


Description
---

Whitelist handles that can be inherited by a child process.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
7dbde820e775cbaeb8db4bc4559ab432903e75ea 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
8da612af2888ff4d4d458ea5b16cdb08779b6f4c 


Diff: https://reviews.apache.org/r/67201/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67075: Fix TEST_F(SubprocessTest, Flags)

[Radhika Jandhyala via Review Board]

> On May 14, 2018, 9:09 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp
> > Lines 779-796 (patched)
> > 
> >
> > This is a good and informative comment; however, [Mesos 
> > style](https://mesos.apache.org/documentation/latest/c++-style-guide/#comments)
> >  wants:
> > 
> > * End each sentence within a comment with a punctuation mark (please 
> > note that we generally prefer periods); this applies to incomplete 
> > sentences as well.
> > * Use backticks when quoting code excerpts or object/variable/function 
> > names.
> > 
> > So something like:
> > 
> > ```
> >   // The Windows version of `echo` is a built-in of the command
> >   // prompt, and it simply reproduces the entire command line string.
> >   // However, the flags class (and thus this test) is expecting the
> >   // semantics of a native binary interpreting the command line
> >   // arguments via the Windows API `CommandLineToArgv`. When a regular
> >   // Windows application (in contrast to `echo`) gets command line
> >   // arguments, the text is processed automatically by
> >   // `CommandLineToArgv`, which converts the command line string into
> >   // an array. For example, this is the output of `echo`:
> >   //
> >   //> cmd.exe /c echo "--s3=\"geek\""
> >   //"--s3=\"geek\""
> >   //
> >   // With `test-echo.exe`, a small native binary that just prints its
> >   // arguments, the output is:
> >   //
> >   // > test-echo.exe "--s3=\"geek\""
> >   // --s3="geek"
> >   //
> >   // This is the behavior expected by the test as the POSIX version of
> >   // `echo` is a native binary.
> > ```

Thanks for writing that up. It is so well written.


- Radhika


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


On May 16, 2018, 5:55 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67075/
> ---
> 
> (Updated May 16, 2018, 5:55 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-8857
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8857
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/67152
> 
> Remove '\\' prefix for test-echo while joining with build dir
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 1b2d75d2b576e528db49635fce54174d56a29c59 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 269918e7f795bb62f8acd84cfc4233ff8682f90d 
>   3rdparty/libprocess/src/tests/test_echo.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67075/diff/4/
> 
> 
> Testing
> ---
> 
> [--] 18 tests from SubprocessTest
> [ RUN  ] SubprocessTest.PipeOutputToFileDescriptor
> [   OK ] SubprocessTest.PipeOutputToFileDescriptor (38 ms)
> [ RUN  ] SubprocessTest.PipeOutputToPath
> [   OK ] SubprocessTest.PipeOutputToPath (33 ms)
> [ RUN  ] SubprocessTest.EnvironmentEcho
> [   OK ] SubprocessTest.EnvironmentEcho (22 ms)
> [ RUN  ] SubprocessTest.Status
> [   OK ] SubprocessTest.Status (29 ms)
> [ RUN  ] SubprocessTest.PipeOutput
> [   OK ] SubprocessTest.PipeOutput (25 ms)
> [ RUN  ] SubprocessTest.PipeLargeOutput
> [   OK ] SubprocessTest.PipeLargeOutput (18 ms)
> [ RUN  ] SubprocessTest.PipeInput
> [   OK ] SubprocessTest.PipeInput (248 ms)
> [ RUN  ] SubprocessTest.PipeRedirect
> [   OK ] SubprocessTest.PipeRedirect (31 ms)
> [ RUN  ] SubprocessTest.PathOutput
> [   OK ] SubprocessTest.PathOutput (54 ms)
> [ RUN  ] SubprocessTest.PathInput
> [   OK ] SubprocessTest.PathInput (232 ms)
> [ RUN  ] SubprocessTest.FdOutput
> [   OK ] SubprocessTest.FdOutput (45 ms)
> [ RUN  ] SubprocessTest.FdInput
> [   OK ] SubprocessTest.FdInput (237 ms)
> [ RUN  ] SubprocessTest.Default
> hello world
> [   OK ] SubprocessTest.Default (21 ms)
> [ RUN  ] SubprocessTest.Flags
> [   OK ] SubprocessTest.Flags (34 ms)
> [ RUN  ] SubprocessTest.Environment
> [   OK ] SubprocessTest.Environment (32 ms)
> [ RUN  ] SubprocessTest.EnvironmentWithSpaces
> [   OK ] SubprocessTest.EnvironmentWithSpaces (19 ms)
> [ RUN  ] SubprocessTest.EnvironmentWithSpacesAndQuotes
> [   OK ] SubprocessTest.EnvironmentWithSpacesAndQuotes (17 ms)
> [ RUN  ] SubprocessTest.EnvironmentOverride
> [   OK ] SubprocessTest.EnvironmentOverride (17 

Re: Review Request 67075: Fix TEST_F(SubprocessTest, Flags)

[Radhika Jandhyala via Review Board]

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

(Updated May 16, 2018, 5:55 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: https://issues.apache.org/jira/browse/MESOS-8857

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8857


Repository: mesos


Description (updated)
---

Review: https://reviews.apache.org/r/67152

Remove '\\' prefix for test-echo while joining with build dir


Diffs (updated)
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
1b2d75d2b576e528db49635fce54174d56a29c59 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
269918e7f795bb62f8acd84cfc4233ff8682f90d 
  3rdparty/libprocess/src/tests/test_echo.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67075/diff/4/

Changes: https://reviews.apache.org/r/67075/diff/3-4/


Testing
---

[--] 18 tests from SubprocessTest
[ RUN  ] SubprocessTest.PipeOutputToFileDescriptor
[   OK ] SubprocessTest.PipeOutputToFileDescriptor (38 ms)
[ RUN  ] SubprocessTest.PipeOutputToPath
[   OK ] SubprocessTest.PipeOutputToPath (33 ms)
[ RUN  ] SubprocessTest.EnvironmentEcho
[   OK ] SubprocessTest.EnvironmentEcho (22 ms)
[ RUN  ] SubprocessTest.Status
[   OK ] SubprocessTest.Status (29 ms)
[ RUN  ] SubprocessTest.PipeOutput
[   OK ] SubprocessTest.PipeOutput (25 ms)
[ RUN  ] SubprocessTest.PipeLargeOutput
[   OK ] SubprocessTest.PipeLargeOutput (18 ms)
[ RUN  ] SubprocessTest.PipeInput
[   OK ] SubprocessTest.PipeInput (248 ms)
[ RUN  ] SubprocessTest.PipeRedirect
[   OK ] SubprocessTest.PipeRedirect (31 ms)
[ RUN  ] SubprocessTest.PathOutput
[   OK ] SubprocessTest.PathOutput (54 ms)
[ RUN  ] SubprocessTest.PathInput
[   OK ] SubprocessTest.PathInput (232 ms)
[ RUN  ] SubprocessTest.FdOutput
[   OK ] SubprocessTest.FdOutput (45 ms)
[ RUN  ] SubprocessTest.FdInput
[   OK ] SubprocessTest.FdInput (237 ms)
[ RUN  ] SubprocessTest.Default
hello world
[   OK ] SubprocessTest.Default (21 ms)
[ RUN  ] SubprocessTest.Flags
[   OK ] SubprocessTest.Flags (34 ms)
[ RUN  ] SubprocessTest.Environment
[   OK ] SubprocessTest.Environment (32 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpaces
[   OK ] SubprocessTest.EnvironmentWithSpaces (19 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpacesAndQuotes
[   OK ] SubprocessTest.EnvironmentWithSpacesAndQuotes (17 ms)
[ RUN  ] SubprocessTest.EnvironmentOverride
[   OK ] SubprocessTest.EnvironmentOverride (17 ms)
[--] 18 tests from SubprocessTest (1201 ms total)


Thanks,

Radhika Jandhyala

Re: Review Request 67075: Fix TEST_F(SubprocessTest, Flags)

[Radhika Jandhyala via Review Board]

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

(Updated May 16, 2018, 5:07 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: https://issues.apache.org/jira/browse/MESOS-8857

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8857


Repository: mesos


Description (updated)
---

Review: https://reviews.apache.org/r/67152


Diffs (updated)
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
1b2d75d2b576e528db49635fce54174d56a29c59 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
269918e7f795bb62f8acd84cfc4233ff8682f90d 
  3rdparty/libprocess/src/tests/test_echo.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67075/diff/3/

Changes: https://reviews.apache.org/r/67075/diff/2-3/


Testing
---

[--] 18 tests from SubprocessTest
[ RUN  ] SubprocessTest.PipeOutputToFileDescriptor
[   OK ] SubprocessTest.PipeOutputToFileDescriptor (38 ms)
[ RUN  ] SubprocessTest.PipeOutputToPath
[   OK ] SubprocessTest.PipeOutputToPath (33 ms)
[ RUN  ] SubprocessTest.EnvironmentEcho
[   OK ] SubprocessTest.EnvironmentEcho (22 ms)
[ RUN  ] SubprocessTest.Status
[   OK ] SubprocessTest.Status (29 ms)
[ RUN  ] SubprocessTest.PipeOutput
[   OK ] SubprocessTest.PipeOutput (25 ms)
[ RUN  ] SubprocessTest.PipeLargeOutput
[   OK ] SubprocessTest.PipeLargeOutput (18 ms)
[ RUN  ] SubprocessTest.PipeInput
[   OK ] SubprocessTest.PipeInput (248 ms)
[ RUN  ] SubprocessTest.PipeRedirect
[   OK ] SubprocessTest.PipeRedirect (31 ms)
[ RUN  ] SubprocessTest.PathOutput
[   OK ] SubprocessTest.PathOutput (54 ms)
[ RUN  ] SubprocessTest.PathInput
[   OK ] SubprocessTest.PathInput (232 ms)
[ RUN  ] SubprocessTest.FdOutput
[   OK ] SubprocessTest.FdOutput (45 ms)
[ RUN  ] SubprocessTest.FdInput
[   OK ] SubprocessTest.FdInput (237 ms)
[ RUN  ] SubprocessTest.Default
hello world
[   OK ] SubprocessTest.Default (21 ms)
[ RUN  ] SubprocessTest.Flags
[   OK ] SubprocessTest.Flags (34 ms)
[ RUN  ] SubprocessTest.Environment
[   OK ] SubprocessTest.Environment (32 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpaces
[   OK ] SubprocessTest.EnvironmentWithSpaces (19 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpacesAndQuotes
[   OK ] SubprocessTest.EnvironmentWithSpacesAndQuotes (17 ms)
[ RUN  ] SubprocessTest.EnvironmentOverride
[   OK ] SubprocessTest.EnvironmentOverride (17 ms)
[--] 18 tests from SubprocessTest (1201 ms total)


Thanks,

Radhika Jandhyala

Re: Review Request 67075: Fix TEST_F(SubprocessTest, Flags)

[Radhika Jandhyala via Review Board]

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

(Updated May 15, 2018, 11:14 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


Bugs: https://issues.apache.org/jira/browse/MESOS-8857

https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8857


Repository: mesos


Description
---

Fix TEST_F(SubprocessTest, Flags)


Diffs (updated)
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
1b2d75d2b576e528db49635fce54174d56a29c59 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
269918e7f795bb62f8acd84cfc4233ff8682f90d 
  3rdparty/libprocess/src/tests/test_echo.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67075/diff/2/

Changes: https://reviews.apache.org/r/67075/diff/1-2/


Testing
---

[--] 18 tests from SubprocessTest
[ RUN  ] SubprocessTest.PipeOutputToFileDescriptor
[   OK ] SubprocessTest.PipeOutputToFileDescriptor (38 ms)
[ RUN  ] SubprocessTest.PipeOutputToPath
[   OK ] SubprocessTest.PipeOutputToPath (33 ms)
[ RUN  ] SubprocessTest.EnvironmentEcho
[   OK ] SubprocessTest.EnvironmentEcho (22 ms)
[ RUN  ] SubprocessTest.Status
[   OK ] SubprocessTest.Status (29 ms)
[ RUN  ] SubprocessTest.PipeOutput
[   OK ] SubprocessTest.PipeOutput (25 ms)
[ RUN  ] SubprocessTest.PipeLargeOutput
[   OK ] SubprocessTest.PipeLargeOutput (18 ms)
[ RUN  ] SubprocessTest.PipeInput
[   OK ] SubprocessTest.PipeInput (248 ms)
[ RUN  ] SubprocessTest.PipeRedirect
[   OK ] SubprocessTest.PipeRedirect (31 ms)
[ RUN  ] SubprocessTest.PathOutput
[   OK ] SubprocessTest.PathOutput (54 ms)
[ RUN  ] SubprocessTest.PathInput
[   OK ] SubprocessTest.PathInput (232 ms)
[ RUN  ] SubprocessTest.FdOutput
[   OK ] SubprocessTest.FdOutput (45 ms)
[ RUN  ] SubprocessTest.FdInput
[   OK ] SubprocessTest.FdInput (237 ms)
[ RUN  ] SubprocessTest.Default
hello world
[   OK ] SubprocessTest.Default (21 ms)
[ RUN  ] SubprocessTest.Flags
[   OK ] SubprocessTest.Flags (34 ms)
[ RUN  ] SubprocessTest.Environment
[   OK ] SubprocessTest.Environment (32 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpaces
[   OK ] SubprocessTest.EnvironmentWithSpaces (19 ms)
[ RUN  ] SubprocessTest.EnvironmentWithSpacesAndQuotes
[   OK ] SubprocessTest.EnvironmentWithSpacesAndQuotes (17 ms)
[ RUN  ] SubprocessTest.EnvironmentOverride
[   OK ] SubprocessTest.EnvironmentOverride (17 ms)
[--] 18 tests from SubprocessTest (1201 ms total)


Thanks,

Radhika Jandhyala

Review Request 67123: Address comments

[Radhika Jandhyala via Review Board]

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

Review request for mesos.


Repository: mesos


Description
---

Address comments


Diffs
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
1b2d75d2b576e528db49635fce54174d56a29c59 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
269918e7f795bb62f8acd84cfc4233ff8682f90d 
  3rdparty/libprocess/src/tests/test_flags.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67123/diff/1/


Testing
---


Thanks,

Radhika Jandhyala

Re: Review Request 67075: Fix TEST_F(SubprocessTest, Flags)

[Radhika Jandhyala via Review Board]

> On May 14, 2018, 9:09 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp
> > Lines 797-802 (patched)
> > 
> >
> > Instead of this Windows code to construct the path to the exe, you can 
> > just use:
> > 
> > ```
> > path::join(BUILD_DIR, "test-echo.exe")
> > ```
> > 
> > (See the use of this in `process_tests.cpp` for `test-linkee` as an 
> > example.)

BUILD_DIR does not contain "debug" or "release" which is where the exes get 
placed


> On May 14, 2018, 9:09 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/libprocess/src/tests/test_flags.cpp
> > Lines 13-17 (patched)
> > 
> >
> > Mesos style uses C++ish `//` comments instead of Cish `/* */` comments. 
> > See 
> > [here](https://mesos.apache.org/documentation/latest/c++-style-guide/#comments)
> >  for more.

I have done as you suggested. test-linkee uses both // and /*


- Radhika


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


On May 10, 2018, 10:24 p.m., Radhika Jandhyala wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67075/
> ---
> 
> (Updated May 10, 2018, 10:24 p.m.)
> 
> 
> Review request for Andrew Schwartzmeyer.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-8857
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8857
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix TEST_F(SubprocessTest, Flags)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 1b2d75d2b576e528db49635fce54174d56a29c59 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 269918e7f795bb62f8acd84cfc4233ff8682f90d 
>   3rdparty/libprocess/src/tests/test_flags.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67075/diff/1/
> 
> 
> Testing
> ---
> 
> [--] 18 tests from SubprocessTest
> [ RUN  ] SubprocessTest.PipeOutputToFileDescriptor
> [   OK ] SubprocessTest.PipeOutputToFileDescriptor (38 ms)
> [ RUN  ] SubprocessTest.PipeOutputToPath
> [   OK ] SubprocessTest.PipeOutputToPath (33 ms)
> [ RUN  ] SubprocessTest.EnvironmentEcho
> [   OK ] SubprocessTest.EnvironmentEcho (22 ms)
> [ RUN  ] SubprocessTest.Status
> [   OK ] SubprocessTest.Status (29 ms)
> [ RUN  ] SubprocessTest.PipeOutput
> [   OK ] SubprocessTest.PipeOutput (25 ms)
> [ RUN  ] SubprocessTest.PipeLargeOutput
> [   OK ] SubprocessTest.PipeLargeOutput (18 ms)
> [ RUN  ] SubprocessTest.PipeInput
> [   OK ] SubprocessTest.PipeInput (248 ms)
> [ RUN  ] SubprocessTest.PipeRedirect
> [   OK ] SubprocessTest.PipeRedirect (31 ms)
> [ RUN  ] SubprocessTest.PathOutput
> [   OK ] SubprocessTest.PathOutput (54 ms)
> [ RUN  ] SubprocessTest.PathInput
> [   OK ] SubprocessTest.PathInput (232 ms)
> [ RUN  ] SubprocessTest.FdOutput
> [   OK ] SubprocessTest.FdOutput (45 ms)
> [ RUN  ] SubprocessTest.FdInput
> [   OK ] SubprocessTest.FdInput (237 ms)
> [ RUN  ] SubprocessTest.Default
> hello world
> [   OK ] SubprocessTest.Default (21 ms)
> [ RUN  ] SubprocessTest.Flags
> [   OK ] SubprocessTest.Flags (34 ms)
> [ RUN  ] SubprocessTest.Environment
> [   OK ] SubprocessTest.Environment (32 ms)
> [ RUN  ] SubprocessTest.EnvironmentWithSpaces
> [   OK ] SubprocessTest.EnvironmentWithSpaces (19 ms)
> [ RUN  ] SubprocessTest.EnvironmentWithSpacesAndQuotes
> [   OK ] SubprocessTest.EnvironmentWithSpacesAndQuotes (17 ms)
> [ RUN  ] SubprocessTest.EnvironmentOverride
> [   OK ] SubprocessTest.EnvironmentOverride (17 ms)
> [--] 18 tests from SubprocessTest (1201 ms total)
> 
> 
> Thanks,
> 
> Radhika Jandhyala
> 
>

Re: Review Request 67021: Windows: Enabled `ProtobufTest` suite.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 8, 2018, 11:41 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67021/
> ---
> 
> (Updated May 8, 2018, 11:41 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests just worked once added to the build. Also, out-of-date
> code was deleted from `systems_tests.cpp`, and a note added to the
> build file that it (and `signals_tests.cpp`) will be not be ported to
> Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
>   3rdparty/stout/tests/os/systems_tests.cpp 
> a2a05ce28d11cdb97a511e5392a05522a45826c8 
> 
> 
> Diff: https://reviews.apache.org/r/67021/diff/1/
> 
> 
> Testing
> ---
> 
> I just noticed that these weren't enabled when checking something else. 
> Turned them on, they passed. Will push when CI passes.
> 
> ```
> Note: Google Test filter = ProtobufTest.*-
> [==] Running 12 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 12 tests from ProtobufTest
> [ RUN  ] ProtobufTest.JSON
> [   OK ] ProtobufTest.JSON (17 ms)
> [ RUN  ] ProtobufTest.JSONArray
> [   OK ] ProtobufTest.JSONArray (1 ms)
> [ RUN  ] ProtobufTest.JsonLargeIntegers
> [   OK ] ProtobufTest.JsonLargeIntegers (2 ms)
> [ RUN  ] ProtobufTest.SimpleMessageEquals
> [   OK ] ProtobufTest.SimpleMessageEquals (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONArray
> [   OK ] ProtobufTest.ParseJSONArray (1 ms)
> [ RUN  ] ProtobufTest.ParseJSONNull
> [   OK ] ProtobufTest.ParseJSONNull (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONNestedError
> [   OK ] ProtobufTest.ParseJSONNestedError (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONUnrecognizedEnum
> [   OK ] ProtobufTest.ParseJSONUnrecognizedEnum (1 ms)
> [ RUN  ] ProtobufTest.Jsonify
> [   OK ] ProtobufTest.Jsonify (1 ms)
> [ RUN  ] ProtobufTest.JsonifyArray
> [   OK ] ProtobufTest.JsonifyArray (0 ms)
> [ RUN  ] ProtobufTest.JsonifyLargeIntegers
> [   OK ] ProtobufTest.JsonifyLargeIntegers (1 ms)
> [ RUN  ] ProtobufTest.JsonifyMap
> [   OK ] ProtobufTest.JsonifyMap (6 ms)
> [--] 12 tests from ProtobufTest (30 ms total)
> 
> [--] Global test environment tear-down
> [==] 12 tests from 1 test case ran. (31 ms total)
> [  PASSED  ] 12 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66954: Windows: Added overlapped aware io.hpp file for `os::read/write`.

[Radhika Jandhyala via Review Board]

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




3rdparty/stout/include/stout/os/windows/io.hpp
Lines 61 (patched)


The in params can be const here



3rdparty/stout/include/stout/os/windows/io.hpp
Lines 89 (patched)


Param can be const along with data and size


- Radhika Jandhyala


On May 4, 2018, 5:21 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66954/
> ---
> 
> (Updated May 4, 2018, 5:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper file for io, because `os::read/write` need to work on
> both non-overlapped and overlapped files, so the logic has gotten
> more complicated. The new file, `io.hpp`, has all the shared logic.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am f2e60224d9c729497bbcfffbec989c4f355d8f85 
>   3rdparty/stout/include/stout/os/windows/io.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66954/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66954: Windows: Added overlapped aware io.hpp file for `os::read/write`.

[Radhika Jandhyala via Review Board]

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




3rdparty/stout/include/stout/os/windows/io.hpp
Lines 175 (patched)


Can this be used to return Result instead


- Radhika Jandhyala


On May 4, 2018, 5:21 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66954/
> ---
> 
> (Updated May 4, 2018, 5:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper file for io, because `os::read/write` need to work on
> both non-overlapped and overlapped files, so the logic has gotten
> more complicated. The new file, `io.hpp`, has all the shared logic.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am f2e60224d9c729497bbcfffbec989c4f355d8f85 
>   3rdparty/stout/include/stout/os/windows/io.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66954/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

[Radhika Jandhyala via Review Board]

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




3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 793 (patched)


Since read_async returned result before write_async finished, is result 
being updated after it is returnend at line 779?


- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!


src/slave/containerizer/mesos/containerizer.cpp
Lines 1864 (patched)


Would this mean that on windows process launched by the containerizer would 
not be able to perform async io?


- Radhika Jandhyala


On May 4, 2018, 5:19 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66959/
> ---
> 
> (Updated May 4, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos containerizer needs an inheritable pipe that is not used for
> stdio of the child process. It is used for signaling when the child
> process should start running, so the pipe needs to be inheritable and
> not overlapped.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef 
> 
> 
> Diff: https://reviews.apache.org/r/66959/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66956: Windows: Added overlapped support to `os::write`.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:21 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66956/
> ---
> 
> (Updated May 4, 2018, 5:21 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added overlapped handle support to `os::write`. Now, `os::write` will do
> a blocking write no matter the handle type. Also, `os::write_async` will
> do an overlapped write on an overlapped handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 295c031c5824d13d74e2c9006e62c391d5020f69 
> 
> 
> Diff: https://reviews.apache.org/r/66956/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66952: Windows: Added overlapped field to WindowsFD.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:15 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66952/
> ---
> 
> (Updated May 4, 2018, 5:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670 and MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an field to WindowsFD for functions that need to know if a
> Windows file is opened in overlapped mode, such as `os::read`, since
> Windows doesn't provide a Win32 API for it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> bab16e869e69c214e18de584d1615311316e001a 
>   3rdparty/stout/include/stout/os/windows/open.hpp 
> 701dcec7c4c1162d38d8f25596af29ed3b32691d 
> 
> 
> Diff: https://reviews.apache.org/r/66952/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66953: Windows: Fixed dup to properly copy the overlapped WindowsFD.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:16 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66953/
> ---
> 
> (Updated May 4, 2018, 5:16 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After the previous commit,`os::dup` was not copying the overlapped
> field, so it has been updated to copy it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> af98054f1bd9c8e55c52b246fda8734e3ca96e21 
> 
> 
> Diff: https://reviews.apache.org/r/66953/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66962: Windows: Added tests for async IO functions.

[Radhika Jandhyala via Review Board]

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




3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 724-729 (patched)


I believe definitions for read_async/write_async might not be in master as 
yet. Would this break the build?



3rdparty/stout/tests/os/filesystem_tests.cpp
Lines 769 (patched)


Does this really block? Then execution would not proceed to read_async?


- Radhika Jandhyala


On May 4, 2018, 5:27 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66962/
> ---
> 
> (Updated May 4, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New asynchronous functions were introduced to the Windows stout read
> and write implementations, so eztra tests were added.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a 
> 
> 
> Diff: https://reviews.apache.org/r/66962/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66952: Windows: Added overlapped field to WindowsFD.

[Radhika Jandhyala via Review Board]

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


Ship it!




Ship It!

- Radhika Jandhyala


On May 4, 2018, 5:15 p.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66952/
> ---
> 
> (Updated May 4, 2018, 5:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, 
> John Kordich, Joseph Wu, and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-8670 and MESOS-8674
> https://issues.apache.org/jira/browse/MESOS-8670
> https://issues.apache.org/jira/browse/MESOS-8674
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an field to WindowsFD for functions that need to know if a
> Windows file is opened in overlapped mode, such as `os::read`, since
> Windows doesn't provide a Win32 API for it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> bab16e869e69c214e18de584d1615311316e001a 
>   3rdparty/stout/include/stout/os/windows/open.hpp 
> 701dcec7c4c1162d38d8f25596af29ed3b32691d 
> 
> 
> Diff: https://reviews.apache.org/r/66952/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 66892: Added `SubprocessTest.PipeLargeOutput`.

[radhika jandhyala via Review Board]

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


Ship it!




Ship It!

- radhika jandhyala


On May 1, 2018, 9:24 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66892/
> ---
> 
> (Updated May 1, 2018, 9:24 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and radhika 
> jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new test checks exercises our `Subprocess::PIPE()` logic more
> thoroughly by specifically piping enough data such that a single
> memory page is insufficient to hold it. This is especially important
> on Windows because the OS-allocated buffer is the size of one memory
> page. On Windows, it also tests that the expected end-of-file
> condition, `ERROR_BROKEN_PIPE`, is set.
> 
> We also fix another use of `os::WindowsFD` in place of `int_fd`, the
> public name of the type; added an important note about why we hold
> onto the process's handle on Windows; pass the file descriptor structs
> by const-ref instead of value; and finally check the return value
> `os::close()` in `createChildProcess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
>   3rdparty/libprocess/src/subprocess_windows.hpp 
> 4cc5675621f0846229e9358d70412f44138e7904 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> be99bd6a88b2abcabed8d4d16763c2e29f38eb14 
> 
> 
> Diff: https://reviews.apache.org/r/66892/diff/1/
> 
> 
> Testing
> ---
> 
> `ctest -V` on Linux and Windows (also running `make check` with Autotools).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>