Re: Review Request 68329: Windows: Made `libwinio` the default option.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68329/#review207640 --- Ship it! Ship It! - Akash Gupta On Aug. 20, 2018, 10:11 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68329/ > --- > > (Updated Aug. 20, 2018, 10:11 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu. > > > Bugs: MESOS-9084 > https://issues.apache.org/jira/browse/MESOS-9084 > > > Repository: mesos > > > Description > --- > > Note that we now add `io_tests.cpp` unconditionally, which will > fail/hang with libevent. However, this is because libevent is buggy, > and should not be used. We would prefer the tests not pass with > libevent enabled, rather than appear to pass because the failing tests > are excluded, giving the false impression that it works correctly. > > > Diffs > - > > 3rdparty/libprocess/src/CMakeLists.txt > 19fa9809c2298ea68649702b94a0c75d806caba3 > 3rdparty/libprocess/src/tests/CMakeLists.txt > a03a77eb5e289b4daac0bbd414dc17c8acc848dc > > > Diff: https://reviews.apache.org/r/68329/diff/1/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 68331: Windows: Made `libwinio` the default option.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68331/#review207639 --- Ship it! Ship It! - Akash Gupta On Aug. 13, 2018, 10:35 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68331/ > --- > > (Updated Aug. 13, 2018, 10:35 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Mahler, and Joseph Wu. > > > Bugs: MESOS-9084 > https://issues.apache.org/jira/browse/MESOS-9084 > > > Repository: mesos > > > Description > --- > > With no options specified, Windows will now build with the > `libwinio` (native Windows Thread API eventing library) by > default (similar to POSIX defaulting to `libev` without options). To > use `libevent` (NOT recommended), it must be explicitly enabled, and > will emit an appropriate warning. Furthermore, the tests which were > previously excluded when `libevent` was enabled are now enabled on > Windows unconditionally. This serves two reasons: (1) it simplifies > the build logic and (2) by failing with `libevent` it demonstrates why > it is not recommended (and avoids giving false impressions). > > > Diffs > - > > 3rdparty/CMakeLists.txt 629b1968775da4d128b8d36c77d75efa303b0b7c > cmake/CompilationConfigure.cmake 10cacfb99e2cff1ddd2285ae441730f61182e06d > docs/windows.md f0f12fc81ed664528cf7496cf75e1cb11eee6013 > src/tests/CMakeLists.txt fed072a8a761c6b9e65b2c75f2e92facd8353f7c > > > Diff: https://reviews.apache.org/r/68331/diff/1/ > > > Testing > --- > > Built with and without `-DENABLE_LIBEVENT` on Windows. > > > Thanks, > > Andrew Schwartzmeyer > >
Review Request 67832: Changed docker subprocess calls to use exec form.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67832/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Repository: mesos Description --- The docker executor used the command line subprocess form even it only ever called the docker executable. The command line form made the executor fail if the `--docker=` path contained spaces. Diffs - src/docker/docker.hpp 25d9ca662fa5d99b32c668a5fdfc75584132cc38 src/docker/docker.cpp baac70f25ef3f944541341822aacb6a395853113 src/tests/containerizer/docker_tests.cpp 7097efc716ab0e1f34d5a1a35d8e0e173b113c91 Diff: https://reviews.apache.org/r/67832/diff/1/ Testing --- Tested by running `HungDockerTest` on a path with a space. Waiting for Windows CI for verification. On Linux, ran mesos-tests. Thanks, Akash Gupta
Re: Review Request 67685: Windows: Added `windows_to_unix_epoch` function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67685/ --- (Updated June 23, 2018, 12:22 a.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- moved file. Bugs: MESOS-8668 and MESOS-8671 https://issues.apache.org/jira/browse/MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8671 Repository: mesos Description --- There are a couple of places where we need to convert a Windows absolute time (epoch at 01/01/1601) to a UNIX absolute time (epoch at 01/01/1970). So, a function has been added to handle it. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/stat.hpp 7838bacabc4e2b9c92a5283f95543ee74b2d6505 3rdparty/stout/include/stout/windows/os.hpp 46cd667e88853fb0945ec5b14bd57319666895fc Diff: https://reviews.apache.org/r/67685/diff/2/ Changes: https://reviews.apache.org/r/67685/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 67391: Windows: Added CMake `ENABLE_LIBWINIO` flag for Windows IOCP backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67391/ --- (Updated June 21, 2018, 11:58 a.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Addressed feedback. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Added Windows IOCP backend to the build system. Now, there are two async backends, which are libevent through `ENABLE_LIBEVENT` and the Windows IOCP (libwinio) through `ENABLE_LIBWINIO`. Diffs (updated) - 3rdparty/CMakeLists.txt 367df4b1318167aef3aeba60a31e87eee65f7a71 3rdparty/libprocess/include/process/io.hpp cc2caf44e065bed40263f3820e95a4f7c378bb98 3rdparty/libprocess/src/CMakeLists.txt 619183eff6d6d301a011ab03f007410f50a0aa4f cmake/CompilationConfigure.cmake 2f92acb3a140faa48e3639c7000be7f43020ad7d Diff: https://reviews.apache.org/r/67391/diff/2/ Changes: https://reviews.apache.org/r/67391/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 67390: Windows: Integrated libwinio with libprocess code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67390/ --- (Updated June 21, 2018, 11:56 a.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- file rename and feedback. Bugs: MESOS-8668, MESOS-8671 and MESOS-8672 https://issues.apache.org/jira/browse/MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8671 https://issues.apache.org/jira/browse/MESOS-8672 Repository: mesos Description --- The libprocess eventloops are used to implement the `EventLoop`, SocketImpl` and `io::read/write` interfaces. Thus, by providing those implementations using libwinio, libprocess now supports the libwinio backend. Diffs (updated) - 3rdparty/libprocess/src/windows/event_loop.hpp PRE-CREATION 3rdparty/libprocess/src/windows/event_loop.cpp PRE-CREATION 3rdparty/libprocess/src/windows/io.cpp PRE-CREATION 3rdparty/libprocess/src/windows/poll_socket.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67390/diff/2/ Changes: https://reviews.apache.org/r/67390/diff/1-2/ Testing --- Thanks, Akash Gupta
Review Request 67685: Windows: Added `windows_to_unix_epoch` function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67685/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668 and MESOS-8671 https://issues.apache.org/jira/browse/MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8671 Repository: mesos Description --- There are a couple of places where we need to convert a Windows absolute time (epoch at 01/01/1601) to a UNIX absolute time (epoch at 01/01/1970). So, a function has been added to handle it. Diffs - 3rdparty/stout/include/stout/os/windows/stat.hpp 7838bacabc4e2b9c92a5283f95543ee74b2d6505 3rdparty/stout/include/stout/windows.hpp 075ad54ac694a11cb1e981a499e99c02cb734bc6 Diff: https://reviews.apache.org/r/67685/diff/1/ Testing --- Thanks, Akash Gupta
Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67388/ --- (Updated June 21, 2018, 11:47 a.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Reworked the file organization. Joesph / Andy please take a look. Summary (updated) - Moved libprocess POSIX and Windows implementations to separate folders. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description (updated) --- In preparation of the new Windows IOCP library, the POSIX and Windows specific files in libprocess have been moved to their own directories for better code organization. Diffs (updated) - 3rdparty/libprocess/Makefile.am 8910416ce4313a0d70721cf1bb1d1453aaf691f9 3rdparty/libprocess/src/CMakeLists.txt 619183eff6d6d301a011ab03f007410f50a0aa4f 3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 3rdparty/libprocess/src/libev.hpp 3rdparty/libprocess/src/libev.cpp 3rdparty/libprocess/src/libev_poll.cpp 3rdparty/libprocess/src/libevent.hpp 3rdparty/libprocess/src/libevent.cpp 3rdparty/libprocess/src/libevent_poll.cpp 3rdparty/libprocess/src/libevent_ssl_socket.hpp 3rdparty/libprocess/src/libevent_ssl_socket.cpp 3rdparty/libprocess/src/poll_socket.cpp 3rdparty/libprocess/src/posix/io.cpp PRE-CREATION 3rdparty/libprocess/src/socket.cpp 504cb541785650d2d05aabd25f5258b9bad52baa 3rdparty/libprocess/src/subprocess.cpp 0b2c02a9651563961532fdd5ab0f6d558f69f74e 3rdparty/libprocess/src/subprocess_posix.hpp 3rdparty/libprocess/src/subprocess_posix.cpp 3rdparty/libprocess/src/subprocess_windows.hpp 3rdparty/libprocess/src/subprocess_windows.cpp Diff: https://reviews.apache.org/r/67388/diff/2/ Changes: https://reviews.apache.org/r/67388/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 67384: Windows: Made socket `int_fd` castable to `HANDLE` type.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67384/ --- (Updated June 21, 2018, 11:34 a.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- address feedback. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Many Win32 functions that take in `HANDLE` also work on `SOCKET`, such as `ReadFile` or `CreateIoCompletionPort`. IOCP on Windows uses the `HANDLE` type uniformly, so we need to be able to convert socket `int_fd` to `HANDLE`. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/fd.hpp 89a037af31ad68a275d2519afbe4f161b23efe91 Diff: https://reviews.apache.org/r/67384/diff/2/ Changes: https://reviews.apache.org/r/67384/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 67587: Updated ZooKeeper retry logic to retry on `ENOENT` too.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67587/#review205027 --- Ship it! Ship It! - Akash Gupta On June 13, 2018, 10:30 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67587/ > --- > > (Updated June 13, 2018, 10:30 p.m.) > > > Review request for mesos, Joseph Wu and Neil Conway. > > > Bugs: MESOS-3790 > https://issues.apache.org/jira/browse/MESOS-3790 > > > Repository: mesos > > > Description > --- > > Per MESOS-3790, the call to `zookeeper_init` maps `EAI_NONAME` and > `EAI_NODATA` to an `errno` value of `ENOENT`, and all others except > `EAI_MEMORY` to `EINVAL`. Mesos's ZooKeeper logic is written to retry > this initialization for ten minutes if the error is `EINVAL`, and > should be updated to also retry if the error is `ENOENT`. > > This is necessary because if the initialization is not retried, the > process crashes due to the `PLOG(FATAL)` call, and if it crashes, it > will interrupt other Mesos threads and potentially leave the > environment in an unknown state. For instance, we have seen > intermittent failures where the systemd unit file > `mesos_executors.slice` is created but empty because Mesos crashed > between creating the file and flushing the write to the file. This > then leads to errors when the agent is restarted (and succeeds to > connect to ZooKeeper), because the agent explicitly does not attempt > to write to the unit file if it already exists. > > > Diffs > - > > src/zookeeper/zookeeper.cpp 52c4af192ccd1361afc4f7a0041889238c01e674 > > > Diff: https://reviews.apache.org/r/67587/diff/1/ > > > Testing > --- > > Testing against our repro right now, but it's flaky, so it'll take a while. > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 67394: White list fds that child processes can inherit in mesos containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67394/#review204523 --- src/tests/containerizer/launcher.hpp Line 60 (original), 60 (patched) <https://reviews.apache.org/r/67394/#comment287066> Just curious but why is `forkImpl` needed instead of the old way? - Akash Gupta On May 31, 2018, 10:50 p.m., Radhika Jandhyala wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67394/ > --- > > (Updated May 31, 2018, 10:50 p.m.) > > > Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jie Yu, and Li > Li. > > > Repository: mesos > > > Description > --- > > White list fds that child processes can inherit in mesos containerizer. > > > Diffs > - > > src/slave/containerizer/mesos/containerizer.cpp > eac1d16f2388385fec04ff8f013ce0ebf4e97f0f > src/slave/containerizer/mesos/launcher.hpp > f69d934d2e1a129e10df8c7f5c78723e832adc7d > src/slave/containerizer/mesos/launcher.cpp > 2fe47d368cb82a46328e1f636baa836272db244c > src/slave/containerizer/mesos/linux_launcher.hpp > 0ea9b875ae46cadea483bc8dd8bf4907fd324dc9 > src/slave/containerizer/mesos/linux_launcher.cpp > 80e444501e429c1e1ae354abcd51f86430316ada > src/tests/containerizer/launcher.hpp > a8e436f164b67d937ebcff35e084d3ca755c003c > src/tests/containerizer/launcher.cpp > a92d9890f0931425d69ef8ce0896d081b8722079 > src/tests/containerizer/mesos_containerizer_tests.cpp > 01f2b38cfa67b144298c361e92170322864ac201 > > > Diff: https://reviews.apache.org/r/67394/diff/1/ > > > Testing > --- > > All mesos tests on windows > > > Thanks, > > Radhika Jandhyala > >
Re: Review Request 67287: White list fds that child processes can inherit in libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67287/#review204522 --- Ship it! Ship It! - Akash Gupta On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67287/ > --- > > (Updated May 24, 2018, 10:47 p.m.) > > > Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie > Yu, Li Li, and Radhika Jandhyala. > > > Bugs: MESOS-8926 > https://issues.apache.org/jira/browse/MESOS-8926 > > > Repository: mesos > > > Description > --- > > White list fds that child processes can inherit in libprocess. > > > Diffs > - > > 3rdparty/libprocess/include/process/subprocess.hpp > 6a1262340c333b617402637e648c12769827ffc4 > 3rdparty/libprocess/src/subprocess.cpp > d7a725363251f9c54072cd7551f5598696938308 > 3rdparty/libprocess/src/subprocess_windows.hpp > c7ed0ad18f5b46a1d5ac2a6e51883aefb7c1692f > > > Diff: https://reviews.apache.org/r/67287/diff/2/ > > > Testing > --- > > All Mesos-tests > > > Thanks, > > Radhika Jandhyala > >
Re: Review Request 67286: White list fds that child processes can inherit in stout.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67286/#review204521
---
Fix it, then Ship it!
3rdparty/stout/include/stout/internal/windows/inherit.hpp
Lines 55 (patched)
<https://reviews.apache.org/r/67286/#comment287064>
Note that this destructor is called regardless of the state of
`attribute_list`. So, it will call `::DeleteProcThreadAttributeList()` for
these two cases, where `::InitializeProcThreadAttributeList` hasn't succeeded
yet.
```
if (attribute_list == nullptr) {
return WindowsError(ERROR_OUTOFMEMORY);
// Destructor calls `::DeleteProc...(nullptr)`
}
result =
::InitializeProcThreadAttributeList(attribute_list.get(), 1, 0, );
if (result == FALSE) {
return WindowsError();
// Destructor calls `::DeleteProc...()` on uninitialized attribute_list.
}
```
3rdparty/stout/include/stout/os/windows/shell.hpp
Lines 245 (patched)
<https://reviews.apache.org/r/67286/#comment287065>
I think the mesos empty vector style is `const std::vector&
whitelist_fds = {}` .
- Akash Gupta
On May 24, 2018, 10:47 p.m., Radhika Jandhyala wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67286/
> ---
>
> (Updated May 24, 2018, 10:47 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Eric Mumau, Jie
> Yu, Li Li, and Radhika Jandhyala.
>
>
> Bugs: MESOS-8926
> https://issues.apache.org/jira/browse/MESOS-8926
>
>
> Repository: mesos
>
>
> Description
> ---
>
> White list fds that child processes can inherit in stout.
>
>
> Diffs
> -
>
> 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> 7dbde820e775cbaeb8db4bc4559ab432903e75ea
> 3rdparty/stout/include/stout/os/windows/shell.hpp
> 8da612af2888ff4d4d458ea5b16cdb08779b6f4c
>
>
> Diff: https://reviews.apache.org/r/67286/diff/2/
>
>
> Testing
> ---
>
> All Mesos-tests
>
>
> Thanks,
>
> Radhika Jandhyala
>
>
Re: Review Request 67465: Windows: Log a fatal error if `WindowsFD(int)` is incorrectly used.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67465/#review204502 --- Ship it! Ship It! - Akash Gupta On June 7, 2018, 9:59 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67465/ > --- > > (Updated June 7, 2018, 9:59 p.m.) > > > Review request for mesos, Akash Gupta and Joseph Wu. > > > Repository: mesos > > > Description > --- > > This constructor _should_ be `explicit` as the other constructors are; > however, for historical reasons it is implicit in order to support the > semantics of assigning from `-1, 0, 1, 2`. Unfortunately, its > implicitness leads to subtle bugs. For instance, the code `int_fd s = > ::socket(...)` on Windows compiles, but behaves incorrectly because > the RHS `SOCKET` type is implicitly converted to an `int` to satisfy > the only available implicit constructor, since the `WindowsFD(SOCKET)` > constructor is marked `explicit`. As we should never construct off any > of these values, but cannot constrain it at compile-time, we should > add a fatal runtime error instead. > > > Diffs > - > > 3rdparty/stout/include/stout/os/windows/fd.hpp > 5dbdff2680370d123579c5e3fdd9b0e0adaf512e > > > Diff: https://reviews.apache.org/r/67465/diff/2/ > > > Testing > --- > > Still building, but this is what I propose. This is better than subtle sad > panda bugs. > > > Thanks, > > Andrew Schwartzmeyer > >
Review Request 67457: Windows: Ported docker_containerizer_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67457/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Bugs: MESOS-7342 https://issues.apache.org/jira/browse/MESOS-7342 Repository: mesos Description --- With some Docker bug fixes and the IOCP backend, the remaining docker have been ported. The only remaining Docker tests that aren't ported are either due to limitations on Windows Containers or unimplemented features in Mesos (Persistent volume and hooks). Diffs - src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 src/tests/containerizer/docker_containerizer_tests.cpp 194308bf9d09a3562e683c49e0da4c9a6463d66e Diff: https://reviews.apache.org/r/67457/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67456: Windows: Wrapped docker executor code in job object.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67456/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Bugs: MESOS-7342 https://issues.apache.org/jira/browse/MESOS-7342 Repository: mesos Description --- One of the docker tests revealed that the docker executor code could leak some of the docker processes if it gets killed. To avoid this, we run the docker executor and all processes launched by it in a job object, so that `os::killtree` will properly kill the process tree. Diffs - src/docker/docker.cpp 1332d4b8cc8dc48abd5f07689b18a5b39ca946cf src/docker/executor.cpp 572594a110aa4b51a698b176287b6ff823b2dd07 src/slave/containerizer/docker.cpp 391700f9698d0658b9273d79857bfa30bf3549be Diff: https://reviews.apache.org/r/67456/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67455: Windows: Fixed incorrect return code for `os:kill`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67455/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Bugs: MESOS-8666 https://issues.apache.org/jira/browse/MESOS-8666 Repository: mesos Description --- `os::kill` was returning the opposite error code than expected. It was returning `KILL_FAIL` on success and `KILL_PASS` on error. Diffs - 3rdparty/stout/include/stout/os/windows/kill.hpp bdb83510b7f0529d41f8e895451a941dc22d21bb Diff: https://reviews.apache.org/r/67455/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67395: Fixed socket creation bug in docker.cpp.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67395/
---
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Repository: mesos
Description
---
On Windows, the statement `int_fd socket = ::socket(...);` would
implictly call the `WindowsFD(int crt)` constructor. Since that
contstructor only accepts values of {0, 1, 2}, it would incorrectly
mark the socket as invalid. The code has been changed to use the stout
network functions, which properly construct the `int_fd`.
Diffs
-
src/slave/containerizer/docker.cpp fc032367400119dc827657d0e6e859d18ebdbb16
Diff: https://reviews.apache.org/r/67395/diff/1/
Testing
---
This bug was found when testing recovery in our Windows cluster and this patch
was confirmed to fix it.
Thanks,
Akash Gupta
Re: Review Request 67389: Windows: Implemented Windows IOCP async backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67389/#review204073 --- 3rdparty/libprocess/src/libwinio_impl.cpp Lines 38 (patched) <https://reviews.apache.org/r/67389/#comment286462> Radhika/Eric do you know a good number for this? I i was thinking of around $NCPUS. - Akash Gupta On May 30, 2018, 6:54 p.m., Akash Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67389/ > --- > > (Updated May 30, 2018, 6:54 p.m.) > > > Review request for mesos. > > > Bugs: MESOS-8668, MESOS-8671 and MESOS-8672 > https://issues.apache.org/jira/browse/MESOS-8668 > https://issues.apache.org/jira/browse/MESOS-8671 > https://issues.apache.org/jira/browse/MESOS-8672 > > > Repository: mesos > > > Description > --- > > The Windows IOCP async backend, called libwinio, provides a single > threaded event loop implementation that allows for async IO and timer > events. libwinio wraps the native Win32 async functions using > libprocess's primitives, which makes it easier to use and more type > safe. > > > Diffs > - > > 3rdparty/libprocess/src/CMakeLists.txt > cf443dffd0663ecf02b7efd6f7094175b94aae19 > 3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION > 3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/67389/diff/1/ > > > Testing > --- > > > Thanks, > > Akash Gupta > >
Re: Review Request 67389: Windows: Implemented Windows IOCP async backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67389/#review204071 --- 3rdparty/libprocess/src/libwinio_impl.cpp Lines 389 (patched) <https://reviews.apache.org/r/67389/#comment286460> Oops! Will delete this commented out code. - Akash Gupta On May 30, 2018, 6:54 p.m., Akash Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67389/ > --- > > (Updated May 30, 2018, 6:54 p.m.) > > > Review request for mesos. > > > Bugs: MESOS-8668, MESOS-8671 and MESOS-8672 > https://issues.apache.org/jira/browse/MESOS-8668 > https://issues.apache.org/jira/browse/MESOS-8671 > https://issues.apache.org/jira/browse/MESOS-8672 > > > Repository: mesos > > > Description > --- > > The Windows IOCP async backend, called libwinio, provides a single > threaded event loop implementation that allows for async IO and timer > events. libwinio wraps the native Win32 async functions using > libprocess's primitives, which makes it easier to use and more type > safe. > > > Diffs > - > > 3rdparty/libprocess/src/CMakeLists.txt > cf443dffd0663ecf02b7efd6f7094175b94aae19 > 3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION > 3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/67389/diff/1/ > > > Testing > --- > > > Thanks, > > Akash Gupta > >
Review Request 67385: Windows: Added IOCP `HANDLE` to `WindowsFD`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67385/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-5371 and MESOS-8668 https://issues.apache.org/jira/browse/MESOS-5371 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Mesos/Libprocess uses `os::nonblock()` in a way that assumes that it is idempotent. Unfortunately, `CreateIoCompletionPort` is not idempotent, so we need to wrap the function around our own code to make it so. We need to keep it inside the `WindowsFD` class, because there isn't a way to determine if a HANDLE is associated with an IOCP through the Win32 API. Diffs - 3rdparty/stout/include/stout/os/windows/dup.hpp 5bda095e676b038cdaea04f7be23ba2a1aca9015 3rdparty/stout/include/stout/os/windows/fd.hpp 5dbdff2680370d123579c5e3fdd9b0e0adaf512e Diff: https://reviews.apache.org/r/67385/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67386: Added `io::prepare_async` and `io::is_async` functions for libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67386/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-5371 and MESOS-8668 https://issues.apache.org/jira/browse/MESOS-5371 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- `os::nonblock` and `os::isNonBlock` are stubs that only do things for Windows sockets. For the IOCP implementation, we need to associate the handles with the IOCP hande, which is created by libprocess. Thus, we need these new functions to prepare the handle for the async IO functions by either setting the hande to non-blocking on UNIX systems or by associating it with an IOCP handle on Windows. Diffs - 3rdparty/libprocess/include/process/io.hpp cc2caf44e065bed40263f3820e95a4f7c378bb98 3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 3rdparty/libprocess/src/socket.cpp 504cb541785650d2d05aabd25f5258b9bad52baa 3rdparty/libprocess/src/tests/subprocess_tests.cpp 568d77b94b3afef8c1ebdde71e14aa0b498da5c8 Diff: https://reviews.apache.org/r/67386/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67387: Updated Mesos code to use `io::prepare_async`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67387/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-5371 and MESOS-8668 https://issues.apache.org/jira/browse/MESOS-5371 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Due to the previous patch, instances that use `os::nonblock` before sending the file descriptor to the async IO functions now need to use `io::prepare_async`. Diffs - src/files/files.cpp 8d896ed59e88de86149e00ba80806b61aba15c86 src/slave/container_loggers/logrotate.cpp bd41912fdf9faa70957a768ea5541e60824864ba Diff: https://reviews.apache.org/r/67387/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67388: Moved: `io::internal::read/write` to separate file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67388/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- The implementation in `io::internal::read/write` uses `io::poll`, which is UNIX specific. The Windows IOCP implementation will not use a polling function, since there no unified mechanism, so the functions have been moved to their own file. Diffs - 3rdparty/libprocess/Makefile.am d434001fbc49d337b6e29f6ac8c9c7475922a819 3rdparty/libprocess/src/CMakeLists.txt cf443dffd0663ecf02b7efd6f7094175b94aae19 3rdparty/libprocess/src/io.cpp 97f2b17092fbd23528cf3220fee5927a1ec38aba 3rdparty/libprocess/src/io_internal.hpp PRE-CREATION 3rdparty/libprocess/src/poll_io.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67388/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67389: Windows: Implemented Windows IOCP async backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67389/ --- Review request for mesos. Bugs: MESOS-8668, MESOS-8671 and MESOS-8672 https://issues.apache.org/jira/browse/MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8671 https://issues.apache.org/jira/browse/MESOS-8672 Repository: mesos Description --- The Windows IOCP async backend, called libwinio, provides a single threaded event loop implementation that allows for async IO and timer events. libwinio wraps the native Win32 async functions using libprocess's primitives, which makes it easier to use and more type safe. Diffs - 3rdparty/libprocess/src/CMakeLists.txt cf443dffd0663ecf02b7efd6f7094175b94aae19 3rdparty/libprocess/src/libwinio_impl.hpp PRE-CREATION 3rdparty/libprocess/src/libwinio_impl.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67389/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67390: Windows: Integrated libwinio with libprocess code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67390/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668, MESOS-8671 and MESOS-8672 https://issues.apache.org/jira/browse/MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8671 https://issues.apache.org/jira/browse/MESOS-8672 Repository: mesos Description --- The libprocess eventloops are used to implement the `EventLoop`, SocketImpl` and `io::read/write` interfaces. Thus, by providing those implementations using libwinio, libprocess now supports the libwinio backend. Diffs - 3rdparty/libprocess/src/libwinio.hpp PRE-CREATION 3rdparty/libprocess/src/libwinio_eventloop.cpp PRE-CREATION 3rdparty/libprocess/src/libwinio_io.cpp PRE-CREATION 3rdparty/libprocess/src/libwinio_socket.cpp PRE-CREATION Diff: https://reviews.apache.org/r/67390/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67391: Windows: Added CMake `ENABLE_LIBWINIO` flag for Windows IOCP backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67391/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Added Windows IOCP backend to the build system. Now, there are two async backends, which are libevent through `ENABLE_LIBEVENT` and the Windows IOCP (libwinio) through `ENABLE_LIBWINIO`. Diffs - 3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04 3rdparty/libprocess/include/process/io.hpp cc2caf44e065bed40263f3820e95a4f7c378bb98 3rdparty/libprocess/src/CMakeLists.txt cf443dffd0663ecf02b7efd6f7094175b94aae19 cmake/CompilationConfigure.cmake 2f92acb3a140faa48e3639c7000be7f43020ad7d Diff: https://reviews.apache.org/r/67391/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67392: Windows: Made PipeLargeOutput test work with IOCP backend.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67392/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- THe PipeLargeOutput test was calling `io::read`, which was returning the wrong error since it operates at a higher level abstraction. The test really just checks if a pipe EOF returns `ERROR_BROKEN_PIPE`, so the test was changed to use the lower level `os::read`. Diffs - 3rdparty/libprocess/src/tests/subprocess_tests.cpp 568d77b94b3afef8c1ebdde71e14aa0b498da5c8 Diff: https://reviews.apache.org/r/67392/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67393: Windows: Ported io_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67393/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-5815 https://issues.apache.org/jira/browse/MESOS-5815 Repository: mesos Description --- With the IOCP backened implemented, io_tests.cpp is now ported to Windows. Since the tests require async pipe IO, the tests are only run if Mesos was compiled with the IOCP backened. Diffs - 3rdparty/libprocess/src/tests/CMakeLists.txt 8c1353b442d5fc7b2e796a4d87f5b41389d4e1c3 3rdparty/libprocess/src/tests/io_tests.cpp de7272b3ef0f414f056509448856c6bbd86ef75c Diff: https://reviews.apache.org/r/67393/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67384: Windows: Made socket `int_fd` castable to `HANDLE` type.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67384/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8668 https://issues.apache.org/jira/browse/MESOS-8668 Repository: mesos Description --- Many Win32 functions that take in `HANDLE` also work on `SOCKET`, such as `ReadFile` or `CreateIoCompletionPort`. IOCP on Windows uses the `HANDLE` type uniformly, so we need to be able to convert socket `int_fd` to `HANDLE`. Diffs - 3rdparty/stout/include/stout/os/windows/fd.hpp 5dbdff2680370d123579c5e3fdd9b0e0adaf512e Diff: https://reviews.apache.org/r/67384/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67313: Windows: Fixed `ROOT_DOCKER_DockerHealthStatusChange` test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67313/ --- Review request for mesos, Andrew Schwartzmeyer and Joseph Wu. Repository: mesos Description --- The `ROOT_DOCKER_DockerHealthStatusChange` was broken if the test ran in a drive other than `C:`, since only `C:` exists in the docker container with the default settings. The test was changed to hardcode the `C:` drive on Windows. Diffs - src/tests/health_check_tests.cpp 7e8f86c903dfad73c2102eaf491fb2adcb7c11cf Diff: https://reviews.apache.org/r/67313/diff/1/ Testing --- Thanks, Akash Gupta
Re: Review Request 66962: Windows: Added tests for async IO functions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66962/ --- (Updated May 22, 2018, 10:55 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Addressed feedback. Bugs: MESOS-8670 https://issues.apache.org/jira/browse/MESOS-8670 Repository: mesos Description --- New asynchronous functions were introduced to the Windows stout read and write implementations, so eztra tests were added. Diffs (updated) - 3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a Diff: https://reviews.apache.org/r/66962/diff/2/ Changes: https://reviews.apache.org/r/66962/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66961/ --- (Updated May 22, 2018, 10:54 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Addressed feedback. Bugs: MESOS-3444 and MESOS-8681 https://issues.apache.org/jira/browse/MESOS-3444 https://issues.apache.org/jira/browse/MESOS-8681 Repository: mesos Description (updated) --- The sendfile tests were using socketpair, but the rest of the tests were crossplatform. By providing a Windows socketpair implementations, the tests are now ported. Diffs (updated) - 3rdparty/stout/tests/CMakeLists.txt 43c5e873a3a3b8f79f0f888a450e186c6123d938 3rdparty/stout/tests/os/sendfile_tests.cpp 05966ae067ae3972598da3370eb16fdce5736c21 Diff: https://reviews.apache.org/r/66961/diff/2/ Changes: https://reviews.apache.org/r/66961/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66960: Windows: Added async version of `os::sendfile`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66960/ --- (Updated May 22, 2018, 10:53 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Addressed feedback. Bugs: MESOS-8681 https://issues.apache.org/jira/browse/MESOS-8681 Repository: mesos Description --- Broke the original `os::sendfile` implementation into an asynchronous `os::sendfile_async`function and a synchronous `os::sendfile` function. The synchronous version simply calls the asynchronous version and waits. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/sendfile.hpp 594d9c7beeff2766f59f16ba25986314f2f7ceb8 Diff: https://reviews.apache.org/r/66960/diff/2/ Changes: https://reviews.apache.org/r/66960/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66957: Windows: Enabled creating overlapped pipes with `os::pipe`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66957/ --- (Updated May 22, 2018, 10:51 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Ran clang-format and updated with Andy's suggestions. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- `os::pipe` was using `CreatePipe`, which does not support overlapped io. Now, the implementation of `os::pipe` has been changed to use `CreateNamedPipe`, which supports overlapped IO. The named pipe is created with an unique random name, so it is effectively anonymous. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/pipe.hpp a3574fd6f2ff1608396b47cad8cbed88134a74ca Diff: https://reviews.apache.org/r/66957/diff/2/ Changes: https://reviews.apache.org/r/66957/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66958: Windows: Fixed inheritance in subprocess_windows.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66958/ --- (Updated May 22, 2018, 10:51 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Rebased. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- subprocess_windows.cpp was creating inheritable pipes and files. On Windows, our policy is to make everything not inheritable and then have `CreateProcessW` set the the right pipes to be inheritable to avoid any handle leaks. Now, subprocess_windows.cpp follows that inheritance policy. Diffs (updated) - 3rdparty/libprocess/src/subprocess_windows.cpp a1e6425faff01f816748f0b8b5307612b6dd8302 Diff: https://reviews.apache.org/r/66958/diff/2/ Changes: https://reviews.apache.org/r/66958/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66956: Windows: Added overlapped support to `os::write`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66956/ --- (Updated May 22, 2018, 10:46 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Updated with Andy's feedback. Bugs: MESOS-8670 https://issues.apache.org/jira/browse/MESOS-8670 Repository: mesos Description --- Added overlapped handle support to `os::write`. Now, `os::write` will do a blocking write no matter the handle type. Also, `os::write_async` will do an overlapped write on an overlapped handle. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/write.hpp 295c031c5824d13d74e2c9006e62c391d5020f69 Diff: https://reviews.apache.org/r/66956/diff/2/ Changes: https://reviews.apache.org/r/66956/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 66955: Windows: Added overlapped support to `os::read`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66955/ --- (Updated May 22, 2018, 10:40 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Updated read with Andy's suggestions. Bugs: MESOS-8670 https://issues.apache.org/jira/browse/MESOS-8670 Repository: mesos Description --- Added overlapped handle support to `os::read`. Now, `os::read` will do a blocking read no matter the handle type. Also, `os::read_async` will do an overlapped read on an overlapped handle. Diffs (updated) - 3rdparty/stout/include/stout/os/windows/read.hpp e957da81e55867b260d356f035d98918b85d1965 Diff: https://reviews.apache.org/r/66955/diff/2/ Changes: https://reviews.apache.org/r/66955/diff/1-2/ Testing --- Thanks, Akash Gupta
Re: Review Request 67243: Windows: Changed test image to custom nanoserver.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67243/ --- (Updated May 22, 2018, 1:39 a.m.) Review request for mesos and Andrew Schwartzmeyer. Changes --- Updated outdated comment. Bugs: MESOS-8499 https://issues.apache.org/jira/browse/MESOS-8499 Repository: mesos Description --- The test docker images was based off of the official nanoserver powershell image that hasn't been updated for 1803 yet. Since we don't actually need powershell for the docker tests, the image has been changed to a custom one based off the official nanoserver image, which is updated for 1803. Diffs (updated) - src/tests/containerizer/docker_common.hpp 172eea31ac6006b4b624fb04a64ed4345bd9268d src/tests/containerizer/docker_tests.cpp e4660f7a931792f33db7978870d215d2d16fa65d src/tests/health_check_tests.cpp c4661b4f005c18a7cdc69ce05576bcccb79d5dee Diff: https://reviews.apache.org/r/67243/diff/2/ Changes: https://reviews.apache.org/r/67243/diff/1-2/ Testing --- This was tested in Windows Server 1709 & 1803. Also, tested on Linux. Thanks, Akash Gupta
Review Request 67243: Windows: Changed test image to custom nanoserver.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67243/ --- Review request for mesos and Andrew Schwartzmeyer. Bugs: MESOS-8499 https://issues.apache.org/jira/browse/MESOS-8499 Repository: mesos Description --- The test docker images was based off of the official nanoserver powershell image that hasn't been updated for 1803 yet. Since we don't actually need powershell for the docker tests, the image has been changed to a custom one based off the official nanoserver image, which is updated for 1803. Diffs - src/tests/containerizer/docker_common.hpp 172eea31ac6006b4b624fb04a64ed4345bd9268d src/tests/containerizer/docker_tests.cpp e4660f7a931792f33db7978870d215d2d16fa65d src/tests/health_check_tests.cpp c4661b4f005c18a7cdc69ce05576bcccb79d5dee Diff: https://reviews.apache.org/r/67243/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67208: Windows: Updated information on docker health checks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67208/ --- Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and Gaston Kleiman. Bugs: MESOS-8499 https://issues.apache.org/jira/browse/MESOS-8499 Repository: mesos Description --- The previous commit changed how the health checks work on Windows, so this commit updates the documentation to reflect on those changes. Diffs - docs/health-checks.md c6f7144e9f62bf38799e1c66297f405be6fd5b30 Diff: https://reviews.apache.org/r/67208/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 67207: Windows: Changed health check image to be an environment variable.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67207/ --- Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and Gaston Kleiman. Bugs: MESOS-8499 https://issues.apache.org/jira/browse/MESOS-8499 Repository: mesos Description --- To support the health check on different Windows hosts (1709, 1803...), the health check image is now passed in as an environment variable instead of being hardcoded. The commands have also been changed to use the same one as Linux (`curl` and `mesos-tcp-connect`), since the image is now controlled by the user. Diffs - src/checks/checker_process.hpp 228ac78715f120b43db05b5691412e8f5520de3b src/checks/checker_process.cpp 7e484510b4fc2b94d4d4385dfa8f68bdd76e6093 src/tests/environment.cpp a30592ac6b0002dad0947086ecbfdf4e2db62da5 src/tests/health_check_tests.cpp c4661b4f005c18a7cdc69ce05576bcccb79d5dee Diff: https://reviews.apache.org/r/67207/diff/1/ Testing --- Thanks, Akash Gupta
Re: Review Request 66961: Windows: Ported sendfile_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66961/ --- (Updated May 7, 2018, 4:15 p.m.) Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Changes --- Added another bug. Bugs: MESOS-3444 and MESOS-8681 https://issues.apache.org/jira/browse/MESOS-3444 https://issues.apache.org/jira/browse/MESOS-8681 Repository: mesos Description --- The sendfile tests were using `socketpair`, but the rest of the tests were crossplatform. By providing a Windows `socketpair` implementations, the tests are now ported. Diffs - 3rdparty/stout/tests/CMakeLists.txt 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 3rdparty/stout/tests/os/sendfile_tests.cpp 05966ae067ae3972598da3370eb16fdce5736c21 Diff: https://reviews.apache.org/r/66961/diff/1/ Testing --- Thanks, Akash Gupta
Re: Review Request 66962: Windows: Added tests for async IO functions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66962/#review202469 --- By the way, this patch set is roughly half of the changes for the IOCP backend. These focus mainly on the stout changes. I'm currently cleaning up some libprocess code so that I can post the remaining half. - Akash Gupta On May 4, 2018, 5:27 p.m., Akash Gupta wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66962/ > --- > > (Updated May 4, 2018, 5:27 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, > John Kordich, Joseph Wu, and Radhika Jandhyala. > > > Bugs: MESOS-8670 > https://issues.apache.org/jira/browse/MESOS-8670 > > > Repository: mesos > > > Description > --- > > New asynchronous functions were introduced to the Windows stout read > and write implementations, so eztra tests were added. > > > Diffs > - > > 3rdparty/stout/tests/os/filesystem_tests.cpp > b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a > > > Diff: https://reviews.apache.org/r/66962/diff/1/ > > > Testing > --- > > > Thanks, > > Akash Gupta > >
Review Request 66962: Windows: Added tests for async IO functions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66962/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- New asynchronous functions were introduced to the Windows stout read and write implementations, so eztra tests were added. Diffs - 3rdparty/stout/tests/os/filesystem_tests.cpp b17ab9aaa94ae14b6707a9ac7a54b9db38615f2a Diff: https://reviews.apache.org/r/66962/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66954: Windows: Added overlapped aware io.hpp file for `os::read/write`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66954/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- Added a helper file for io, because `os::read/write` need to work on both non-overlapped and overlapped files, so the logic has gotten more complicated. The new file, `io.hpp`, has all the shared logic. Diffs - 3rdparty/stout/include/Makefile.am f2e60224d9c729497bbcfffbec989c4f355d8f85 3rdparty/stout/include/stout/os/windows/io.hpp PRE-CREATION Diff: https://reviews.apache.org/r/66954/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66956: Windows: Added overlapped support to `os::write`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66956/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- Added overlapped handle support to `os::write`. Now, `os::write` will do a blocking write no matter the handle type. Also, `os::write_async` will do an overlapped write on an overlapped handle. Diffs - 3rdparty/stout/include/stout/os/windows/write.hpp 295c031c5824d13d74e2c9006e62c391d5020f69 Diff: https://reviews.apache.org/r/66956/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66955: Windows: Added overlapped support to `os::read`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66955/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- Added overlapped handle support to `os::read`. Now, `os::read` will do a blocking read no matter the handle type. Also, `os::read_async` will do an overlapped read on an overlapped handle. Diffs - 3rdparty/stout/include/stout/os/windows/read.hpp e957da81e55867b260d356f035d98918b85d1965 Diff: https://reviews.apache.org/r/66955/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66957: Windows: Enabled creating overlapped pipes with `os::pipe`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66957/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- `os::pipe` was using `CreatePipe`, which does not support overlapped io. Now, the implementation of `os::pipe` has been changed to use `CreateNamedPipe`, which supports overlapped IO. The named pipe is created with an unique random name, so it is effectively anonymous. Diffs - 3rdparty/stout/include/stout/os/windows/pipe.hpp a3574fd6f2ff1608396b47cad8cbed88134a74ca Diff: https://reviews.apache.org/r/66957/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66958: Windows: Fixed inheritance in subprocess_windows.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66958/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- subprocess_windows.cpp was creating inheritable pipes and files. On Windows, our policy is to make everything not inheritable and then have `CreateProcessW` set the the right pipes to be inheritable to avoid any handle leaks. Now, subprocess_windows.cpp follows that inheritance policy. Diffs - 3rdparty/libprocess/src/subprocess_windows.cpp a1e6425faff01f816748f0b8b5307612b6dd8302 Diff: https://reviews.apache.org/r/66958/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66959: Windows: Fixed pipe inheritance in Mesos containerizer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66959/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- The Mesos containerizer needs an inheritable pipe that is not used for stdio of the child process. It is used for signaling when the child process should start running, so the pipe needs to be inheritable and not overlapped. Diffs - src/slave/containerizer/mesos/containerizer.cpp 01386ac3d36ec7a401b8d1be7834bc1f3fce55ef Diff: https://reviews.apache.org/r/66959/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66960: Windows: Added async version of `os::sendfile`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66960/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- Broke the original `os::sendfile` implementation into an asynchronous `os::sendfile_async`function and a synchronous `os::sendfile` function. The synchronous version simply calls the asynchronous version and waits. Diffs - 3rdparty/stout/include/stout/os/windows/sendfile.hpp 594d9c7beeff2766f59f16ba25986314f2f7ceb8 Diff: https://reviews.apache.org/r/66960/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66961: Windows: Ported sendfile_tests.cpp.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66961/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- The sendfile tests were using `socketpair`, but the rest of the tests were crossplatform. By providing a Windows `socketpair` implementations, the tests are now ported. Diffs - 3rdparty/stout/tests/CMakeLists.txt 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 3rdparty/stout/tests/os/sendfile_tests.cpp 05966ae067ae3972598da3370eb16fdce5736c21 Diff: https://reviews.apache.org/r/66961/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66953: Windows: Fixed dup to properly copy the overlapped WindowsFD.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66953/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- After the previous commit,`os::dup` was not copying the overlapped field, so it has been updated to copy it. Diffs - 3rdparty/stout/include/stout/os/windows/dup.hpp af98054f1bd9c8e55c52b246fda8734e3ca96e21 Diff: https://reviews.apache.org/r/66953/diff/1/ Testing --- Thanks, Akash Gupta
Review Request 66952: Windows: Added overlapped field to WindowsFD.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66952/ --- Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, Eric Mumau, John Kordich, Joseph Wu, and Radhika Jandhyala. Bugs: MESOS-8674 https://issues.apache.org/jira/browse/MESOS-8674 Repository: mesos Description --- Added an field to WindowsFD for functions that need to know if a Windows file is opened in overlapped mode, such as `os::read`, since Windows doesn't provide a Win32 API for it. Diffs - 3rdparty/stout/include/stout/os/windows/fd.hpp bab16e869e69c214e18de584d1615311316e001a 3rdparty/stout/include/stout/os/windows/open.hpp 701dcec7c4c1162d38d8f25596af29ed3b32691d Diff: https://reviews.apache.org/r/66952/diff/1/ Testing --- Thanks, Akash Gupta
Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66437/#review202138 --- Ship it! Ship It! - Akash Gupta 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 66836: Fixed `mesos-tcp-connect` to use `net::socket`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66836/#review202137 --- Ship it! Ship It! - Akash Gupta 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 66835: Replaced `int` and `HANDLE` types with `int_fd`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66835/#review202136 --- Ship it! Ship It! - Akash Gupta 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`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66834/#review202135 --- Ship it! Ship It! - Akash Gupta 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 66790: Windows: Ported the rest of the `SubprocessTest` suite.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66790/#review202133 --- Fix it, then Ship it! 3rdparty/libprocess/src/tests/subprocess_tests.cpp Line 234 (original), 233 (patched) <https://reviews.apache.org/r/66790/#comment283769> I think the first half of this test could be out of the #ifdef. - Akash Gupta On April 24, 2018, 11:18 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66790/ > --- > > (Updated April 24, 2018, 11:18 p.m.) > > > Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, and Joseph > Wu. > > > Bugs: MESOS-8833 > https://issues.apache.org/jira/browse/MESOS-8833 > > > Repository: mesos > > > Description > --- > > These tests mostly "just worked" on Windows, with only minor changes, > such as converting POSIX shell-script to Batch or PowerShell, and > expecting Windows line-endings and quoting semantics. > > > Diffs > - > > 3rdparty/libprocess/src/tests/subprocess_tests.cpp > be99bd6a88b2abcabed8d4d16763c2e29f38eb14 > > > Diff: https://reviews.apache.org/r/66790/diff/1/ > > > Testing > --- > > 13 more tests running on Windows :) > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66773: Windows: Fixed `os::stat::mtime()` to use `GetFileTime()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66773/#review202132 --- Fix it, then Ship it! 3rdparty/stout/include/stout/os/windows/stat.hpp Lines 186 (patched) <https://reviews.apache.org/r/66773/#comment283768> the comment should be `x / (10 * 1000 * 1000)` :) - Akash Gupta 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 66641: Added `FsTest.Open` to cover `os::open()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66641/#review202129 --- Ship it! Ship It! - Akash Gupta On April 16, 2018, 8:24 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66641/ > --- > > (Updated April 16, 2018, 8:24 p.m.) > > > Review request for mesos, Akash Gupta, John Kordich, and Joseph Wu. > > > Repository: mesos > > > Description > --- > > Added `FsTest.Open` to cover `os::open()`. > > > Diffs > - > > 3rdparty/stout/tests/os/filesystem_tests.cpp > c190baa2230298e428d4034b90dccffb59b4e710 > > > Diff: https://reviews.apache.org/r/66641/diff/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66578: Windows: Ported more unit tests from `os_tests.cpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66578/#review202128 --- Fix it, then Ship it! 3rdparty/stout/include/stout/windows/os.hpp Lines 338 (patched) <https://reviews.apache.org/r/66578/#comment283767> The POSIX implementation ignores `EINTR` so I think the equivalent on Windows would be a non-alertable sleep. 3rdparty/stout/tests/os_tests.cpp Lines 204 (patched) <https://reviews.apache.org/r/66578/#comment283766> I guess you can try if you can set a valid socket to non blocking mode. - Akash Gupta 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 66431: Windows: Fixed `os::read()` to use `ReadFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66431/#review200785 --- Ship it! Ship It! - Akash Gupta On April 9, 2018, 10:53 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66431/ > --- > > (Updated April 9, 2018, 10:53 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/read.hpp > 49878e499209fa2f91fede0ebdabb8f088a9d018 > 3rdparty/stout/include/stout/os/windows/read.hpp > 8047ad590fcc46d3ec46b551472d8c518ae49cc1 > > > Diff: https://reviews.apache.org/r/66431/diff/4/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66440/#review200764 --- Fix it, then Ship it! 3rdparty/libprocess/src/http_proxy.cpp Lines 159 (patched) <https://reviews.apache.org/r/66440/#comment281617> There's also `ERROR_PATH_NOT_FOUND`. To be safe, you might want to check both. - Akash Gupta On April 6, 2018, 11:17 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66440/ > --- > > (Updated April 6, 2018, 11:17 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 > --- > > 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/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66437/#review200763 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:15 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66437/ > --- > > (Updated April 6, 2018, 11:15 p.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/3/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66432: Windows: Fixed `os::write()` to use `WriteFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66432/#review200762 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:14 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66432/ > --- > > (Updated April 6, 2018, 11:14 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. > > > Diffs > - > > 3rdparty/stout/include/stout/os/windows/write.hpp > 71006489918d9495d37d2fdfdca08b40b419481a > 3rdparty/stout/include/stout/os/write.hpp > 9ff749f209e6dd6ca3695907108a029c9a2b4f05 > > > Diff: https://reviews.apache.org/r/66432/diff/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66455: Windows: Fixed `os::ftruncate()` to use `FileEndOfFileInfo`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66455/#review200752 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 11:09 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66455/ > --- > > (Updated April 6, 2018, 11:09 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 > calling `SetFileInformationByHandle(FileEndOfFileInfo)`. This is > significantly easier than using `SetEndOfFile()`, as that requires (1) > saving the original position, (2) seeking to the new position, (3) > setting the end of the file at the new position, then (4) seeking back > to the original position. Instead, this method just sets the end of > the file directly based on the given `length`, much like `ftruncate()` > on POSIX systems. > > > Diffs > - > > 3rdparty/stout/include/stout/os/windows/ftruncate.hpp > fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 > > > Diff: https://reviews.apache.org/r/66455/diff/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66424/#review200749 --- Ship it! Ship It! - Akash Gupta On April 6, 2018, 10:52 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66424/ > --- > > (Updated April 6, 2018, 10:52 p.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/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 66445: Windows: Cleaned up included CRT headers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66445/#review200620 --- Ship it! Ship It! - Akash Gupta 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 66444: Windows: Made `signals.hpp` compile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66444/#review200619 --- Ship it! Ship It! - Akash Gupta 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()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66443/#review200618 --- Ship it! Ship It! - Akash Gupta 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 66442: Windows: Fixed `os::abort()` to use `WriteFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66442/#review200617 --- Ship it! Ship It! - Akash Gupta On April 5, 2018, 2:06 a.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66442/ > --- > > (Updated April 5, 2018, 2:06 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 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66440/#review200616 --- Fix it, then Ship it! 3rdparty/libprocess/src/http_proxy.cpp Line 155 (original), 155 (patched) <https://reviews.apache.org/r/66440/#comment281388> We might need to `#ifdef` here to handle the `GetLastError` on Windows and `errno` on Linux. - Akash Gupta 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 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66439/#review200615 --- Ship it! Ship It! - Akash Gupta 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 66438: Windows: Made `libevent` use CRT file descriptor explicitly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66438/#review200614 --- Ship it! Ship It! - Akash Gupta 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 66433: Windows: Made `net::download()` use CRT file descriptor explicitly.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66433/#review200613 --- Ship it! Ship It! - Akash Gupta 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 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66437/#review200607
---
3rdparty/stout/include/stout/os/windows/fd.hpp
Line 57 (original), 58 (patched)
<https://reviews.apache.org/r/66437/#comment281387>
I'm not sure if every default constructed WindowsFD will be an invalid
WindowsFD handle. Do you know what happens to the enum and union? I thought C++
doesn't specify anything, so it can have any value if uninitialized. I don't
know how these default constructed WindowsFD are used, but you might want to
set it to an invalid handle explcitly.
3rdparty/stout/include/stout/os/windows/fd.hpp
Line 82 (original), 72 (patched)
<https://reviews.apache.org/r/66437/#comment281376>
small nit, but you can use the handle constructor here:
`WindowsFD(int crt) : WindowsFD(INVALID_HANDLE_VALUE) { ... }`
3rdparty/stout/include/stout/os/windows/fd.hpp
Line 85 (original), 84 (patched)
<https://reviews.apache.org/r/66437/#comment281377>
Same thing here with using the socket constructor.
- Akash Gupta
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 66436: Removed use of `fstat()` from `http.cpp` and `http_proxy.cpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66436/#review200604 --- Ship it! Ship It! - Akash Gupta 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 66435: Added overloads for `int_fd` to `os::stat::isdir()` and `size()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66435/#review200603 --- Fix it, then Ship it! 3rdparty/stout/include/stout/os/posix/stat.hpp Lines 66 (patched) <https://reviews.apache.org/r/66435/#comment281370> Did you mean `const int_fd& fd`? 3rdparty/stout/include/stout/os/posix/stat.hpp Lines 99-101 (patched) <https://reviews.apache.org/r/66435/#comment281371> Ditto on `const int_fd& fd` 3rdparty/stout/include/stout/os/posix/stat.hpp Lines 133 (patched) <https://reviews.apache.org/r/66435/#comment281372> Ditto on `const int_fd& fd` - Akash Gupta 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 66434: Windows: Refactored `subprocess_windows.cpp` to use `os::open()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66434/#review200602 --- Ship it! Ship It! - Akash Gupta 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 66432: Windows: Fixed `os::write()` to use `WriteFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66432/#review200601 --- 3rdparty/stout/include/stout/os/windows/write.hpp Lines 35 (patched) <https://reviews.apache.org/r/66432/#comment281369> Same comment as previous read.hpp: Do you know why the return value is `ssize_t` instead of a `Try`. Is it due to it being async signal safe? I imagine the callers of this function will check for error using `errno` then? Since we aren't using the CRT anymore, we might need to map some Windows errors to errno... - Akash Gupta 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 66431: Windows: Fixed `os::read()` to use `ReadFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66431/#review200600 --- 3rdparty/stout/include/stout/os/windows/read.hpp Lines 43 (patched) <https://reviews.apache.org/r/66431/#comment281368> Do you know why the return value is `ssize_t` instead of a `Try`. Is it due to it being async signal safe? I imagine the callers of this function will check for error using `errno` then? Since we aren't using the CRT anymore, we might need to map some Windows errors to errno... - Akash Gupta 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 66430: Windows: Fixed `os::dup()` to use `DuplicateHandle()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66430/#review200598 --- Ship it! Ship It! - Akash Gupta 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 66429: Windows: Deleted dead code from `process::internal` namespace.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66429/#review200597 --- Ship it! Ship It! - Akash Gupta 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 66455: Windows: Fixed `os::ftruncate()` to use `SetEndOfFile()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66455/#review200596 --- 3rdparty/stout/include/stout/os/windows/ftruncate.hpp Lines 34 (patched) <https://reviews.apache.org/r/66455/#comment281366> I think Windows treats the "logical" file as zero-extended due to security reasons; it just doesn't actually write the 0s on disk. So, if you try reading from the extended region, you will get zeros. The only difference would be if you try seeking far into the extended region and then trying to write something. That `WriteFile` operation could take a lot of time, because it has to zero fill the previous bytes & write to disk. 3rdparty/stout/include/stout/os/windows/ftruncate.hpp Line 36 (original), 41 (patched) <https://reviews.apache.org/r/66455/#comment281365> ftruncate doesn't change the file offset, so make sure you store the original file offset and restore it. It's currently setting it to the eof of the new file. - Akash Gupta 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 66428: Windows: Fixed `os::lseek()` to use `SetFilePointerEx()`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66428/#review200594 --- Ship it! Ship It! - Akash Gupta 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 66427: Split `stout/os/lseek.hpp` into Windows and POSIX files.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66427/#review200590 --- Ship it! Ship It! - Akash Gupta 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 66426: Windows: More constness in stout.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66426/#review200589 --- Ship it! Ship It! - Akash Gupta 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 66425: Windows: Replaced `WindowsFD` with `int_fd` typedef.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66425/#review200588 --- Ship it! Ship It! - Akash Gupta 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 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66424/#review200579
---
Fix it, then Ship it!
3rdparty/stout/include/stout/os/windows/mktemp.hpp
Line 54 (original), 54 (patched)
<https://reviews.apache.org/r/66424/#comment281318>
Huh that's a strange function. Seems like it can cause time check/use race
conditions. We probably want to replace this with the native temp directory
functions in the future.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 33 (patched)
<https://reviews.apache.org/r/66424/#comment281320>
:)
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/66424/#comment281319>
I *think* FILE_FLAG_WRITE_THROUGH == O_SYNC.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/66424/#comment281351>
I believe the default behavior(`SECURITY_ATTRIBUTES == NULL`) is to inherit
from the parent directory / access token. We should ask around on what's the
proper settings for security.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 61 (patched)
<https://reviews.apache.org/r/66424/#comment281344>
It's up to you, but I find it clearer to have a switch statement on the
bitfields like this:
```
switch(oflag & (O_RDONLY | O_WRONLY | O_RDWR)) {
case O_RDONLY:
case O_WRONLY:
case O_RDWR:
default:
}
```
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 73 (patched)
<https://reviews.apache.org/r/66424/#comment281348>
Same thing as above. I think it's more import on this one because there's
techically 8 cases for `O_CREAT`, `O_EXCL`, `O_TRUNC`, but POSIX and Windows
only have defined behavior for 5 of them, so you can explicitly mention the
undefined behavior cases like this:
```
switch(oflag & (O_CREAT | O_EXCL | O_TRUNC)) {
case O_CREAT | O_EXCL:
case O_CREAT | O_EXCL | O_TRUNC :
// Ignore O_TRUNC if we get O_CREAT | O_EXCL
create = CREATE_NEW; break;
case ...:
}
- Akash Gupta
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 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66424/#review200579
---
Fix it, then Ship it!
3rdparty/stout/include/stout/os/windows/mktemp.hpp
Line 54 (original), 54 (patched)
<https://reviews.apache.org/r/66424/#comment281318>
Huh that's a strange function. Seems like it can cause time check/use race
conditions. We probably want to replace this with the native temp directory
functions in the future.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 33 (patched)
<https://reviews.apache.org/r/66424/#comment281320>
:)
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/66424/#comment281319>
I *think* FILE_FLAG_WRITE_THROUGH == O_SYNC.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/66424/#comment281351>
I believe the default behavior(`SECURITY_ATTRIBUTES == NULL`) is to inherit
from the parent directory / access token. We should ask around on what's the
proper settings for security.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 61 (patched)
<https://reviews.apache.org/r/66424/#comment281344>
It's up to you, but I find it clearer to have a switch statement on the
bitfields like this:
```
switch(oflag & (O_RDONLY | O_WRONLY | O_RDWR)) {
case O_RDONLY:
case O_WRONLY:
case O_RDWR:
default:
}
```
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 73 (patched)
<https://reviews.apache.org/r/66424/#comment281348>
Same thing as above. I think it's more import on this one because there's
techically 8 cases for `O_CREAT`, `O_EXCL`, `O_TRUNC`, but POSIX and Windows
only have defined behavior for 5 of them, so you can explicitly mention the
undefined behavior cases like this:
```
switch(oflag & (O_CREAT | O_EXCL | O_TRUNC)) {
case O_CREAT | O_EXCL:
case O_CREAT | O_EXCL | O_TRUNC :
// Ignore O_TRUNC if we get O_CREAT | O_EXCL
create = CREATE_NEW; break;
case ...:
}
- Akash Gupta
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 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66424/#review200579
---
Fix it, then Ship it!
3rdparty/stout/include/stout/os/windows/mktemp.hpp
Line 54 (original), 54 (patched)
<https://reviews.apache.org/r/66424/#comment281318>
Huh that's a strange function. Seems like it can cause time check/use race
conditions. We probably want to replace this with the native temp directory
functions in the future.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 33 (patched)
<https://reviews.apache.org/r/66424/#comment281320>
:)
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/66424/#comment281319>
I *think* FILE_FLAG_WRITE_THROUGH == O_SYNC.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/66424/#comment281351>
I believe the default behavior(`SECURITY_ATTRIBUTES == NULL`) is to inherit
from the parent directory / access token. We should ask around on what's the
proper settings for security.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 61 (patched)
<https://reviews.apache.org/r/66424/#comment281344>
It's up to you, but I find it clearer to have a switch statement on the
bitfields like this:
```
switch(oflag & (O_RDONLY | O_WRONLY | O_RDWR)) {
case O_RDONLY:
case O_WRONLY:
case O_RDWR:
default:
}
```
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 73 (patched)
<https://reviews.apache.org/r/66424/#comment281348>
Same thing as above. I think it's more import on this one because there's
techically 8 cases for `O_CREAT`, `O_EXCL`, `O_TRUNC`, but POSIX and Windows
only have defined behavior for 5 of them, so you can explicitly mention the
undefined behavior cases like this:
```
switch(oflag & (O_CREAT | O_EXCL | O_TRUNC)) {
case O_CREAT | O_EXCL:
case O_CREAT | O_EXCL | O_TRUNC :
// Ignore O_TRUNC if we get O_CREAT | O_EXCL
create = CREATE_NEW; break;
case ...:
}
- Akash Gupta
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 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66424/#review200579
---
Fix it, then Ship it!
3rdparty/stout/include/stout/os/windows/mktemp.hpp
Line 54 (original), 54 (patched)
<https://reviews.apache.org/r/66424/#comment281318>
Huh that's a strange function. Seems like it can cause time check/use race
conditions. We probably want to replace this with the native temp directory
functions in the future.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 33 (patched)
<https://reviews.apache.org/r/66424/#comment281320>
:)
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/66424/#comment281319>
I *think* FILE_FLAG_WRITE_THROUGH == O_SYNC.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 45 (patched)
<https://reviews.apache.org/r/66424/#comment281351>
I believe the default behavior(`SECURITY_ATTRIBUTES == NULL`) is to inherit
from the parent directory / access token. We should ask around on what's the
proper settings for security.
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 61 (patched)
<https://reviews.apache.org/r/66424/#comment281344>
It's up to you, but I find it clearer to have a switch statement on the
bitfields like this:
```
switch(oflag & (O_RDONLY | O_WRONLY | O_RDWR)) {
case O_RDONLY:
case O_WRONLY:
case O_RDWR:
default:
}
```
3rdparty/stout/include/stout/os/windows/open.hpp
Lines 73 (patched)
<https://reviews.apache.org/r/66424/#comment281348>
Same thing as above. I think it's more import on this one because there's
techically 8 cases for `O_CREAT`, `O_EXCL`, `O_TRUNC`, but POSIX and Windows
only have defined behavior for 5 of them, so you can explicitly mention the
undefined behavior cases like this:
```
switch(oflag & (O_CREAT | O_EXCL | O_TRUNC)) {
case O_CREAT | O_EXCL:
case O_CREAT | O_EXCL | O_TRUNC :
// Ignore O_TRUNC if we get O_CREAT | O_EXCL
create = CREATE_NEW; break;
case ...:
}
- Akash Gupta
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 66423: Split `stout/os/open.hpp` into Windows and POSIX files.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66423/#review200576 --- Ship it! Ship It! - Akash Gupta 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 66422: Windows: Included used `jobobject.hpp` stout header in Mesos.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66422/#review200575 --- Ship it! Ship It! - Akash Gupta 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 66421: Windows: Included used `jobobject.hpp` stout header in libprocess.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66421/#review200573 --- Ship it! Ship It! - Akash Gupta 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 66420: Windows: Extracted job object code into `os/windows/jobobject.hpp`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66420/#review200572 --- Ship it! Ship It! - Akash Gupta 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 65957: Updated Windows documentation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65957/#review198835 --- Fix it, then Ship it! docs/windows.md Line 17 (original), 17 (patched) <https://reviews.apache.org/r/65957/#comment279081> I don't think "unpaid" can be used in that way. "free of charge" works. docs/windows.md Lines 99 (patched) <https://reviews.apache.org/r/65957/#comment279082> Ninja will pick the first compiler that's in your `$PATH`, so it picked GCC for me. You should mention that you can fix that by setting CC and CXX environmental variables like `$env:CC=$(Get-Command).Source` and `$env:CXX=$(Get-Command cl).Source` - Akash Gupta On March 7, 2018, 8:58 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65957/ > --- > > (Updated March 7, 2018, 8:58 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, Eric > Mumau, and Joseph Wu. > > > Repository: mesos > > > Description > --- > > This notes that Developer Mode is required for non-admin symlinks, > removes reference to the close JIRA epic, and adds information on how > to build with Ninja on Windows. > > > Diffs > - > > docs/windows.md cf2c7c230ec9fc062b4a4fe721d7669ef646253c > > > Diff: https://reviews.apache.org/r/65957/diff/1/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
Re: Review Request 65720: Windows: Fixed location of imported libraries for Ninja.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65720/#review198748 --- Ship it! Ship It! - Akash Gupta On March 6, 2018, 8:05 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65720/ > --- > > (Updated March 6, 2018, 8:05 p.m.) > > > Review request for mesos, Akash Gupta, Benjamin Bannier, Jeff Coffler, and > Joseph Wu. > > > Bugs: MESOS-8599 > https://issues.apache.org/jira/browse/MESOS-8599 > > > Repository: mesos > > > Description > --- > > The Ninja generator does not emit binaries to "Debug" or "Release" > folders (unlike the Visual Studio generator), as it is not a > multi-configuration generator. To fix this, we explicitly set the > `IMPORTED_LOCATION` instead of `IMPORTED_LOCATION_(DEBUG|RELEASE)` > when using the Ninja generator on Windows. This does not reuse the > location used on non-Windows platforms, as they're not consistent > despite the generator being the same. > > > Diffs > - > > 3rdparty/CMakeLists.txt da605707b89bbe9b3db9e60bc0b0a26dac46e56e > > > Diff: https://reviews.apache.org/r/65720/diff/2/ > > > Testing > --- > > > Thanks, > > Andrew Schwartzmeyer > >
