Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.

2018-06-26 Thread Benjamin Mahler 


> On June 4, 2018, 11:53 p.m., Joseph Wu wrote:
> > General note on code organization:
> > 
> > Since you are splitting the `io::internal::read/write` functions into Posix 
> > and Windows implementations, it would be helpful to organize the code like:
> > ```
> > 3rdparty/libprocess/src/
> > |-- io/
> > |-- io.cpp
> > |
> > |-- posix/
> > |   |-- io.hpp // Instead of io_internal.hpp
> > |   |-- io.cpp // Instead of poll_io.cpp
> > |
> > |-- windows/
> > |   // Instead of libwinio_impl.hpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio.hpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.hpp 
> > |
> > |   // Instead of libwinio_impl.cpp ( 
> > https://reviews.apache.org/r/67389/ )
> > |   // Possibly combine libwinio_eventloop.cpp too ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- event_loop.cpp 
> > |
> > |-- io.hpp 
> > |-- io.cpp // Instead of libwinio_io.cpp ( 
> > https://reviews.apache.org/r/67390/ )
> > |-- ...
> > ```
> 
> Andrew Schwartzmeyer wrote:
> +1

Doesn't this type of approach mean that I have look at two different headers 
(POSIX and Windows) whenever I want to write cross-platform code? I would hope 
that I only have to look at one header unless I need to #ifdef in the call site.


- Benjamin


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


On June 21, 2018, 11:47 a.m., Akash Gupta wrote:
> 
> ---
> 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.
> 
> 
> Bugs: MESOS-8668
> https://issues.apache.org/jira/browse/MESOS-8668
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.

2018-06-25 Thread Joseph Wu 

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


Ship it!





3rdparty/libprocess/src/io.cpp
Lines 136-147 (original), 41-50 (patched)


Technically, you could remove these and then move the implementation in 
`src/posix/io.cpp` out of the `internal` namespace.

I'm fine with the diff as-is though.


- Joseph Wu


On June 21, 2018, 4:47 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated June 21, 2018, 4:47 a.m.)
> 
> 
> 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
> ---
> 
> 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.

2018-06-21 Thread Andrew Schwartzmeyer 

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


Fix it, then Ship it!




Only thing I'm not totally sold on is libevent being under POSIX, but it'll be 
true very soon.


3rdparty/libprocess/src/posix/io.cpp
Lines 53-101 (patched)


Let's not forget to remove these Windows parts in whatever review removes 
this code from Windows. (Close if already done :)


- Andrew Schwartzmeyer


On June 21, 2018, 4:47 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67388/
> ---
> 
> (Updated June 21, 2018, 4:47 a.m.)
> 
> 
> 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
> ---
> 
> 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 67388: Moved libprocess POSIX and Windows implementations to separate folders.

2018-06-21 Thread Akash Gupta 

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