You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2015/09/21 12:43:05 UTC

Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

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

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
-------

Mesos will need to use libevent to build the process library on Windows.

This commit will add a default version of libevent, which we will
eventually retrieve from the 3rdparty GitHub repository, which for now
is our "official" distribution channel for out-of-band third-party
dependencies (especially on Windows, which has no package manager).


Diffs
-----

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

Diff: https://reviews.apache.org/r/38549/diff/


Testing
-------

Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100586
-----------------------------------------------------------

Ship it!


Ship It!

- haosdent huang


On Sept. 21, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100736
-----------------------------------------------------------



CMakeLists.txt (line 37)
<https://reviews.apache.org/r/38549/#comment157987>

    Instead of making this an invisible default, let's make the user opt in. Then, if compiling on Windows and the user does not pass in the ENABLE_LIBEVENT flag, let's fail and explain that libev is not supported, and that $HERE_BE_DRAGONS because libevent is not libev.


- Alex Clemmer


On Sept. 21, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Alex Clemmer <cl...@gmail.com>.

> On Sept. 26, 2015, 6:09 a.m., Joris Van Remoortere wrote:
> > This is a big decision because on linux right now the default is libev. The code paths are significantly different between libev and libevent.
> > hausdorff is going to follow up with a JIRA for this, and force users to compile with the --enable-libevent flag. It will error otherwise. This is a nice compromise to ensure that users understand the decision point.

A small note about what's going on here. This last revision will allow the user to pass in a `-DENABLE_LIBEVENT=1` flag and compile with libevent. The catch is, I forgot to make a few things condition on `ENABLE_LIBEVENT` (such as the `-lev` flag. So, rather than making those changes and threading it through like 9 commits as this code moves around and gets altered, I've added a CMake error that will throw an error if you try to use libevent on Unix, and I'll push a patch up today that actually handles this correctly, which will apply after all these patches go in.

Does that make sense?


- Alex


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


On Sept. 26, 2015, 9:56 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2015, 9:56 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100735
-----------------------------------------------------------

Ship it!


This is a big decision because on linux right now the default is libev. The code paths are significantly different between libev and libevent.
hausdorff is going to follow up with a JIRA for this, and force users to compile with the --enable-libevent flag. It will error otherwise. This is a nice compromise to ensure that users understand the decision point.


3rdparty/cmake/Versions.cmake (line 7)
<https://reviews.apache.org/r/38549/#comment157989>

    todo for compiling non-beta version of libevent



CMakeLists.txt (line 37)
<https://reviews.apache.org/r/38549/#comment157988>

    comment about difference of default from linux


- Joris Van Remoortere


On Sept. 21, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100607
-----------------------------------------------------------


LGTM.

- Artem Harutyunyan


On Sept. 21, 2015, 11:31 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 11:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/
-----------------------------------------------------------

(Updated Sept. 26, 2015, 9:56 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
-------

Mesos will need to use libevent to build the process library on Windows.

This commit will add a default version of libevent, which we will
eventually retrieve from the 3rdparty GitHub repository, which for now
is our "official" distribution channel for out-of-band third-party
dependencies (especially on Windows, which has no package manager).


Diffs (updated)
-----

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

Diff: https://reviews.apache.org/r/38549/diff/


Testing
-------

Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/
-----------------------------------------------------------

(Updated Sept. 21, 2015, 6:31 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.


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


Repository: mesos


Description
-------

Mesos will need to use libevent to build the process library on Windows.

This commit will add a default version of libevent, which we will
eventually retrieve from the 3rdparty GitHub repository, which for now
is our "official" distribution channel for out-of-band third-party
dependencies (especially on Windows, which has no package manager).


Diffs (updated)
-----

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

Diff: https://reviews.apache.org/r/38549/diff/


Testing
-------

Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review99799
-----------------------------------------------------------

Ship it!


Ship It!

- Joseph Wu


On Sept. 21, 2015, 3:43 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3380
>     https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -----
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>