Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-19 Thread John Kordich via Review Board

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

(Updated Sept. 19, 2017, 4:57 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Greg Mann, Joseph 
Wu, and Till Toenshoff.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
  3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
  3rdparty/cyrus-sasl-2.1.27rc3.patch PRE-CREATION 


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

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


Testing
---


Thanks,

John Kordich



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-19 Thread John Kordich via Review Board

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

(Updated Sept. 19, 2017, 4:57 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Greg Mann, Jie 
Yu, Joseph Wu, and Till Toenshoff.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Enabled CRAM MD5 Authentication on Windows and associated tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
  docs/configuration-cmake.md d2eb571c9458c7fade415e8077c676dd34e63242 
  docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
  src/sched/sched.cpp 09f255e4affe79b7482ba62efcfa5cf78327735e 
  src/slave/slave.cpp 391a0d6db29660d5eb38eab367a2ccf9d453e69d 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
  src/tests/mesos.cpp fc7f8cb09e223000ea160aa4ab83a003cf4af2fb 
  support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 


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

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


Testing
---

Ran the automake build and the cmake build on Linux, as well as the cmake build 
on Windows, with no errors.  Ran tests on both platforms as well.  Note: The 
mesos-3rdparty pull request which contains the cyrus-sasl package needs to be 
merged to master first before these patches will work. (Or, have the cyrus-sasl 
tarball available in a local 3rdparty directory, configured with mesos)


Thanks,

John Kordich



Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-19 Thread John Kordich via Review Board

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

(Updated Sept. 19, 2017, 4:58 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Greg Mann, Joseph 
Wu, and Till Toenshoff.


Changes
---

Rebased on new changes for dependent patches


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


Repository: mesos


Description
---

Added cmake dependency check for libsasl2 on non-Windows platforms.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 


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

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


Testing
---

I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
exist, it fails the cmake configure/build.  I did not test this on any other 
platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
Windows build.  I'm uncertain if there is much support for other kinds of 
builds (like Mac OS), but this should be platform independent.


Thanks,

John Kordich



Review Request 63590: Enabled CredentialsTests on Windows.

2017-11-06 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Enabled CredentialsTests on Windows.


Diffs
-

  src/tests/credentials_tests.cpp 7eb5abd21a1be35d4739c4928cb922f028cfc9a7 


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


Testing
---

Tested on Windows, all tests pass. This is a set of tests that was not enabled 
after the authentication changes. Not tested on non-Windows systems, as this 
change should have zero effect on non-Windows systems.

On Windows:
[--] 3 tests from CredentialsTest
[ RUN  ] CredentialsTest.AuthenticatedSlave
[   OK ] CredentialsTest.AuthenticatedSlave (123 ms)
[ RUN  ] CredentialsTest.AuthenticatedSlaveText
[   OK ] CredentialsTest.AuthenticatedSlaveText (119 ms)
[ RUN  ] CredentialsTest.AuthenticatedSlaveJSON
[   OK ] CredentialsTest.AuthenticatedSlaveJSON (114 ms)
[--] 3 tests from CredentialsTest (370 ms total)


Thanks,

John Kordich



Re: Review Request 63809: Windows: Fixed symlink code to not need admin privileges.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63809/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-7370
> https://issues.apache.org/jira/browse/MESOS-7370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This new feature of Windows 10 allows us to make symlinks without being
> an administrator. In the future, we may want to do a version check, as
> this is likely to fail on versions of Windows which don't support this
> flag. Resolves MESOS-7370.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
> 
> 
> Diff: https://reviews.apache.org/r/63809/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `mesos-tests` not in an admin prompt!
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63810: Windows: Added internal `fullpath` API to normalize paths.

2017-11-15 Thread John Kordich via Review Board

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


Fix it, then Ship it!





3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp
Lines 178 (patched)


I've seen this function "stringify" before, but I don't have any context 
about it. Does it convert from wide string to UTF-8?  Do we need to worry about 
non-ascii paths?


- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63810/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This explicitly does not check for existence, nor does it follow
> symlinks. It simply normalizes a given path to an absolute path. This
> removes the `reparsepoint.hpp` and `symlink.hpp` dependency os
> `os::realpath`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
>   3rdparty/stout/include/stout/internal/windows/symlink.hpp 
> b9cf1229ec0b6c70fe9b9d358e867e91e475dfaa 
>   3rdparty/stout/include/stout/windows/fs.hpp 
> d7b883ca1f3f8972e1bf9fbd211cc564c7c30f14 
> 
> 
> Diff: https://reviews.apache.org/r/63810/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63811: Windows: Added `get_handle_follow` which follows symlinks.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




I'm not an expert, but I don't see any problems here. Now I know how to 
dereference Windows Symlinks when opening them using the Windows API. Cool!

- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63811/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the opposite of `get_handle_no_follow`. They both use the
> inappropriately named Windows API `CreateFile` to retrieve a `HANDLE` to
> an existing file or directory. This existing `get_handle_no_follow`
> explicitly gets a handle to the symlink itself, and the new
> `get_handle_follow` explicitly resolves symlinks and gets a handle to
> the target.
> 
> This may fail if the target doesn't exist.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
> e3d04480cf793482d958ea9571d96a29b6bf09b1 
> 
> 
> Diff: https://reviews.apache.org/r/63811/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63812: Windows: Fixed `os::realpath` to behave like POSIX version.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:14 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63812/
> ---
> 
> (Updated Nov. 14, 2017, 11:14 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing implementation (1) did not resolve symlinks and (2) did not
> check for existence of the path. This resulted in unexpected behaviors
> by users of the function.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/realpath.hpp 
> 81a33bd2708c0871c4f23c12fdd29fc8d35682e3 
> 
> 
> Diff: https://reviews.apache.org/r/63812/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63813: Windows: Fixed name of default executor.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:15 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63813/
> ---
> 
> (Updated Nov. 14, 2017, 11:15 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6735
> https://issues.apache.org/jira/browse/MESOS-6735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This path is checked with `os::realpath`, which now correctly errors if
> the path does not exist. Therefore the name must be correct.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 401590605350ef1749b45b32c659afdaa43f43ad 
> 
> 
> Diff: https://reviews.apache.org/r/63813/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63814: Windows: Fixed `os::host_default_path()`.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63814/
> ---
> 
> (Updated Nov. 14, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the necessary and expected default paths to the default `PATH`
> value represented by this function. Especially needed was the
> `WindowsPowerShell` folder so that tasks using PowerShell can run.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63814/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63815: Windows: Fixed environment priorities in `shell.hpp`.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




Nice fix, that would have been annoying otherwise.

- John Kordich


On Nov. 14, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63815/
> ---
> 
> (Updated Nov. 14, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Legacy code had caused the system environment to explicitly override the
> supplied environment, but this is incorrect. The system environment
> should be the default, with any supplied environment variables
> overriding those.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 5c711837a3adbc89ad8793666e68eff7102566b3 
> 
> 
> Diff: https://reviews.apache.org/r/63815/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63816: Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.

2017-11-15 Thread John Kordich via Review Board

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


Ship it!




- John Kordich


On Nov. 14, 2017, 11:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63816/
> ---
> 
> (Updated Nov. 14, 2017, 11:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
> Michael Park.
> 
> 
> Bugs: MESOS-6816
> https://issues.apache.org/jira/browse/MESOS-6816
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By removing the explicit system environment override code, and instead
> setting the system environment as the default in the `CreateProcess`
> wrapper, the `SlaveTest.ExecutorEnvironmentVariables` can be enabled.
> Note that this also required fixing `os::host_default_path()` and using
> it, because the test used `PATH = /bin`, which breaks on Windows.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/launch.cpp 
> b1584ff292ada5463917792908a13e00859fd1ae 
>   src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 
> 
> 
> Diff: https://reviews.apache.org/r/63816/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Review Request 64102: Renamed curl target to libcurl, and staging of curl.exe on Windows.

2017-11-27 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


Repository: mesos


Description
---

Renamed curl target to libcurl, and staging of curl.exe on Windows.


Diffs
-

  3rdparty/CMakeLists.txt c34b65d5ff306e17c3519667ae303a8678fca43d 


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


Testing
---

Ran all tests on Windows/Linux (for this patch plus #64103 and #64104). No 
related tests failed on either Linux or Windows.


Thanks,

John Kordich



Review Request 64104: Added dependency of curl to agent. Enabled most health check tests.

2017-11-27 Thread John Kordich via Review Board

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

Review request for mesos.


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


Repository: mesos


Description
---

Added dependency of curl to agent. Enabled most health check tests.


Diffs
-

  src/slave/CMakeLists.txt 6f08f3dc95f3a1408cbef7b8f0b0cc2522558924 
  src/tests/health_check_tests.cpp c0dcba265363f2149b217b189ee5a8e925e40ea1 


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


Testing
---

There are two health check tests which are not enabled yet, because they 
require the IOSwitchboard to be ported first. We can't resolve MESOS-6709 quite 
yet until that is addressed. The rest of the tests pass on both Linux/Windows, 
as mentioned in #64102


Thanks,

John Kordich



Review Request 64103: Changed dependency of curl to libcurl for stout.

2017-11-27 Thread John Kordich via Review Board

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

Review request for mesos.


Repository: mesos


Description
---

Changed dependency of curl to libcurl for stout.


Diffs
-

  3rdparty/stout/CMakeLists.txt 145b772a4f5da5653b918f746fbfc44e58321e47 


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


Testing
---


Thanks,

John Kordich



Re: Review Request 64103: Changed dependency of curl to libcurl for stout.

2017-12-04 Thread John Kordich via Review Board

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

(Updated Dec. 4, 2017, 6:54 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Rebased on master


Repository: mesos


Description
---

Changed dependency of curl to libcurl for stout.


Diffs (updated)
-

  3rdparty/stout/CMakeLists.txt 145b772a4f5da5653b918f746fbfc44e58321e47 


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

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


Testing
---


Thanks,

John Kordich



Re: Review Request 64102: Renamed curl target to libcurl, and staging of curl.exe on Windows.

2017-12-04 Thread John Kordich via Review Board

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

(Updated Dec. 4, 2017, 6:54 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Rebased on master


Repository: mesos


Description
---

Renamed curl target to libcurl, and staging of curl.exe on Windows.


Diffs (updated)
-

  3rdparty/CMakeLists.txt c34b65d5ff306e17c3519667ae303a8678fca43d 


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

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


Testing
---

Ran all tests on Windows/Linux (for this patch plus #64103 and #64104). No 
related tests failed on either Linux or Windows.


Thanks,

John Kordich



Re: Review Request 64104: Added dependency of curl to agent. Enabled most health check tests.

2017-12-04 Thread John Kordich via Review Board

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

(Updated Dec. 4, 2017, 6:54 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Rebased on master and addressed comments.


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


Repository: mesos


Description
---

Added dependency of curl to agent. Enabled most health check tests.


Diffs (updated)
-

  src/slave/CMakeLists.txt 6f08f3dc95f3a1408cbef7b8f0b0cc2522558924 
  src/tests/health_check_tests.cpp 56721fac4a464f3cad40dbf4e6bc4fb7b1a780d4 


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

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


Testing
---

There are two health check tests which are not enabled yet, because they 
require the IOSwitchboard to be ported first. We can't resolve MESOS-6709 quite 
yet until that is addressed. The rest of the tests pass on both Linux/Windows, 
as mentioned in #64102


Thanks,

John Kordich



Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

2017-03-21 Thread John Kordich via Review Board

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

Review request for mesos.


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


Repository: mesos


Description
---

Filtered stout tests with symlinks when unable to create symlinks.


Diffs
-

  3rdparty/stout/Makefile.am ebf1069eb1b787f063a2066a4db0b3f5de4a56da 
  3rdparty/stout/tests/CMakeLists.txt 4bbe713f259e7858d423dcb33956d41e62a915eb 
  3rdparty/stout/tests/environment.hpp PRE-CREATION 
  3rdparty/stout/tests/environment.cpp PRE-CREATION 
  3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
  3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 


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


Testing
---


Thanks,

John Kordich



Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

2017-03-21 Thread John Kordich via Review Board

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



I'm still learning how review board works, so please let me know if I posted 
this incorrectly. Not sure if this is where comments go. There's another 
environment.cpp file in mesos-tests, but I was informed that I can't abstract 
something out that both mesos-tests and stout-tests relies on.  The 
environment.cpp file in this review for stout-tests is basically a watered down 
version of the environment.cpp file in mesos-tests.  This review checks if the 
user on Windows is able to create a symlink, and if so, enables tests that 
require the creation of symlinks. Otherwise, if unable to create a symlink, the 
tests that require the creation of symlinks are disabled.

- John Kordich


On March 21, 2017, 11:44 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57824/
> ---
> 
> (Updated March 21, 2017, 11:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-6731
> https://issues.apache.org/jira/browse/MESOS-6731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Filtered stout tests with symlinks when unable to create symlinks.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am ebf1069eb1b787f063a2066a4db0b3f5de4a56da 
>   3rdparty/stout/tests/CMakeLists.txt 
> 4bbe713f259e7858d423dcb33956d41e62a915eb 
>   3rdparty/stout/tests/environment.hpp PRE-CREATION 
>   3rdparty/stout/tests/environment.cpp PRE-CREATION 
>   3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
>   3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 
> 
> 
> Diff: https://reviews.apache.org/r/57824/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 57857: Fix post-reviews.py to work on Windows.

2017-03-22 Thread John Kordich via Review Board

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



Nice change, I will appreciate being able to post reviews from Windows. Looks 
like a relatively simple change, and I'd normally approve but I don't have the 
experience or knowledge yet to do so.

- John Kordich


On March 22, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57857/
> ---
> 
> (Updated March 22, 2017, 6:11 p.m.)
> 
> 
> Review request for mesos, John Kordich and Joseph Wu.
> 
> 
> Bugs: MESOS-7287
> https://issues.apache.org/jira/browse/MESOS-7287
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows cannot find `rbt` because the command name is `rbt.cmd`;
> the extension is required.
> 
> Because Windows (PowerShell and cmd) does not support shebangs,
> the usage of this script on Windows is:
> 
> python support\post-reviews.py
> 
> Windows does not have the `cat` command, so replace its usage
> with `git rev-parse --verify `.
> 
> Note that colors do not always work on Windows, but can work,
> so we leave them enabled.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py e4620e893a77dc2442b63b6d9c49626d82860d8d 
> 
> 
> Diff: https://reviews.apache.org/r/57857/diff/1/
> 
> 
> Testing
> ---
> 
> Posted this patch.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

2017-03-27 Thread John Kordich via Review Board

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

(Updated March 27, 2017, 10:13 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Filtered stout tests with symlinks when unable to create symlinks.


Diffs (updated)
-

  3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
  3rdparty/stout/tests/environment.hpp PRE-CREATION 
  3rdparty/stout/tests/environment.cpp PRE-CREATION 
  3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
  3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 


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

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


Testing (updated)
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Review Request 57971: Added test filtering framework to libprocess-tests.

2017-03-27 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added test filtering framework to libprocess-tests.


Diffs
-

  3rdparty/libprocess/src/tests/main.cpp 
cda7d61e1f10fde8a457c6077d4eb7aa78b390fc 


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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Review Request 57972: Added base stout Environment class to mesos-tests Environment class.

2017-03-27 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added base stout Environment class to mesos-tests Environment class.


Diffs
-

  src/tests/environment.hpp 06b6d54fc76b327e3e26914eb61c16619a36de42 
  src/tests/main.cpp 5d062c3451bdfb5d5fc459ac7c071ab18e6d8043 


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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

2017-04-05 Thread John Kordich via Review Board

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

(Updated April 5, 2017, 7:08 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


Changes
---

I'm not sure if I should leave it to the reviewer to check "Fixed" or "Drop", 
or if I should do it! In any case, I believe I've addressed all of the comments 
to the previous iteration.


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


Repository: mesos


Description
---

Filtered stout tests with symlinks when unable to create symlinks.


Diffs (updated)
-

  3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
  3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
  3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57971: Added test filtering framework to libprocess-tests.

2017-04-05 Thread John Kordich via Review Board

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

(Updated April 5, 2017, 7:08 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added test filtering framework to libprocess-tests.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/main.cpp 
cda7d61e1f10fde8a457c6077d4eb7aa78b390fc 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57972: Added base stout Environment class to mesos-tests Environment class.

2017-04-05 Thread John Kordich via Review Board

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

(Updated April 5, 2017, 7:08 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added base stout Environment class to mesos-tests Environment class.


Diffs (updated)
-

  src/tests/environment.hpp 06b6d54fc76b327e3e26914eb61c16619a36de42 
  src/tests/environment.cpp e3cff1847c44bb06bbe898211bfca35cf851217a 
  src/tests/main.cpp 5d062c3451bdfb5d5fc459ac7c071ab18e6d8043 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

2017-04-10 Thread John Kordich via Review Board

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

(Updated April 10, 2017, 9:41 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Filtered stout tests with symlinks when unable to create symlinks.


Diffs (updated)
-

  3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
  3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
  3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57972: Added base stout Environment class to mesos-tests Environment class.

2017-04-10 Thread John Kordich via Review Board

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

(Updated April 10, 2017, 9:41 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added base stout Environment class to mesos-tests Environment class.


Diffs (updated)
-

  src/tests/environment.hpp 06b6d54fc76b327e3e26914eb61c16619a36de42 
  src/tests/environment.cpp e3cff1847c44bb06bbe898211bfca35cf851217a 
  src/tests/main.cpp 5d062c3451bdfb5d5fc459ac7c071ab18e6d8043 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Re: Review Request 57971: Added test filtering framework to libprocess-tests.

2017-04-10 Thread John Kordich via Review Board

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

(Updated April 10, 2017, 9:41 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


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


Repository: mesos


Description
---

Added test filtering framework to libprocess-tests.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/main.cpp 
cda7d61e1f10fde8a457c6077d4eb7aa78b390fc 


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

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


Testing
---

Ran make check on Linux and the tests on Windows.


Thanks,

John Kordich



Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

2017-04-13 Thread John Kordich via Review Board

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

Review request for mesos.


Repository: mesos


Description
---

Changed some failing Base64 tests to use 'u8' string literals.


Diffs
-

  3rdparty/stout/tests/base64_tests.cpp 
0473221abb352195e03ae970709eadcfc44d77b0 


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


Testing
---

Ran make check on Linux and ran stout-tests on Windows.


Thanks,

John Kordich



Re: Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

2017-04-13 Thread John Kordich via Review Board

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

(Updated April 13, 2017, 6:48 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


Changes
---

These changes will fix the unit test failures on Windows by making sure that 
the string literals are encoded into bytes identically across platforms.


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


Repository: mesos


Description
---

Changed some failing Base64 tests to use 'u8' string literals.


Diffs
-

  3rdparty/stout/tests/base64_tests.cpp 
0473221abb352195e03ae970709eadcfc44d77b0 


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


Testing
---

Ran make check on Linux and ran stout-tests on Windows.


Thanks,

John Kordich



Review Request 59353: Enabled DOCKER and ROOT filter flags on Windows.

2017-05-17 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Joseph Wu, and Li 
Li.


Repository: mesos


Description
---

Enabled DOCKER and ROOT filter flags on Windows.


Diffs
-

  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/docker_tests.cpp 
3d3c9afbe81ebdcdf7c5542c56f9ca0e17e2484d 
  src/tests/default_executor_tests.cpp 22af7e973f8e6ca583c3126a80bc092bf88fea33 
  src/tests/environment.cpp 047798c781707c42641baa9473177ae25a451989 
  src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
  src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
  src/tests/slave_tests.cpp 8f81f77aa29d16616d5a38197729baab0fb0cab5 


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


Testing
---

Tested on both Windows and Linux.  Everything passes on Linux, but there are a 
number of tests that are failing on Windows. The tests that are failing on 
Windows are related to the tests that are currently failing on master on 
Windows. These tests are failing due to some long path changes in the Windows 
creators update, and I believe Andy is working on fixes to them.


Thanks,

John Kordich



Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-05 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.


Diffs
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
  3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
  3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
faf94909f995f7486b5f9cb7532af58a90a9eed3 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
ee5ea3dee3be197e923be544aab96806f0adf1cf 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
76cc007963b4cbe162556d03d3e41a0a5e660167 
  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
2bc9b0890d384e9ce8d5d4679c974c5e88231eed 


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


Testing
---


Thanks,

John Kordich



Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-05 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

Enabled CRAM MD5 Authentication on Windows and associated tests.


Diffs
-

  cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
  docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/authentication/cram_md5/authenticatee.cpp 
7b3f767e29163de489018081089e67a6f22971c5 
  src/authentication/cram_md5/authenticator.cpp 
2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
  src/authentication/cram_md5/auxprop.hpp 
dedcbe514283dd86a924d92ccdd17173ebe878cb 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 


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


Testing
---

Ran the automake build and the cmake build on Linux, as well as the cmake build 
on Windows, with no errors.  Ran tests on both platforms as well.  Note: The 
mesos-3rdparty pull request which contains the cyrus-sasl package needs to be 
merged to master first before these patches will work. (Or, have the cyrus-sasl 
tarball available in a local 3rdparty directory, configured with mesos)


Thanks,

John Kordich



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-06 Thread John Kordich via Review Board

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




3rdparty/CMakeLists.txt
Lines 208 (patched)


This needs to change, but I'm not sure how to use cmake appropriately here. 
 We should be able to inherit the configure command from the main mesos build, 
I think.  How would I do this? Maybe an "add_subdirectory"?


- John Kordich


On Sept. 6, 2017, 12:05 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62105/
> ---
> 
> (Updated Sept. 6, 2017, 12:05 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
>   3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
>   3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> faf94909f995f7486b5f9cb7532af58a90a9eed3 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> ee5ea3dee3be197e923be544aab96806f0adf1cf 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 76cc007963b4cbe162556d03d3e41a0a5e660167 
>   src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
> 2bc9b0890d384e9ce8d5d4679c974c5e88231eed 
> 
> 
> Diff: https://reviews.apache.org/r/62105/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-06 Thread John Kordich via Review Board

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

(Updated Sept. 6, 2017, 8:35 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Rebased on master


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


Repository: mesos


Description
---

Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
  3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
  3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 


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

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


Testing
---


Thanks,

John Kordich



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-06 Thread John Kordich via Review Board

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

(Updated Sept. 6, 2017, 8:35 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Rebased on master


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


Repository: mesos


Description
---

Enabled CRAM MD5 Authentication on Windows and associated tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
  docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/authentication/cram_md5/authenticatee.cpp 
7b3f767e29163de489018081089e67a6f22971c5 
  src/authentication/cram_md5/authenticator.cpp 
2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
  src/authentication/cram_md5/auxprop.hpp 
dedcbe514283dd86a924d92ccdd17173ebe878cb 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 


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

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


Testing
---

Ran the automake build and the cmake build on Linux, as well as the cmake build 
on Windows, with no errors.  Ran tests on both platforms as well.  Note: The 
mesos-3rdparty pull request which contains the cyrus-sasl package needs to be 
merged to master first before these patches will work. (Or, have the cyrus-sasl 
tarball available in a local 3rdparty directory, configured with mesos)


Thanks,

John Kordich



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-06 Thread John Kordich via Review Board

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

(Updated Sept. 7, 2017, 1:11 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Enabled CRAM MD5 Authentication on Windows and associated tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
  docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/authentication/cram_md5/authenticatee.cpp 
7b3f767e29163de489018081089e67a6f22971c5 
  src/authentication/cram_md5/authenticator.cpp 
2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
  src/authentication/cram_md5/auxprop.hpp 
dedcbe514283dd86a924d92ccdd17173ebe878cb 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
  support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 


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

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


Testing
---

Ran the automake build and the cmake build on Linux, as well as the cmake build 
on Windows, with no errors.  Ran tests on both platforms as well.  Note: The 
mesos-3rdparty pull request which contains the cyrus-sasl package needs to be 
merged to master first before these patches will work. (Or, have the cyrus-sasl 
tarball available in a local 3rdparty directory, configured with mesos)


Thanks,

John Kordich



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-06 Thread John Kordich via Review Board

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

(Updated Sept. 7, 2017, 1:11 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
  3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
  3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 


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

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


Testing
---


Thanks,

John Kordich



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-06 Thread John Kordich via Review Board


> On Sept. 6, 2017, 8:48 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/cyrus_sasl-2.1.27rc3.patch
> > Lines 1 (patched)
> > 
> >
> > Since it's a patch, I won't review it too much. However, we should 
> > ensure we submit this change upstream to libsasl2, and at that point clean 
> > it up a bit.

Unfortunately, they will not take this patch.  I had to make some changes to 
make libsasl2 build statically, and enabled it to build only one of the 
modules.  In order to build many of the authentication modules, many different 
development headers need to be installed on the system building those modules, 
which we don't have right now.  I could spend more time and make the build  
configurable so only modules which have headers would be built, but I think 
that's not within the scope of what I'm working on right now. But if Li thinks 
this is important, I'm fine with doing it.


- John


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


On Sept. 7, 2017, 1:11 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62105/
> ---
> 
> (Updated Sept. 7, 2017, 1:11 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
> https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
>   3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
>   3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62105/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-07 Thread John Kordich via Review Board

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

Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


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


Repository: mesos


Description
---

Added cmake dependency check for libsasl2 on non-Windows platforms.


Diffs
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 


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


Testing
---

I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
exist, it fails the cmake configure/build.  I did not test this on any other 
platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
Windows build.  I'm uncertain if there is much support for other kinds of 
builds (like Mac OS), but this should be platform independent.


Thanks,

John Kordich



Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.

2017-09-08 Thread John Kordich via Review Board

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

(Updated Sept. 8, 2017, 10:13 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Addressed new comments


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


Repository: mesos


Description
---

Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
  3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
  3rdparty/cyrus-sasl-2.1.27rc3.patch PRE-CREATION 


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

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


Testing
---


Thanks,

John Kordich



Re: Review Request 62106: Enabled CRAM MD5 Authentication on Windows and associated tests.

2017-09-08 Thread John Kordich via Review Board

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

(Updated Sept. 8, 2017, 10:13 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Addressed new comments


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


Repository: mesos


Description
---

Enabled CRAM MD5 Authentication on Windows and associated tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 4cb8dddf93694722af2377c4a55a195260c8a0cb 
  docs/configuration-cmake.md d2eb571c9458c7fade415e8077c676dd34e63242 
  docs/windows.md b47ba5a0db3d41d6f4bb002283b40622a2bb3e0f 
  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am a4f2ee008eba263fe52c4e58c305e3fb9ba42f6b 
  src/authentication/cram_md5/authenticatee.cpp 
7b3f767e29163de489018081089e67a6f22971c5 
  src/authentication/cram_md5/authenticator.cpp 
2a12bb3f4aec79056ec6c82a8c896ee3af15bb4c 
  src/authentication/cram_md5/auxprop.hpp 
dedcbe514283dd86a924d92ccdd17173ebe878cb 
  src/master/master.cpp 14d94cfa0e96881f77dd6e0c3449fdc0613e36ce 
  src/sched/sched.cpp ef73c1dccfd736b79f40a057951f022df7f60644 
  src/slave/slave.cpp 6d1516a5d5b5db684f79385e60d892ff75fd00fd 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/authentication_tests.cpp b04be76d23788fbde337eaf85716ec7436ade691 
  src/tests/mesos.cpp fc7f8cb09e223000ea160aa4ab83a003cf4af2fb 
  support/windows-build.bat 100013ed68ecc2f431c20a3f081f8aa6eca961ef 


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

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


Testing
---

Ran the automake build and the cmake build on Linux, as well as the cmake build 
on Windows, with no errors.  Ran tests on both platforms as well.  Note: The 
mesos-3rdparty pull request which contains the cyrus-sasl package needs to be 
merged to master first before these patches will work. (Or, have the cyrus-sasl 
tarball available in a local 3rdparty directory, configured with mesos)


Thanks,

John Kordich



Re: Review Request 62176: Added cmake dependency check for libsasl2 on non-Windows platforms.

2017-09-08 Thread John Kordich via Review Board

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

(Updated Sept. 8, 2017, 10:35 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.


Changes
---

Addressed suggested comments


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


Repository: mesos


Description
---

Added cmake dependency check for libsasl2 on non-Windows platforms.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 


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

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


Testing
---

I tested this on my Ubuntu 16.04 system. When the libsasl2 library doesn't 
exist, it fails the cmake configure/build.  I did not test this on any other 
platform, but this code is in a "if (NOT WIN32)" block and won't affect a 
Windows build.  I'm uncertain if there is much support for other kinds of 
builds (like Mac OS), but this should be platform independent.


Thanks,

John Kordich



Review Request 65936: Changed flags to CreateFile to support Windows symlink path resolution.

2018-03-07 Thread John Kordich via Review Board

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Jeff Coffler.


Repository: mesos


Description
---

Changed flags to CreateFile to support Windows symlink path resolution.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
858b3c7c27e95b9cf46a7457c1aaa05a0e19188d 


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


Testing
---

I encountered a permissions error stating "The process cannot access the file 
because it is being used by another process." when attempting to download 
stderr/stdout from a task running on a Windows agent, or when attempting to 
download the mesos agent log on a Windows agent. 

After some very helpful information from Andy and Akash, I made this change and 
confirmed it fixed this issue with the task/agent logs.

I also ran the automated tests on Windows and all tests pass.

I did not test this on Linux, but because this file is compiled only on 
Windows, it shouldn't be necessary.


Thanks,

John Kordich



Re: Review Request 65936: Changed flags to CreateFile to support Windows symlink path resolution.

2018-03-07 Thread John Kordich via Review Board

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

(Updated March 7, 2018, 10:24 a.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Jeff Coffler.


Changes
---

Added bug.


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


Repository: mesos


Description
---

Changed flags to CreateFile to support Windows symlink path resolution.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
858b3c7c27e95b9cf46a7457c1aaa05a0e19188d 


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


Testing
---

I encountered a permissions error stating "The process cannot access the file 
because it is being used by another process." when attempting to download 
stderr/stdout from a task running on a Windows agent, or when attempting to 
download the mesos agent log on a Windows agent. 

After some very helpful information from Andy and Akash, I made this change and 
confirmed it fixed this issue with the task/agent logs.

I also ran the automated tests on Windows and all tests pass.

I did not test this on Linux, but because this file is compiled only on 
Windows, it shouldn't be necessary.


Thanks,

John Kordich



Re: Review Request 65936: Changed flags to CreateFile to support Windows symlink path resolution.

2018-03-08 Thread John Kordich via Review Board

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

(Updated March 8, 2018, 7:22 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Addressed comments and added regression test


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


Repository: mesos


Description
---

Changed flags to CreateFile to support Windows symlink path resolution.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
858b3c7c27e95b9cf46a7457c1aaa05a0e19188d 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
b84f1ae17246d94947538efeaf504a2cd247f20e 


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

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


Testing
---

I encountered a permissions error stating "The process cannot access the file 
because it is being used by another process." when attempting to download 
stderr/stdout from a task running on a Windows agent, or when attempting to 
download the mesos agent log on a Windows agent. 

After some very helpful information from Andy and Akash, I made this change and 
confirmed it fixed this issue with the task/agent logs.

I also ran the automated tests on Windows and all tests pass.

I did not test this on Linux, but because this file is compiled only on 
Windows, it shouldn't be necessary.


Thanks,

John Kordich



Re: Review Request 65936: Changed flags to CreateFile to support Windows symlink path resolution.

2018-03-08 Thread John Kordich via Review Board

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

(Updated March 8, 2018, 11:21 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Addressed comments


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


Repository: mesos


Description
---

Changed flags to CreateFile to support Windows symlink path resolution.


Diffs (updated)
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
858b3c7c27e95b9cf46a7457c1aaa05a0e19188d 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
b84f1ae17246d94947538efeaf504a2cd247f20e 


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

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


Testing
---

I encountered a permissions error stating "The process cannot access the file 
because it is being used by another process." when attempting to download 
stderr/stdout from a task running on a Windows agent, or when attempting to 
download the mesos agent log on a Windows agent. 

After some very helpful information from Andy and Akash, I made this change and 
confirmed it fixed this issue with the task/agent logs.

I also ran the automated tests on Windows and all tests pass.

I did not test this on Linux, but because this file is compiled only on 
Windows, it shouldn't be necessary.


Thanks,

John Kordich



Review Request 66002: Fixed the HTTP API path variables on Windows to use proper seperators.

2018-03-09 Thread John Kordich via Review Board

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Repository: mesos


Description
---

Fixed the HTTP API path variables on Windows to use proper seperators.


Diffs
-

  src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
  src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 


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


Testing
---

Tested on both Windows and Linux, all tests pass, including the two newly 
enabled tests on Windows.


Thanks,

John Kordich



Re: Review Request 66002: Fixed the HTTP API path variables on Windows to use proper seperators.

2018-03-09 Thread John Kordich via Review Board

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




src/tests/files_tests.cpp
Line 338 (original), 338 (patched)


Regarding this use of c_str():  I'm fairly certain this won't deallocate 
the result of path::join("1", "2") until after the call to stat() is complete, 
so the string should still be valid.  However, I'm not 100% certain, and I'd 
like someone else to judge this as well.

To be safe, I could allocate these paths outside of this function call, to 
guarantee that this string won't be deallocated.


- John Kordich


On March 9, 2018, 7:27 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> ---
> 
> (Updated March 9, 2018, 7:27 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, 
> and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the HTTP API path variables on Windows to use proper seperators.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
>   src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 
> 
> 
> Diff: https://reviews.apache.org/r/66002/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux, all tests pass, including the two newly 
> enabled tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 66002: Fixed the HTTP API path variables on Windows.

2018-03-09 Thread John Kordich via Review Board

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

(Updated March 9, 2018, 8:17 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Expanded commit message


Summary (updated)
-

Fixed the HTTP API path variables on Windows.


Repository: mesos


Description (updated)
---

Many mesos frameworks assume that path separators are forward slashes,
like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
this patch, if a forward slash was given in the path variable to an HTTP
API function call to the mesos agent on a Windows system, the Windows
API would fail at recognizing the path, because the Windows API accepts
only backslashes as path separators. To remedy this issue, we now
convert all forward slashes passed as a path to the HTTP API to an agent
to back slashes on Windows agents by using the path::from_uri function.


Diffs (updated)
-

  src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
  src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 


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

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


Testing
---

Tested on both Windows and Linux, all tests pass, including the two newly 
enabled tests on Windows.


Thanks,

John Kordich



Re: Review Request 66014: Windows: Made SASL use default CRT linking.

2018-03-09 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66014/
> ---
> 
> (Updated March 9, 2018, 10:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Made SASL use default CRT linking.
> 
> 
> Diffs
> -
> 
>   3rdparty/cyrus-sasl-2.1.27rc3.patch 
> e363b9e92452a3558f5341faf1653514d85364d4 
> 
> 
> Diff: https://reviews.apache.org/r/66014/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66007: CMake: Set C++11 as standard automatically.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66007/
> ---
> 
> (Updated March 9, 2018, 10:37 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8033
> https://issues.apache.org/jira/browse/MESOS-8033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of setting the compiler option manually, we use the
> `CMAKE_CXX_STANDARD` variable to set the default for all targets. This
> automatically appends the correct flag for each compiler.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
> 
> 
> Diff: https://reviews.apache.org/r/66007/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66009: CMake: Added `-Wno-unused-local-typedefs` to Boost interface.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66009/
> ---
> 
> (Updated March 9, 2018, 10:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As found in the Autotools builds, GCC and Clang will emit an used
> local typedef warning for headers including Boost. Since it is
> 3rdparty code, we disable this warning in its interface.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66009/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66010: Windows: Switched to default CRT linkage.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66010/
> ---
> 
> (Updated March 9, 2018, 10:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8659
> https://issues.apache.org/jira/browse/MESOS-8659
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We previously attempted to manually override the CRT to be static
> everywhere. Not only did this emit warnings, it was also error-prone
> and unnecessary. We can, and should, just use the defaults, which is
> `/MDd` in debug mode (multi-threaded, dynamic, debug linkage). Linking
> to the CRT dynamically results in smaller libraries and executables,
> reduces linking time, and avoids bugs when sharing allocated memory
> across modules.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
> 
> 
> Diff: https://reviews.apache.org/r/66010/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66013: Windows: Made ZooKeeper use default CRT linking.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66013/
> ---
> 
> (Updated March 9, 2018, 10:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8659
> https://issues.apache.org/jira/browse/MESOS-8659
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also deleted extraneous patch files.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
>   3rdparty/zookeeper-06d3f3f.patch 7f66875c7ef8bea3cc077d064b90e056c706487f 
>   3rdparty/zookeeper-3.4.8.patch c532a696f8bf3373b7341ddf32dcf9c528ee4412 
>   3rdparty/zookeeper-3.5.2-alpha.patch 
> 6d5db536437b22a2903c1fa3348613e12fd35df0 
> 
> 
> Diff: https://reviews.apache.org/r/66013/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66011: Windows: Set 3rdparty libraries to link to CRT dynamically.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66011/
> ---
> 
> (Updated March 9, 2018, 10:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8659
> https://issues.apache.org/jira/browse/MESOS-8659
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This also fixes a bug where the `libevent` build on Windows would add
> `''` to the compiler flags, which is an unrecognized flag, and sets
> the names of our `CMAKE_FORWARD_ARGS` variables consistently (some
> were missing the `FORWARD` part).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66011/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66012: CMake: Split `CMAKE_FORWARD_ARGS` into `C` and `CXX` versions.

2018-03-12 Thread John Kordich via Review Board

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


Ship it!




I'm fine with these changes, but the description can be improved a bit I think. 
I know that this change relates to some of the later patches, so maybe mention 
that several of the fixes are built on top of this one (I guess that's implied, 
though).  Maybe mention that this allows for granularity between C/CPP cmake 
flags, even though it's rather clear from the code.

- John Kordich


On March 9, 2018, 10:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66012/
> ---
> 
> (Updated March 9, 2018, 10:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By passing `CMAKE_C_FORWARD_ARGS` to C libraries, e.g. curl, libapr,
> libevent, and zlib, we avoid CMake warning us about set but unused
> variables (since they do not use the `CXX` variables).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 61dc788edf96ca6e8e3ee736eb232c0c118e6191 
> 
> 
> Diff: https://reviews.apache.org/r/66012/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66008: CMake: Enabled compiler warnings.

2018-03-13 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On March 9, 2018, 10:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66008/
> ---
> 
> (Updated March 9, 2018, 10:38 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-8658
> https://issues.apache.org/jira/browse/MESOS-8658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We had previously been using the default sets of warnings, but now we
> use the same warnings as on Autotools. This also means that we can
> safely turn on the treat-warnings-as-errors (at least for the Mesos
> code). However, doing so requires that we disable two common
> possible-loss-of-data warnings on Windows that are not part of the
> GNU/Clang default warnings.
> 
> This also replaces the use of `string(APPEND CMAKE_CXX_FLAGS)` with
> the canonical command `add_compile_options` (though setting these on a
> per-target basis would be even better, it's a separate issue).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake efee36c1ffda096a97af23d481fc0d0903124e54 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
> 
> 
> Diff: https://reviews.apache.org/r/66008/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66002: Fixed the HTTP API path variables on Windows.

2018-03-19 Thread John Kordich via Review Board

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

(Updated March 19, 2018, 6:20 p.m.)


Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and 
Joseph Wu.


Changes
---

Addressed comments


Repository: mesos


Description (updated)
---

Many mesos frameworks assume that path separators are forward slashes,
like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
this patch, if a forward slash was given in the path variable to an HTTP
API function call to the mesos agent on a Windows system, the Windows
API would fail at recognizing the path, because the Windows API accepts
only backslashes as path separators. To remedy this issue, we will now
convert all forward slashes passed as a path to the HTTP API to back
slashes on Windows agents by using the path::from_uri function.


Diffs (updated)
-

  src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078 
  src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469 


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

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


Testing
---

Tested on both Windows and Linux, all tests pass, including the two newly 
enabled tests on Windows.


Thanks,

John Kordich



Re: Review Request 66421: Windows: Included used `jobobject.hpp` stout header in libprocess.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:47 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66421/
> ---
> 
> (Updated April 4, 2018, 5:47 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The job object functions were refactored upstream from
> `windows/os.hpp` to `os/windows/jobobject.hpp`.
> 
> Also removed `windows/os.hpp` because `os.hpp` includes it for us.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/windows/jobobject.hpp 
> 0de374d374804c2d4f06a352d80062d3b42ed9b2 
> 
> 
> Diff: https://reviews.apache.org/r/66421/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66422: Windows: Included used `jobobject.hpp` stout header in Mesos.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:47 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66422/
> ---
> 
> (Updated April 4, 2018, 5:47 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The job object functions were refactored upstream from
> `windows/os.hpp` to `os/windows/jobobject.hpp`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 383655230fcdad30b652ea984cb9f5fc9c49dd38 
>   src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
> 69b0ff8ab1c391c8771954de64a17b6ad1c5c10c 
>   src/slave/containerizer/mesos/isolators/windows/mem.cpp 
> 6cc65b942fd7d11d2ea35eab0225427ffdd9d1ee 
>   src/slave/containerizer/mesos/launch.cpp 
> 8c739ccd39675d5417437609a31f5a21784535b7 
> 
> 
> Diff: https://reviews.apache.org/r/66422/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66426: Windows: More constness in stout.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:48 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66426/
> ---
> 
> (Updated April 4, 2018, 5:48 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also small fixes such as `reserve` over an allocation, and a bad name
> `si` instead of `info`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/getcwd.hpp 
> f316d618226872b57d950b468352176a7a0cb45a 
>   3rdparty/stout/include/stout/os/windows/getenv.hpp 
> 58012e03aca5dfa2f65fc183b21533dd0ed91d8d 
>   3rdparty/stout/include/stout/os/windows/jobobject.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> ce8bdcd18305ffb758f22a6c2bbc7393675aebdf 
>   3rdparty/stout/include/stout/os/windows/mkdir.hpp 
> 8d8d80bee77253086939c28333b0413bd8b8b8b6 
>   3rdparty/stout/include/stout/os/windows/pagesize.hpp 
> ddf23c1c5d15c1dd7de37e98673b70836a0e2c5c 
>   3rdparty/stout/include/stout/os/windows/realpath.hpp 
> c6bad5063c8d8255b29a3a6cb9ea51e13c42275c 
>   3rdparty/stout/include/stout/os/windows/su.hpp 
> 1bfbb261edbd25b4552742fc0597e331012cad98 
>   3rdparty/stout/include/stout/os/windows/temp.hpp 
> 9cf467fd9358cc4de702e0501263abcd28a0fa8c 
> 
> 
> Diff: https://reviews.apache.org/r/66426/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66436: Removed use of `fstat()` from `http.cpp` and `http_proxy.cpp`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:54 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66436/
> ---
> 
> (Updated April 4, 2018, 5:54 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `os::stat::size()` and `os::stat::isdir()` are now
> overloaded for an `int_fd` type, using `fstat()` on POSIX, and the
> equivalent functions with a `HANDLE` on Windows. This allowed us to
> remove the use of `::fstat()`, which was not abstracted, and not
> supported on Windows without the use of a CRT integer file descriptor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 63dd2c1f629ac316d0b31f8a854e482ae6eda634 
>   3rdparty/libprocess/src/http_proxy.cpp 
> 25d63791e4788a488f96303aabeed0fa77ad7992 
> 
> 
> Diff: https://reviews.apache.org/r/66436/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66438: Windows: Made `libevent` use CRT file descriptor explicitly.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 7:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66438/
> ---
> 
> (Updated April 4, 2018, 7:19 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is an edge case where a third-party library (libevent) requires a
> CRT integer file descriptor. Thus we duplicate the `int_fd` and then
> explicitly allocate via `crt()`, which requires that we also manually
> close it via `_close()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 4de161dbf9198e9c74b1e80838b8a5d52006a562 
> 
> 
> Diff: https://reviews.apache.org/r/66438/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:57 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66440/
> ---
> 
> (Updated April 4, 2018, 5:57 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `open()` with `os::open()` in `http_proxy.cpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http_proxy.cpp 
> 25d63791e4788a488f96303aabeed0fa77ad7992 
> 
> 
> Diff: https://reviews.apache.org/r/66440/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66441: Fixed mismatched types in `process.cpp`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:57 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66441/
> ---
> 
> (Updated April 4, 2018, 5:57 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because of the ternary, the primitive integer `-1` must be explicitly
> constructed into an `int_fd` to match the type held in the `sockets`
> container.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 
> 
> 
> Diff: https://reviews.apache.org/r/66441/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66442: Windows: Fixed `os::abort()` to use `WriteFile()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:57 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66442/
> ---
> 
> (Updated April 4, 2018, 5:57 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Fixed `os::abort()` to use `WriteFile()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/abort.hpp 
> 4fd233dfcd4359791dd176820f3a6040947bb291 
> 
> 
> Diff: https://reviews.apache.org/r/66442/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66444: Windows: Made `signals.hpp` compile.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66444/
> ---
> 
> (Updated April 4, 2018, 5:58 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This file had never been included before, so it didn't compile. It
> needed to include `unimplemented.hpp`, and because `Suppressor` isn't
> implemented, the initializers had to be deleted.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/signals.hpp 
> 0ed24771625e58c1de8b1aa96b70f5aae1638bd4 
> 
> 
> Diff: https://reviews.apache.org/r/66444/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66443: Fixed `Subprocess::ChildHook::CHDIR()` to use `os::chdir()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66443/
> ---
> 
> (Updated April 4, 2018, 5:58 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This needed to use the Stout API so that the correct Windows
> implementation is used, as `::chdir` is part of the CRT.
> 
> Also included used but not included `stout/os/*` headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 898326360d6b4f0a50d6ef3f7c86141d0aa70438 
> 
> 
> Diff: https://reviews.apache.org/r/66443/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66445: Windows: Cleaned up included CRT headers.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 7:26 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66445/
> ---
> 
> (Updated April 4, 2018, 7:26 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8759
> https://issues.apache.org/jira/browse/MESOS-8759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The set `errno` value in `os::kill()` is never checked (especially on
> Windows), so `_set_errno()` and thus `errno.h` were removed.
> 
> The `fcntl.h` is used only to provide `O_CREAT` etc., and so belonged
> in `open.hpp`, not `windows.hpp`.
> 
> The remaining headers, `direct.h`, `io.h`, `process.h`, and `stdlib.h`
> were no longer used or needed as the respective CRT APIs were
> replaced.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> 9cec1117fac3cf6bd624fc7db524ef1ad10cd55d 
>   3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
> 9181429383a991fe2b87701d2bfd0e858ac2537b 
>   3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> a2926dab2c8e219cf5938c4df27f83488198dd6b 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> fff5872db6b3f69464e41e6d108a107e4eeabd12 
>   3rdparty/stout/include/stout/windows.hpp 
> 1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 
>   3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
> 5b3cbf4f36ea9ac0411df52b4cfea8ef75fecbb5 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 739ee4da3f09d2a9597d4451e755e77903e9287d 
> 
> 
> Diff: https://reviews.apache.org/r/66445/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66420: Windows: Extracted job object code into `os/windows/jobobject.hpp`.

2018-04-04 Thread John Kordich via Review Board

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


Fix it, then Ship it!




Oops. I just realized as I was getting through this code review that it was 
just a code move into jobobject.hpp.  You can read my comment if you'd like. 
But I was going to sign off anyway :)


3rdparty/stout/include/stout/os/windows/jobobject.hpp
Lines 139 (patched)


This is interesting. I understand how you're using this template function 
to allocate this structure on the stack which you use a reinterpret_cast on 
later as a substitute for the JOBOBJECT_BASIC_PROCESS_ID_LIST.

But is this really worth doing?  Allocation and deallocation would happen 
entirely within this function. I imagine the data we need will be copied out 
before deallocation during the insert calls below on the set object, 
so the only real issue is the extra time associated with dynamic memory 
allocation/deallocation.

What's hairy about the size calculations? I imagine it's not that bad, 
probably nothing you wouldn't do normally in C :)

If you do end up staying with this structure, is there a reason the 
ProcessIdList member is a DWORD and not a ULONG_PTR, as per 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms684150(v=vs.85).aspx 
?

It's probably the case that on x86_64 that they are the same size, but why 
not mirror the structure identically?


- John Kordich


On April 4, 2018, 5:46 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66420/
> ---
> 
> (Updated April 4, 2018, 5:46 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions written to deal with job objects on Windows had become
> large enough to warrant being refactored into their own file. Also
> was the perfect opportunity to fix formatting issues.
> 
> When including `jobobject.hpp` for `killtree.hpp`, other unnecessary
> headers were removed.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
>   3rdparty/stout/include/stout/os/windows/jobobject.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> ce8bdcd18305ffb758f22a6c2bbc7393675aebdf 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 739ee4da3f09d2a9597d4451e755e77903e9287d 
> 
> 
> Diff: https://reviews.apache.org/r/66420/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66420: Windows: Extracted job object code into `os/windows/jobobject.hpp`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:46 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66420/
> ---
> 
> (Updated April 4, 2018, 5:46 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions written to deal with job objects on Windows had become
> large enough to warrant being refactored into their own file. Also
> was the perfect opportunity to fix formatting issues.
> 
> When including `jobobject.hpp` for `killtree.hpp`, other unnecessary
> headers were removed.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
>   3rdparty/stout/include/stout/os/windows/jobobject.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> ce8bdcd18305ffb758f22a6c2bbc7393675aebdf 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 739ee4da3f09d2a9597d4451e755e77903e9287d 
> 
> 
> Diff: https://reviews.apache.org/r/66420/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66423: Split `stout/os/open.hpp` into Windows and POSIX files.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:47 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66423/
> ---
> 
> (Updated April 4, 2018, 5:47 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8673
> https://issues.apache.org/jira/browse/MESOS-8673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The logic remained the same, just with the Windows code removed from
> the POSIX code, and vice versa.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
>   3rdparty/stout/include/stout/os/open.hpp 
> 4dc5b087abf40ac9b81f0fd611aca192e5d33ce7 
>   3rdparty/stout/include/stout/os/posix/open.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66423/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:47 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66424/
> ---
> 
> (Updated April 4, 2018, 5:47 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8673
> https://issues.apache.org/jira/browse/MESOS-8673
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using the CRT implementation of `_wopen()` for the
> `os::open()` API, we now use the Windows API `CreateFileW()`, mapping
> each of the Linux `open()` flags to their semantic equivalents. This
> will make implementing overlapped I/O possible, and is a step toward
> removing the use of integer file descriptors on Windows.
> 
> Note that instead of redefining the C flags like `O_RDONLY`, we just
> use them directly in our mapping logic, and set the used but
> unsupported flags to zero.
> 
> This change uncovered several bugs such as incorrect access flags, and
> used-but-not-included headers.
> 
> We currently ignore creation permissions as they will be handled in a
> broader project to map permissions to Windows correctly.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/net.hpp 
> d2992c05b221ea90dae1c06d27753932f7411925 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/mktemp.hpp 
> 5c775c45c415d9ddd6a80ab814fb55475e9f871e 
>   3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/windows.hpp 
> 1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 
> 
> 
> Diff: https://reviews.apache.org/r/66424/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66427: Split `stout/os/lseek.hpp` into Windows and POSIX files.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:48 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66427/
> ---
> 
> (Updated April 4, 2018, 5:48 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8685
> https://issues.apache.org/jira/browse/MESOS-8685
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split `stout/os/lseek.hpp` into Windows and POSIX files.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
>   3rdparty/stout/include/stout/os/lseek.hpp 
> 77fe272afc89f41836c2540de42135dc364917ce 
>   3rdparty/stout/include/stout/os/posix/lseek.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/lseek.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66427/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66425: Windows: Replaced `WindowsFD` with `int_fd` typedef.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:47 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66425/
> ---
> 
> (Updated April 4, 2018, 5:47 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The latter should be used everywhere but in the implementation for
> consistency with the POSIX side of the code.
> 
> Also meant fixing the included header (and spacing).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp 
> 6da6f8eb1382d226c8b16a8e4cbb454205ef4045 
>   3rdparty/stout/include/stout/os/windows/close.hpp 
> ff635e44235d63888a210cd68d49f6678a851e31 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/fsync.hpp 
> 8405247280b51e74a172317816096ca77fdfd1e7 
>   3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
> fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 
>   3rdparty/stout/include/stout/os/windows/pipe.hpp 
> 365db9480f6258a03ef2e760a19abef8ab177e58 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
>   3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> fff5872db6b3f69464e41e6d108a107e4eeabd12 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> aacd746922495f994891aa85d3e4fa95e2bd1c44 
>   3rdparty/stout/include/stout/os/windows/socket.hpp 
> 259b05b8c85e399feaccec698d58b7d540cad368 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 739ee4da3f09d2a9597d4451e755e77903e9287d 
> 
> 
> Diff: https://reviews.apache.org/r/66425/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66428: Windows: Fixed `os::lseek()` to use `SetFilePointerEx()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!





3rdparty/stout/include/stout/os/windows/lseek.hpp
Lines 36 (patched)


That sounds like it would be user error here if this ever happens!


- John Kordich


On April 4, 2018, 5:49 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66428/
> ---
> 
> (Updated April 4, 2018, 5:49 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8685
> https://issues.apache.org/jira/browse/MESOS-8685
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note the TODO, we may want to synchronize this code later.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/lseek.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66428/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66429: Windows: Deleted dead code from `process::internal` namespace.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 7:17 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66429/
> ---
> 
> (Updated April 4, 2018, 7:17 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The deleted code was purely self-referential.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess_windows.cpp 
> 1a91fbe7aeb44174ccfa2e7e299bc7dd52a11b8a 
> 
> 
> Diff: https://reviews.apache.org/r/66429/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66430: Windows: Fixed `os::dup()` to use `DuplicateHandle()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:50 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66430/
> ---
> 
> (Updated April 4, 2018, 5:50 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8684
> https://issues.apache.org/jira/browse/MESOS-8684
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note that for now we need to keep the original CRT code, as it can't
> be removed until `FD_CRT` is removed too.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
> 
> 
> Diff: https://reviews.apache.org/r/66430/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!





3rdparty/stout/include/stout/os/windows/ftruncate.hpp
Lines 36 (patched)


I'm fine with a truncate function not writing null bytes to the part of a 
file that was truncated. Truncate should be a fast operation, in my opinion :)


- John Kordich


On April 4, 2018, 7:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66431/
> ---
> 
> (Updated April 4, 2018, 7:13 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8676
> https://issues.apache.org/jira/browse/MESOS-8676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can eventually support overlapped I/O.
> 
> The Windows API `ReadFile()` returns an error if the pipe is broken,
> where `_read()` did not, but this is not an error for us as the data
> is still read correctly. So we ignore it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
> 
> 
> Diff: https://reviews.apache.org/r/66431/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66432: Windows: Fixed `os::write()` to use `WriteFile()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:50 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66432/
> ---
> 
> (Updated April 4, 2018, 5:50 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8676
> https://issues.apache.org/jira/browse/MESOS-8676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can eventually support overlapped I/O.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
> 
> 
> Diff: https://reviews.apache.org/r/66432/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66433: Windows: Made `net::download()` use CRT file descriptor explicitly.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 7:18 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66433/
> ---
> 
> (Updated April 4, 2018, 7:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is an edge case where a third-party library (libcurl) requires a
> CRT integer file descriptor. Thus we explicitly allocate one via
> `crt()`, which requires that we also manually close it via `_close()`,
> not `os::close()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/net.hpp 
> d2992c05b221ea90dae1c06d27753932f7411925 
> 
> 
> Diff: https://reviews.apache.org/r/66433/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66434: Windows: Refactored `subprocess_windows.cpp` to use `os::open()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 7:18 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66434/
> ---
> 
> (Updated April 4, 2018, 7:18 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `os::open()` used the CRT function `_wopen()`, and so this
> file was written to use the `CreateFile()` API directly. Now that
> `os::open()` uses the Windows API, all this duplicate code can be
> deleted in favor of using the `os::open()` and
> `internal::windows::set_inherit()`. The major benefit here is that the
> logic now almost exactly matches the POSIX counterpart in
> `subprocess_posix.cpp`, to the point that we may want to recombine
> these files in the future.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess_windows.cpp 
> 1a91fbe7aeb44174ccfa2e7e299bc7dd52a11b8a 
> 
> 
> Diff: https://reviews.apache.org/r/66434/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66435: Added overloads for `int_fd` to `os::stat::isdir()` and `size()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 4, 2018, 5:52 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66435/
> ---
> 
> (Updated April 4, 2018, 5:52 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8682
> https://issues.apache.org/jira/browse/MESOS-8682
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These should be refactored to share the common code, and the
> additional overloads added to the other APIs too. However, it is not
> currently necessary, and would go unused.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/stat.hpp 
> 58353742b39bac4fbfcb2ab7708f0f8719ea5b3b 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
> 
> 
> Diff: https://reviews.apache.org/r/66435/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!





3rdparty/stout/include/stout/os/windows/fd.hpp
Line 50 (original), 52 (patched)


Nice fix. It's so easy to forget about enum classes.



3rdparty/stout/include/stout/os/windows/fd.hpp
Lines 128 (patched)


Yep this is weird, but your comment addresses why that's the case. I'm fine 
with it


- John Kordich


On April 4, 2018, 5:55 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66437/
> ---
> 
> (Updated April 4, 2018, 5:55 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675 and MESOS-8683
> https://issues.apache.org/jira/browse/MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8683
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After all the CRT APIs were replaced with Windows APIs, we no longer
> need to support the semantics of an `int` file descriptor in
> general (in the sense of opening a CRT handle that's associated with
> the actual kernel object for the given `HANDLE`). There are specific
> use cases (usually third-party code) which still require a CRT
> int-like file descriptor, which the `crt()` function explicitly
> allocates (this allocation used to be done in the constructor).
> 
> Thus the entire `FD_CRT` type was removed from the `WindowsFD`
> abstraction. It still acts like an `int` in the sense that it can be
> constructed from one and compared to one. However, construction via
> `int` only supports the standard file descriptors 0, 1, and 2 for
> `stdin`, `stdout`, and `stderr`. Any other construction creates an
> `int_fd` which holds an `INVALID_HANDLE` value. When being compared to
> an `int`, the abstraction simply returns -1 if it is invalid (based on
> the result of the `is_valid()` method) or 0 if it is valid. This is to
> support the semantics of checking validity by something like
> `if (fd < 0)` or `if (fd == -1)`.
> 
> With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
> APIs that switched on the type were simplified, with the last of the
> CRT code deleted.
> 
> Note that a new enum type `INVALID` was added to support default
> construction semantics, and only exists for that reason. An actual
> "invalid" handle should be constructed via `int_fd fd = -1` (or
> `int_fd fd = INVALID_HANDLE_VALUE` when being explicit on Windows).
> 
> Because `int_fd` is now castable to an `int`, and the `FD_CRT` type
> was removed, the comparison operators became much simpler.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/close.hpp 
> ff635e44235d63888a210cd68d49f6678a851e31 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> d7f8cdf1ad877eb55589bf5a9e75d295f91990a7 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> c190baa2230298e428d4034b90dccffb59b4e710 
> 
> 
> Diff: https://reviews.apache.org/r/66437/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.

2018-04-04 Thread John Kordich via Review Board

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




3rdparty/stout/include/stout/protobuf.hpp
Line 83 (original), 87 (patched)


This comment is kind of confusing. Maybe describe the scenario first, then 
describe what you've implemented to resolve the scenario?

Maybe something like: "Users of 'protobuf::write' will call 'os::close' on 
the original 'HANDLE'. This is a problem because..."


- John Kordich


On April 4, 2018, 5:56 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66439/
> ---
> 
> (Updated April 4, 2018, 5:56 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is another edge case where a third-party library (protobuf)
> requires a CRT integer file descriptor. Thus we duplicate the `int_fd`
> and then explicitly allocate via `crt()`, which requires that we also
> manually close it via `_close()`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 2fa5072e3c62c487da0dccffdd38d2fa1a615dc0 
> 
> 
> Diff: https://reviews.apache.org/r/66439/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66455: Windows: Fixed `os::ftruncate()` to use `SetEndOfFile()`.

2018-04-04 Thread John Kordich via Review Board

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


Ship it!





3rdparty/stout/include/stout/os/windows/ftruncate.hpp
Lines 36 (patched)


I left this comment on a different review, which this was then extracted 
on: "I'm fine with a truncate function not writing null bytes to the part of a 
file that was truncated. Truncate should be a fast operation, in my opinion :)"


- John Kordich


On April 4, 2018, 7:16 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66455/
> ---
> 
> (Updated April 4, 2018, 7:16 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8692
> https://issues.apache.org/jira/browse/MESOS-8692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This previously used the CRT API `_chsize_s()`, which required a CRT
> integer file descriptor. Instead, we can achieve the same behavior by
> first using `os::lseek()` (which uses `SetFilePointerEx()`) to seek
> `length`, and then use `SetEndOfFile()` to truncate. The only
> difference is that the file is not filled with null bytes when
> expanded, but we do not seem to rely on this behavior.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
> fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 
> 
> 
> Diff: https://reviews.apache.org/r/66455/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

2018-04-05 Thread John Kordich via Review Board

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


Ship it!




This really does feel hacky, but the alternative requires such a significant 
refactoring that it's probably not worth it (potential bug factory) just to get 
this working properly on Windows. I'll hold my nose and sign off.

- John Kordich


On April 5, 2018, 1:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66437/
> ---
> 
> (Updated April 5, 2018, 1:58 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675 and MESOS-8683
> https://issues.apache.org/jira/browse/MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8683
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After all the CRT APIs were replaced with Windows APIs, we no longer
> needed to support the semantics of an `int` file descriptor in
> general (in the sense of opening a CRT handle that's associated with
> the actual kernel object for the given `HANDLE`). There are specific
> use cases (usually third-party code) which still require a CRT
> int-like file descriptor, which the `crt()` function explicitly
> allocates (this allocation used to be done in the constructor).
> 
> Thus the entire `FD_CRT` type was removed from the `WindowsFD`
> abstraction. It still acts like an `int` in the sense that it can be
> constructed from one and compared to one. However, construction via
> `int` only supports the standard file descriptors 0, 1, and 2 for
> `stdin`, `stdout`, and `stderr`. Any other construction creates an
> `int_fd` which holds an `INVALID_HANDLE` value. When being compared to
> an `int`, the abstraction simply returns -1 if it is invalid (based on
> the result of the `is_valid()` method) or 0 if it is valid. This is to
> support the semantics of checking validity by something like `if (fd <
> 0)` or `if (fd == -1)`.
> 
> With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
> APIs that switched on the type were simplified, with the last of the
> CRT code deleted.
> 
> Thanks to the introduction of the private `int get_valid()` function,
> and the removal of the `FD_CRT` type, the comparison operators became
> much simpler.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/close.hpp 
> ff635e44235d63888a210cd68d49f6678a851e31 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> d7f8cdf1ad877eb55589bf5a9e75d295f91990a7 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> c190baa2230298e428d4034b90dccffb59b4e710 
> 
> 
> Diff: https://reviews.apache.org/r/66437/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:17 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66437/
> ---
> 
> (Updated April 27, 2018, 4:17 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-8675 and MESOS-8683
> https://issues.apache.org/jira/browse/MESOS-8675
> https://issues.apache.org/jira/browse/MESOS-8683
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After all the CRT APIs were replaced with Windows APIs, we no longer
> needed to support the semantics of an `int` file descriptor in
> general (in the sense of opening a CRT handle that's associated with
> the actual kernel object for the given `HANDLE`). There are specific
> use cases (usually third-party code) which still require a CRT
> int-like file descriptor, which the `crt()` function explicitly
> allocates (this allocation used to be done in the constructor).
> 
> Thus the entire `FD_CRT` type was removed from the `WindowsFD`
> abstraction. It still acts like an `int` in the sense that it can be
> constructed from one and compared to one. However, construction via
> `int` only supports the standard file descriptors 0, 1, and 2 for
> `stdin`, `stdout`, and `stderr`. Any other construction creates an
> `int_fd` which holds an `INVALID_HANDLE_VALUE`. When being compared to
> an `int`, the abstraction simply returns -1 if it is invalid (based on
> the result of the `is_valid()` method) or 0 if it is valid. This is to
> support the semantics of checking validity by something like
> `if (fd < 0)` or `if (fd == -1)`.
> 
> With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
> APIs that switched on the type were simplified, with the last of the
> CRT code deleted.
> 
> Thanks to the introduction of the private `int get_valid()` function,
> and the removal of the `FD_CRT` type, the comparison operators became
> much simpler.
> 
> Several unit tests in the `FsTest` suite became cross-platform, with
> the `Close` test being simplified to test against an `int_fd`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/close.hpp 
> ff635e44235d63888a210cd68d49f6678a851e31 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> 265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
>   3rdparty/stout/include/stout/os/windows/fd.hpp 
> d7f8cdf1ad877eb55589bf5a9e75d295f91990a7 
>   3rdparty/stout/include/stout/os/windows/pipe.hpp 
> 365db9480f6258a03ef2e760a19abef8ab177e58 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> 8047ad590fcc46d3ec46b551472d8c518ae49cc1 
>   3rdparty/stout/include/stout/os/windows/socket.hpp 
> 259b05b8c85e399feaccec698d58b7d540cad368 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 71006489918d9495d37d2fdfdca08b40b419481a 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> c190baa2230298e428d4034b90dccffb59b4e710 
>   3rdparty/stout/tests/os/socket_tests.cpp 
> 8ea0f1238e4293a5cd313b4ee38a920b381f4022 
> 
> 
> Diff: https://reviews.apache.org/r/66437/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66578: Windows: Ported more unit tests from `os_tests.cpp`.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:20 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66578/
> ---
> 
> (Updated April 27, 2018, 4:20 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Michael Park.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `os::sleep()` to return an invalid parameter error if given a
> negative value.
> 
> Fixed tests around the `cloexec` and `nonblock` stubs.
> 
> Extended the `bootid` test to use `std::chrono` to assert the boot
> id (which is the system boot time) is a reasonable value.
> 
> Permanently disabled `OsTest.Libraries` because there is no
> equivalent.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/bootid.hpp 
> d24e115b97ebcabfa808a9437fb41512f25a5271 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 900baf9be4e3089cb43e444b14b155a80bcd1591 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66578/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:21 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66773/
> ---
> 
> (Updated April 27, 2018, 4:21 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu.
> 
> 
> Bugs: MESOS-8275
> https://issues.apache.org/jira/browse/MESOS-8275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The functions `mode()`, `dev()`, and `inode()` are unused and do not
> make sense on Windows, so they were explicitly deleted. The function
> `mtime()` is used and has a logical mapping, `GetFileTime()`. However,
> Windows reports time differently from POSIX, so a conversion must also
> be performed such that the API `os::stat::mtime()` remains consistent
> with its POSIX version.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 453e60c7268db516c2c94501e11a92fe8f490498 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> c04953ee42f45dd80b6362fbeeddf4a0a20e7412 
>   3rdparty/stout/tests/os_tests.cpp 4221ecdcefb5602ece20cc90b13c3f17057fcb4d 
> 
> 
> Diff: https://reviews.apache.org/r/66773/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66835: Replaced `int` and `HANDLE` types with `int_fd`.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:22 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66835/
> ---
> 
> (Updated April 27, 2018, 4:22 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `int` and `HANDLE` types with `int_fd`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 0f66d6b49cbea9964457e890eff3cc7f1e058118 
> 
> 
> Diff: https://reviews.apache.org/r/66835/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66834: Windows: Specialized `flags::parse`.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:22 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66834/
> ---
> 
> (Updated April 27, 2018, 4:22 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Specialized `flags::parse`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> eb6d5272ffefcfe4fe7b97f9c7c7084893269b64 
> 
> 
> Diff: https://reviews.apache.org/r/66834/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 66836: Fixed `mesos-tcp-connect` to use `net::socket`.

2018-04-27 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


On April 27, 2018, 4:22 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66836/
> ---
> 
> (Updated April 27, 2018, 4:22 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use the stout wrapper instead of `::socket` so we have built-in error
> checking (and don't have to worry about types).
> 
> 
> Diffs
> -
> 
>   src/checks/tcp_connect.cpp f5df732415b1e3ac02e9624909d822febe60fe8c 
> 
> 
> Diff: https://reviews.apache.org/r/66836/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



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

2018-05-01 Thread John Kordich via Review Board

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


Ship it!




Ship It!

- John Kordich


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



Review Request 67065: Added a new stout utility header file which interfaces with libarchive.

2018-05-10 Thread John Kordich via Review Board

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.


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


Repository: mesos


Description
---

This archiver utility can be invoked to extract a compressed or
uncompressed archive with several different supported formats.


Diffs
-

  3rdparty/stout/CMakeLists.txt 24a1f0acbee8a464fcc5159cf41d7e93aa8148fc 
  3rdparty/stout/Makefile.am ef22a02a8a11326c6af19eb11d79eb82ff7861da 
  3rdparty/stout/include/stout/archiver.hpp PRE-CREATION 
  3rdparty/stout/tests/CMakeLists.txt 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
  3rdparty/stout/tests/archiver_tests.cpp PRE-CREATION 


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


Testing
---

I've built on Windows with CMake and have run all tests, which all pass.
I've also built on Linux with both the autotools build and cmake build and have 
run all tests, which all pass.


Thanks,

John Kordich



Review Request 67066: Modified the fetcher to use libarchive and added associated tests.

2018-05-10 Thread John Kordich via Review Board

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.


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


Repository: mesos


Description
---

Modified the fetcher to use libarchive and added associated tests.


Diffs
-

  src/launcher/fetcher.cpp 4cff7273fdc9e3897074da4da7dad02483086c2d 
  src/tests/fetcher_tests.cpp 311be6a36f137819ee91187a16df2b8e61def478 


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


Testing
---

I've built on Windows with CMake and have run all tests, which all pass.
I've also built on Linux with both the autotools build and cmake build and have 
run all tests, which all pass.


Thanks,

John Kordich



Review Request 67064: Added libarchive, bzip2, and xz tarballs and associated build changes.

2018-05-10 Thread John Kordich via Review Board

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, 
Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.


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


Repository: mesos


Description
---

These libraries being available will allow stout to be changed such that
stout can invoke libarchive instead of shelling out to tar to extract
archives. On Windows, tar usually doesn't exist, and even if it does, it
rarely supports most compression formats. On Windows, we will build
libarchive to support each of those compression formats.


Diffs
-

  3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
  3rdparty/Makefile.am 8d9fa85dd65a94d91670d54dab36deea80d14996 
  3rdparty/bzip2-1.0.6.patch PRE-CREATION 
  3rdparty/bzip2-1.0.6.tar.gz PRE-CREATION 
  3rdparty/cmake/Versions.cmake 2828acd2aa00c8778c6c462bc594716b2b6289ba 
  3rdparty/libarchive-3.3.2.patch PRE-CREATION 
  3rdparty/libarchive-3.3.2.tar.gz PRE-CREATION 
  3rdparty/versions.am ada998d62d012a45b2cf17256c1ae16b92d611f2 
  3rdparty/xz-5.2.3.patch PRE-CREATION 
  3rdparty/xz-5.2.3.tar.gz PRE-CREATION 
  configure.ac 6e91ecf4659ebec2e124ca4b2218c830608abeb0 
  src/Makefile.am 7e91681e3b8b217f8b23fa5347e059640c62fc65 


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


Testing
---

I've built on Windows with CMake and have run all tests, which all pass.
I've also built on Linux with both the autotools build and cmake build and have 
run all tests, which all pass.


Thanks,

John Kordich



  1   2   >