Re: Review Request 39852: Windows: Replaced global `GetMessage` macro with inline function.

2015-12-15 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Nov. 16, 2015, 9:15 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39852/
> ---
> 
> (Updated Nov. 16, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Replaced global `GetMessage` macro with inline function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39852/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39888: Windows: Added compatibility code for `grp.h` and `pwd.h`.

2015-12-15 Thread Alex Naparu

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/grp.hpp (line 
37)
<https://reviews.apache.org/r/39888/#comment170464>

Returning NULL without setting errno is probably not a good idea. We might 
want to standardize on '0' for group/user IDs on Windows.


- Alex Naparu


On Nov. 16, 2015, 9:15 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39888/
> ---
> 
> (Updated Nov. 16, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code will be particularly useful when we expand Windows support for
> `files/files.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/grp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/pwd.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39888/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39888: Windows: Added compatibility code for `grp.h` and `pwd.h`.

2015-12-15 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Nov. 16, 2015, 9:15 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39888/
> ---
> 
> (Updated Nov. 16, 2015, 9:15 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code will be particularly useful when we expand Windows support for
> `files/files.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/grp.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/pwd.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39888/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39850: Process: Added headers to make `process/mime.hpp` standalone.

2015-12-15 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Nov. 16, 2015, 9:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39850/
> ---
> 
> (Updated Nov. 16, 2015, 9:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Process: Added headers to make `process/mime.hpp` standalone.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/mime.hpp 
> decdfb6bc2eb60bfc6b25bc7227b11e8a11d5aff 
> 
> Diff: https://reviews.apache.org/r/39850/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

2015-12-14 Thread Alex Naparu

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 62)
<https://reviews.apache.org/r/39584/#comment170217>

Nit: You're not reusing these, so might as well inline the calls.


- Alex Naparu


On Nov. 16, 2015, 9:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Nov. 16, 2015, 9:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

2015-12-14 Thread Alex Naparu

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



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 79)
<https://reviews.apache.org/r/39019/#comment170225>

Consider using a smart pointer instead. Same below.



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 215)
<https://reviews.apache.org/r/39019/#comment170231>

This is copied here, but malloc'd elsewhere. Is that safe?



3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
(line 230)
<https://reviews.apache.org/r/39019/#comment170237>

Consider using safe versions of string functions



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 74)
<https://reviews.apache.org/r/39019/#comment170220>

Might want to add this as a reference too: 
https://msdn.microsoft.com/en-us/library/930f87yf.aspx

MAX_PATH is horribly obsolete, we might want to consider checking whether 
we're running on NTFS for all FS-related ops. That will be true in the vast 
majority of cases these days.


- Alex Naparu


On Nov. 17, 2015, 7:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Nov. 17, 2015, 7:06 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 1a7037d64afeedc340258c92067e95d1d3caa027 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

2015-12-14 Thread Alex Naparu

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



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 39)
<https://reviews.apache.org/r/39584/#comment170194>

What if the path ends with multiple '\' chars to begin with?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 47)
<https://reviews.apache.org/r/39584/#comment170206>

What happens if the input path already contains a wildcard? Might be a good 
idea to check that "path" is a directory before moving forward. Also might be a 
good idea to use realpath here.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 51)
<https://reviews.apache.org/r/39584/#comment170192>

Calling FindClose on an invalid handle is not the best idea. At best it 
will be a no-op.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 62)
<https://reviews.apache.org/r/39584/#comment170195>

Nit: tou're not reusing these, so might as well inline the callse.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 73)
<https://reviews.apache.org/r/39584/#comment170196>

Attributes should already be available in WIN32_FIND_DATA (see 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365740(v=vs.85).aspx),
 you might not need to call GetFileAttribiutes here. If you do need to get the 
attributes, consider using CreateFile and GetFileInformationByHandle, which 
will always give the up-to-date information even on NTFS (see 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa364952(v=vs.85).aspx 
for details)



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 77)
<https://reviews.apache.org/r/39584/#comment170197>

Consider using a smart pointer for closing the handle.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 97)
<https://reviews.apache.org/r/39584/#comment170213>

Failure of ::remove alone might not be reason enough for aborting the 
entire call. For instance, if ::remove fails with "file not found", we're still 
good to go.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 108)
<https://reviews.apache.org/r/39584/#comment170214>

Same comment as for line 108.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (lines 
110 - 111)
<https://reviews.apache.org/r/39584/#comment170199>

Nit: error message is the same as the one on line 100, which makes it hard 
to tell what happened, even for someone with access to the source code.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp (line 138)
<https://reviews.apache.org/r/39584/#comment170200>

Consider using ::RemoveDirectory here, which will delete the directory when 
the last handle is closed. Unless that's not the desired behavior...


- Alex Naparu


On Nov. 16, 2015, 9:13 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Nov. 16, 2015, 9:13 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> a8c35c086ecae21701f6a720f25231c1b0d4e329 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5c1df81193b4b888d2ed5c7dbfa0b5e2fae48467 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> e26df59d9b837e1f4a4b92577f0a3de4b9076cb4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> edf17d5ad8efbc988e909bfb8ffa5a015ecdc89d 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39804: Windows: Moved `os::find` to its own file, `stout/os/find.hpp`.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 4, 2016, 6:35 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39804/
> ---
> 
> (Updated Jan. 4, 2016, 6:35 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Moved `os::find` to its own file, `stout/os/find.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/find.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39804/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39805: Moved filesystems tests to their own file.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 4, 2016, 6:39 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39805/
> ---
> 
> (Updated Jan. 4, 2016, 6:39 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved filesystems tests to their own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 5ab7bc4966fe32eaddd573a4dbfd997f98b5d481 
>   3rdparty/libprocess/3rdparty/stout/include/stout/tests/utils.hpp 
> 3cff837886d94d623c732371380f1c62750022f4 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 62ad461eb228b688f1ceac16cfb003561ed5a806 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> e0a898d91e3521d4b228621a81412e1dd5ddf63d 
> 
> Diff: https://reviews.apache.org/r/39805/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39802: Windows: Implemented `stout/os/windows/ls.hpp`.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 4, 2016, 11:20 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39802/
> ---
> 
> (Updated Jan. 4, 2016, 11:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `stout/os/windows/ls.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ls.hpp 
> 7dba79d31559d15a3e84eff506ce7df3e57cf5f3 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
> 5b6fba13ce215af5801fd0867f6e774e100689ca 
> 
> Diff: https://reviews.apache.org/r/39802/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39584: Windows: Implemented `os::rmdir.hpp`.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 4, 2016, 12:02 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39584/
> ---
> 
> (Updated Jan. 4, 2016, 12:02 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented `os::rmdir.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 14fbca6d222bdfc0e8be301050b4ea1a8a6e7758 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rmdir.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 4cf693fb7e8c6bb3ad1920ebe90d61f0adb5dc99 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> e738cdbf5846950c475c159fb9a770acc45159f5 
> 
> Diff: https://reviews.apache.org/r/39584/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39834: Made `path_tests.cpp` standalone.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Nov. 16, 2015, 9:14 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39834/
> ---
> 
> (Updated Nov. 16, 2015, 9:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Michael Hopcroft, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `path_tests.cpp` standalone.
> 
> NB, more of these tests will move when `Hopcroft` is done with his 
> `os::symlink` changeset.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> ad9ce324eaf940f68d04c6db7ba37d05efb1216a 
> 
> Diff: https://reviews.apache.org/r/39834/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 41604: CMake: Added missing protobuf files to CMake build.

2016-01-04 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Dec. 23, 2015, 6:52 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41604/
> ---
> 
> (Updated Dec. 23, 2015, 6:52 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Diana Arroyo, Daniel Pravat, Artem 
> Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added missing protobuf files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bdc45ae604c940dadc27ab6e8b8a3327bd00642b 
> 
> Diff: https://reviews.apache.org/r/41604/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 39019: Windows: Added dirent compat code for non-Unix systems.

2015-12-22 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Dec. 23, 2015, 2:15 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39019/
> ---
> 
> (Updated Dec. 23, 2015, 2:15 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3441
> https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dirent compat code for non-Unix systems.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> d46e262e0fd1c2de36f3bf19d8bd693c23bf58cd 
> 
> Diff: https://reviews.apache.org/r/39019/diff/
> 
> 
> Testing
> ---
> 
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 42035: Windows: Removed the `--switch_user` flag in Windows.

2016-01-12 Thread Alex Naparu

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

Ship it!


Ship It!

- Alex Naparu


On Jan. 7, 2016, 9:47 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42035/
> ---
> 
> (Updated Jan. 7, 2016, 9:47 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Jie 
> Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4310
> https://issues.apache.org/jira/browse/MESOS-4310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Mesos agent provides a flag, `--[no-]switch_user`, which instructs
> the agent to run tasks as the user who submitted the task instead of the
> user that is running the agent process itself.
> 
> On POSIX, this requires the usual suspects (`su`, `chown`, and `chroot`,
> for example to do things like make sure the correct user owns the
> executor directory, and things like that.
> 
> On Windows, many of these operations have very different semantics. To
> use one example, to `chown` on Windows, you normally have to give
> *permission* for something to `chown` an object, and then that thing
> actually has to use that permission to `chown` it, on its own.
> 
> Clearly a semantic mismatch of this magnitude will require some work to
> formally bridge, and the result is that we will simply disable the
> functionality entirely on the Windows agent until we are prepared to go
> back and support it formally.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/slave/flags.hpp 6857fde027fd57b4934cb43ddf435d12900e0b87 
>   src/slave/flags.cpp 19c2996c4572b992030f8824380f3979ced7e526 
>   src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 
> 
> Diff: https://reviews.apache.org/r/42035/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43905: Windows: Removed `user` launcher flag, preventing `su`.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:17 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43905/
> ---
> 
> (Updated Feb. 25, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `su` does not exist on Windows. Unfortunately, the launcher also depends
> on it. In this commit, we remove Windows support for the launcher flag
> `user`, which controls whether we use `su` in the launcher. This
> allows us to divest ourselves of `su` altogether on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> d917a99a46841156dc1e359c44010938cc45e943 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 129406abdff715e321f683911e404c46676b6daf 
>   src/slave/containerizer/mesos/launch.hpp 
> 7e29ca2b8bec1c20aef122472cff60f6003603ad 
>   src/slave/containerizer/mesos/launch.cpp 
> 6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 
> 
> Diff: https://reviews.apache.org/r/43905/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43904: Windows: Removed `rootfs` launcher flag, preventing `chroot`.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:17 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43904/
> ---
> 
> (Updated Feb. 25, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4780
> https://issues.apache.org/jira/browse/MESOS-4780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `chroot` is does not exist on Windows. Unfortunately, the launcher also
> depends on it. In this commit, we remove Windows support for the
> launcher flag `rootfs`, which controls whether we use `chroot` in the
> launcher. This allows us to divest ourselves of `chroot` altogether on
> Windows.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 129406abdff715e321f683911e404c46676b6daf 
>   src/slave/containerizer/mesos/launch.hpp 
> 7e29ca2b8bec1c20aef122472cff60f6003603ad 
>   src/slave/containerizer/mesos/launch.cpp 
> 6b3bf163e2a577e6318a4a62f96d6bfd98ef9ae9 
> 
> Diff: https://reviews.apache.org/r/43904/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43907: Stout:[1/2] Fix error reporting bug in `os::rmdir`.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:41 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43907/
> ---
> 
> (Updated Feb. 25, 2016, 7:41 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[1/2] Fix error reporting bug in `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/43907/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43906: CMake: Added files to be built as part of libmesos.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:17 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43906/
> ---
> 
> (Updated Feb. 25, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added files to be built as part of libmesos.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 5cf0ec8c475839ad8717192a37f01546cbcccd7a 
> 
> Diff: https://reviews.apache.org/r/43906/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43903: Stout: Add `WindowsError` constructor to `Result`.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:13 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43903/
> ---
> 
> (Updated Feb. 25, 2016, 7:13 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Add `WindowsError` constructor to `Result`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp 
> 577c8e459f505b7b9021153d5e42e9d5a892ec33 
> 
> Diff: https://reviews.apache.org/r/43903/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-02-26 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43407: CMake: Force GMock and libevent to build and link statically.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43407/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Force GMock to build and link statically.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43407/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43409: Windows: Added `src/resource_estimator.cpp` to build.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43409/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `src/resource_estimator.cpp` to build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
> 
> Diff: https://reviews.apache.org/r/43409/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43411: Windows: Added dynamic library loading tests to build.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added dynamic library loading tests to build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 
> 27626ae28db090f1a002239ff5c674b82e8fc9a8 
> 
> Diff: https://reviews.apache.org/r/43411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43415: CMake: Moved Windows build to version of glog that builds with CMake.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:08 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43415/
> ---
> 
> (Updated Feb. 18, 2016, 2:08 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Moved Windows build to version of glog that builds with CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43415/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43413: CMake:[1/2] Allow downloading third-party dependencies from mirror.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43413/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[1/2] Allow downloading third-party dependencies from mirror.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3253da73aa517a335be94148d567510147dae08d 
>   CMakeLists.txt 7f83dc84997d3b824d1f63012894bd9fc5284053 
> 
> Diff: https://reviews.apache.org/r/43413/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43414: CMake:[2/2] Canonicalize location of third-party dependencies.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43414/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/2] Canonicalize location of third-party dependencies.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
> 
> Diff: https://reviews.apache.org/r/43414/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43415: CMake: Moved Windows build to version of glog that builds with CMake.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:08 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43415/
> ---
> 
> (Updated Feb. 18, 2016, 2:08 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Moved Windows build to version of glog that builds with CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43415/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43416: Windows: Removed ambiguous call to `::write`.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43416/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed ambiguous call to `::write`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 
> 88b355e09f76f0412c74ad69556572f0079deb8f 
> 
> Diff: https://reviews.apache.org/r/43416/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43417: Windows: Marked functions in headers `inline` to avoid linker errors.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43417/
> ---
> 
> (Updated Feb. 18, 2016, 12:59 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Marked functions in headers `inline` to avoid linker errors.
> 
> 
> Diffs
> -
> 
>   
> 3rdparty/libprocess/3rdparty/stout/include/stout/internal/windows/dirent.hpp 
> 046388189823c0c41ce6cc135d5d3838e9131087 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 9a592c4ec9f45fdd8ae8c724c3cab67876de72f5 
> 
> Diff: https://reviews.apache.org/r/43417/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43691: CMake:[2/2] Fixed http-parser library directory for Windows builds.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:24 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43691/
> ---
> 
> (Updated Feb. 18, 2016, 2:24 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/2] Fixed http-parser library directory for Windows builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
> 
> Diff: https://reviews.apache.org/r/43691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43418: Windows: Added slave/status_update_manager.cpp and other files.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 1:01 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43418/
> ---
> 
> (Updated Feb. 18, 2016, 1:01 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added slave/status_update_manager.cpp and other files.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
> 
> Diff: https://reviews.apache.org/r/43418/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43692: CMake:[1/4] Transitioned to 64-bit build of ZK on Windows.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:26 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43692/
> ---
> 
> (Updated Feb. 18, 2016, 2:26 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[1/4] Transitioned to 64-bit build of ZK on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 3253da73aa517a335be94148d567510147dae08d 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 0c80fb8d799ea1252492cd98ac0780f1228aadcd 
>   3rdparty/zookeeper-06d3f3f.patch 8f8f72cb712c4bca328bf865ab082926106fbd94 
>   cmake/MesosConfigure.cmake 9a4fdb57e1281d9ec421e639819de5786c11744a 
>   src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
> 
> Diff: https://reviews.apache.org/r/43692/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43693: CMake:[2/4] Transitioned ZK build to be 64-bit and static.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:27 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43693/
> ---
> 
> (Updated Feb. 18, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/4] Transitioned ZK build to be 64-bit and static.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat 
> 8da05bcf8e6b2335770cff7c43218b823721d50c 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake 
> 632696e883e8ccb9bfa82288f62e7ad3771b043e 
> 
> Diff: https://reviews.apache.org/r/43693/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43694: Windows:[3/4] Transitioned to static-everything builds on Windows.

2016-02-19 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/CMakeLists.txt (line 206)
<https://reviews.apache.org/r/43694/#comment181329>

Maybe remove it altogether?


- Alex Naparu


On Feb. 18, 2016, 2:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43694/
> ---
> 
> (Updated Feb. 18, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[3/4] Transitioned to static-everything builds on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 2a37fdb6501aaf7baac2ada0a714bbe67e7c5aca 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3a2e0999722007475c023ade75719093e35cfc80 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 12dfaf61b801372b6ec70c535080fde350866fb8 
> 
> Diff: https://reviews.apache.org/r/43694/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43694: Windows:[3/4] Transitioned to static-everything builds on Windows.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43694/
> ---
> 
> (Updated Feb. 18, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[3/4] Transitioned to static-everything builds on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 2a37fdb6501aaf7baac2ada0a714bbe67e7c5aca 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 3a2e0999722007475c023ade75719093e35cfc80 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 12dfaf61b801372b6ec70c535080fde350866fb8 
> 
> Diff: https://reviews.apache.org/r/43694/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43695: Windows:[4/4] Included Socket library for 64-bit builds.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43695/
> ---
> 
> (Updated Feb. 18, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[4/4] Included Socket library for 64-bit builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> a27cb98fa45cbd135ebfeca65e215fb3ff054739 
> 
> Diff: https://reviews.apache.org/r/43695/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43700: CMake: Fixed Find* scripts to not explode if invoked twice.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:31 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43700/
> ---
> 
> (Updated Feb. 18, 2016, 2:31 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Fixed Find* scripts to not explode if invoked twice.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/FindApr.cmake 
> 4b28aa170f48d37ae9096bc28a64d8a32e8d35dd 
>   3rdparty/libprocess/3rdparty/stout/cmake/FindSvn.cmake 
> c975a16975e76b38f028d7575775abd31c9090f5 
> 
> Diff: https://reviews.apache.org/r/43700/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43689: CMake: Begin to require 64-bit builds.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 2:29 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43689/
> ---
> 
> (Updated Feb. 18, 2016, 2:29 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3525
> https://issues.apache.org/jira/browse/MESOS-3525
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Candidate compilation configuration finalization.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake ab503b23f054ebc9a3877a3eca27b1b4190aa51b 
> 
> Diff: https://reviews.apache.org/r/43689/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43697: CMake:[1/3] Move Stout configuration to its own file.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 5:49 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43697/
> ---
> 
> (Updated Feb. 18, 2016, 5:49 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4703
> https://issues.apache.org/jira/browse/MESOS-4703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[1/3] Move Stout configuration to its own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> a27cb98fa45cbd135ebfeca65e215fb3ff054739 
> 
> Diff: https://reviews.apache.org/r/43697/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43698: CMake:[2/3] Use new Stout config script in libprocess 3rdparty build.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 4:34 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43698/
> ---
> 
> (Updated Feb. 18, 2016, 4:34 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4703
> https://issues.apache.org/jira/browse/MESOS-4703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[2/3] Use new Stout config script in libprocess 3rdparty build.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 9b61376ea6aad304607c20c9823d9ef19013eca0 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> d36fa2fbe903fb278e6c00b47bfa4b81cf8f4673 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 2a37fdb6501aaf7baac2ada0a714bbe67e7c5aca 
> 
> Diff: https://reviews.apache.org/r/43698/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 43699: CMake:[3/3] Used Stout config script in agent build.

2016-02-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 18, 2016, 4:35 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43699/
> ---
> 
> (Updated Feb. 18, 2016, 4:35 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4703
> https://issues.apache.org/jira/browse/MESOS-4703
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[3/3] Used Stout config script in agent build.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 7f83dc84997d3b824d1f63012894bd9fc5284053 
>   src/slave/cmake/SlaveConfigure.cmake 
> a8270a9ac6ceee4f370e185bee82126c309ec134 
> 
> Diff: https://reviews.apache.org/r/43699/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 45987: Fixed `rmdir.hpp` Windows build breaks.

2016-04-12 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On April 10, 2016, 10:34 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45987/
> ---
> 
> (Updated April 10, 2016, 10:34 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed `rmdir.hpp` Windows build breaks.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> dd5cc6923ecc503d58e56128cf4ae736d8145fd7 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> cc92953ccc53f37c54b87e738b16ea1fb521b987 
> 
> Diff: https://reviews.apache.org/r/45987/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

2016-04-12 Thread Alex Naparu


> On April 12, 2016, 2:37 p.m., Joris Van Remoortere wrote:
> > There is no JIRA for this.
> > Initializing the winsock system seems like functionality that belongs in a 
> > helper function rather than inside a test?
> > How is libprocess going to initialize it? What about mesos?
> > I'm guessing we'll want to have something like:
> > ```
> > Try winsock_initialize = winsock_initialize();
> > ...
> > ```
> > 
> > and do this in `libprocess::initialize()`
> > 
> > Thoughts?
> 
> Alex Naparu wrote:
> This should likely be wrapped in the constructor of a class, while 
> WSACleanup should be called from the destructor.

This: 
https://github.com/dpravat/mesos/commit/5b2e549737edcb6a8522026e01b8f4ed70da4d9f


- Alex


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


On April 11, 2016, 8:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46008/
> -------
> 
> (Updated April 11, 2016, 8:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Initialize Windows socket stack in Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
> c449de87bede4d3bf1df368eaf1d22f857c2298f 
> 
> Diff: https://reviews.apache.org/r/46008/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 44123: Stout: [1/2] Implemented assorted `os::` functions on Windows.

2016-04-12 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 134)
<https://reviews.apache.org/r/44123/#comment191995>

You can probably inline this.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 212)
<https://reviews.apache.org/r/44123/#comment191996>

This would probably fit on one line if you wrote it as "if 
(!::GetEnvironmentVariable..."


- Alex Naparu


On April 11, 2016, 4:20 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44123/
> ---
> 
> (Updated April 11, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: [1/2] Implemented assorted `os::` functions on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/44123/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

2016-04-12 Thread Alex Naparu


> On April 12, 2016, 2:37 p.m., Joris Van Remoortere wrote:
> > There is no JIRA for this.
> > Initializing the winsock system seems like functionality that belongs in a 
> > helper function rather than inside a test?
> > How is libprocess going to initialize it? What about mesos?
> > I'm guessing we'll want to have something like:
> > ```
> > Try winsock_initialize = winsock_initialize();
> > ...
> > ```
> > 
> > and do this in `libprocess::initialize()`
> > 
> > Thoughts?

This should likely be wrapped in the constructor of a class, while WSACleanup 
should be called from the destructor.


- Alex


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


On April 11, 2016, 8:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46008/
> -------
> 
> (Updated April 11, 2016, 8:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Initialize Windows socket stack in Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
> c449de87bede4d3bf1df368eaf1d22f857c2298f 
> 
> Diff: https://reviews.apache.org/r/46008/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 45614: Windows: Implemented os::close for Windows.

2016-04-12 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On April 2, 2016, 6:35 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45614/
> ---
> 
> (Updated April 2, 2016, 6:35 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented os::close for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/close.hpp 
> 1c912c3edc815e7e0b1a562286279897d773a516 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> ee13d12fcffcd564c7ded2d2f541d7bbdf6633c1 
> 
> Diff: https://reviews.apache.org/r/45614/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 45614: Windows: Implemented os::close for Windows.

2016-04-12 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp (line 29)
<https://reviews.apache.org/r/45614/#comment191990>

`return ErrnoError()` if this fails? Same for ::closesocket().


- Alex Naparu


On April 2, 2016, 6:35 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45614/
> ---
> 
> (Updated April 2, 2016, 6:35 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Implemented os::close for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/close.hpp 
> 1c912c3edc815e7e0b1a562286279897d773a516 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> ee13d12fcffcd564c7ded2d2f541d7bbdf6633c1 
> 
> Diff: https://reviews.apache.org/r/45614/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

2016-04-12 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp (line 164)
<https://reviews.apache.org/r/46013/#comment191997>

I assume "disappear" means "between enumeration and now". Can you expand 
this comment a bit please?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 270)
<https://reviews.apache.org/r/46013/#comment192018>

Doesn't follow stout naming convention (and can be mistaken for a WinAPI 
function).



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 272)
<https://reviews.apache.org/r/46013/#comment192025>

Not entirely sure we need this output arg



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 286)
<https://reviews.apache.org/r/46013/#comment192020>

spacing



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 289)
<https://reviews.apache.org/r/46013/#comment192023>

Comparing a HANDLE to NULL is a risky thing to do. I don't think MSDN 
defines NULL (which is defined as 0 in WinAPI) as an invalid handle value. We 
might just trigger a false negative here. Any HANDLE result should only be 
tested against INVALID_HANDLE_VALUE.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 294)
<https://reviews.apache.org/r/46013/#comment192024>

I think we have some sort of safe_handle type by now, right?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 304)
<https://reviews.apache.org/r/46013/#comment192027>

This can probably be scoped to each code block. No need to define it here, 
just creates confusion.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 339)
<https://reviews.apache.org/r/46013/#comment192028>

These can all be on the same line.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 372)
<https://reviews.apache.org/r/46013/#comment192015>

I don't think we should be using this flag. The fact that FindProcess 
returned an error is a solid indication that something is wrong.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 382)
<https://reviews.apache.org/r/46013/#comment192019>

Can you qualify these calls with `::` please?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 383)
<https://reviews.apache.org/r/46013/#comment192009>

Can you document this flag a bit please? GetProcessTimes requires 
PROCESS_QUERY_LIMITED_INFORMATION, while GetProcessMemoryInfo requires the 
additional PROCESS_VM_READ.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 399)
<https://reviews.apache.org/r/46013/#comment192011>

Extra newline?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 419)
<https://reviews.apache.org/r/46013/#comment192012>

Newline



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 423)
<https://reviews.apache.org/r/46013/#comment192007>

I don't think these can be negative, consider using ULARGE_INTEGER instead.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 429)
<https://reviews.apache.org/r/46013/#comment192008>

Do you need system_time and user_time? I think these can be inlined below.


- Alex Naparu


On April 11, 2016, 7:19 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> -------
> 
> (Updated April 11, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

2016-04-12 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/tests/main.cpp (line 81)
<https://reviews.apache.org/r/46008/#comment191974>

We must also call WSACleanup on successful WSAStartup, per MSDN.


- Alex Naparu


On April 11, 2016, 8:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46008/
> ---
> 
> (Updated April 11, 2016, 8:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Initialize Windows socket stack in Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
> c449de87bede4d3bf1df368eaf1d22f857c2298f 
> 
> Diff: https://reviews.apache.org/r/46008/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 46008: Stout: Initialize Windows socket stack in Stout tests.

2016-04-12 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On April 11, 2016, 8:28 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46008/
> ---
> 
> (Updated April 11, 2016, 8:28 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Initialize Windows socket stack in Stout tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/main.cpp 
> c449de87bede4d3bf1df368eaf1d22f857c2298f 
> 
> Diff: https://reviews.apache.org/r/46008/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 44131: Libprocess: [2/2] Implemented assorted `os::` functions on Windows.

2016-04-12 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On April 11, 2016, 4:21 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44131/
> ---
> 
> (Updated April 11, 2016, 4:21 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: [2/2] Implemented assorted `os::` functions on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d2c458ed93307f75358bb642aaf2ed8e17b2fe97 
> 
> Diff: https://reviews.apache.org/r/44131/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 44123: Stout: [1/2] Implemented assorted `os::` functions on Windows.

2016-04-12 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 86)
<https://reviews.apache.org/r/44123/#comment192052>

Is it worth handling the error case here?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 164)
<https://reviews.apache.org/r/44123/#comment192053>

Do we no longer have a "not implemented" error?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 228)
<https://reviews.apache.org/r/44123/#comment192054>

We don't have a X_OK flag in Windows, so we might want to mask that out 
before calling `::access`.


- Alex Naparu


On April 11, 2016, 4:20 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44123/
> ---
> 
> (Updated April 11, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: [1/2] Implemented assorted `os::` functions on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/44123/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 44760: Windows: Fixed non-blocking connect.

2016-03-19 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On March 17, 2016, 6:54 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44760/
> ---
> 
> (Updated March 17, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Alex 
> Clemmer, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Fixed non-blocking connect.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/network.hpp 
> 7d203f0ff1cdb3145bc2b914f8bd606203878f09 
>   3rdparty/libprocess/src/poll_socket.cpp 
> 6e6634b4b352e3723096521843546cf56ec6dd8b 
> 
> Diff: https://reviews.apache.org/r/44760/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 47489: Windows: Symplified `os::exists`.

2016-05-17 Thread Alex Naparu

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


Ship it!




Ship It!

- Alex Naparu


On May 17, 2016, 7:55 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47489/
> ---
> 
> (Updated May 17, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Symplified `os::exists`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/exists.hpp 
> 423e4a81b4d34460f7f9f073577e11dfa2d2a520 
> 
> Diff: https://reviews.apache.org/r/47489/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>



Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

2016-04-13 Thread Alex Naparu


> On April 12, 2016, 11:10 p.m., Alex Naparu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp, line 383
> > <https://reviews.apache.org/r/46013/diff/2/?file=1339790#file1339790line383>
> >
> > Can you document this flag a bit please? GetProcessTimes requires 
> > PROCESS_QUERY_LIMITED_INFORMATION, while GetProcessMemoryInfo requires the 
> > additional PROCESS_VM_READ.
> 
> Alex Clemmer wrote:
> Sure, but what part would you like me to document?

I would like to see some explanatory note as to why the THREAD_ALL_ACCESS flag 
is required (and appropriate) in this context. Looking at MSDN, none of the 
functions that are using the process handle (GetProcessTimes and 
GetProcessMemoryInfo) make any note of this flag. It might be that 
THREAD_ALL_ACCESS is overkill in this context.


- Alex


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


On April 13, 2016, 9:49 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 9:49 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

2016-04-13 Thread Alex Naparu

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




3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 310)
<https://reviews.apache.org/r/46013/#comment192236>

`break` is not needed after `return`


- Alex Naparu


On April 13, 2016, 10:06 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 13, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>