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/25 08:18:43 UTC

Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

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


Repository: mesos


Description
-------

CMake: Pull third-party configuration logic into its own .cmake file.


Diffs
-----

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

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


Testing
-------

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

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

(Updated Sept. 26, 2015, 10:05 p.m.)


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


Repository: mesos


Description
-------

CMake: Pull third-party configuration logic into its own .cmake file.


Diffs (updated)
-----

  3rdparty/libprocess/3rdparty/CMakeLists.txt b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

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


Testing
-------

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

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

> On Sept. 26, 2015, 6:10 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake, line 82
> > <https://reviews.apache.org/r/38751/diff/2/?file=1084978#file1084978line82>
> >
> >     Do we not need lflags for libevent?
> 
> Alex Clemmer wrote:
>     The short answer is that we didn't need this because we had only intended this to work on Windows. This will change with the next set of revisions, because we have decided we want to make the user opt into libevent when we build on Windows, which requires (obviously) wiring up the flag to be user-facing. Until now, the flag wasn't user-facing, so we never actually hit this code path. Now that it is, we will correctly handle this too. :)

So I'll address this in the next patch. This would be too annoying to thread through and rebase all the commits where this code is moving around or getting altered. It's a trivial patch to do.


- Alex


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


On Sept. 26, 2015, 10:05 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

> On Sept. 26, 2015, 6:10 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake, line 82
> > <https://reviews.apache.org/r/38751/diff/2/?file=1084978#file1084978line82>
> >
> >     Do we not need lflags for libevent?

The short answer is that we didn't need this because we had only intended this to work on Windows. This will change with the next set of revisions, because we have decided we want to make the user opt into libevent when we build on Windows, which requires (obviously) wiring up the flag to be user-facing. Until now, the flag wasn't user-facing, so we never actually hit this code path. Now that it is, we will correctly handle this too. :)


- Alex


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


On Sept. 25, 2015, 10:11 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

Ship it!



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (line 82)
<https://reviews.apache.org/r/38751/#comment157993>

    Do we not need lflags for libevent?


- Joris Van Remoortere


On Sept. 25, 2015, 10:11 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:11 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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

(Updated Sept. 25, 2015, 10:11 p.m.)


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


Repository: mesos


Description
-------

CMake: Pull third-party configuration logic into its own .cmake file.


Diffs (updated)
-----

  3rdparty/libprocess/3rdparty/CMakeLists.txt b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

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


Testing
-------

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

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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


LGTM.

- Artem Harutyunyan


On Sept. 24, 2015, 11:18 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

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


Looks good.

Won't hurt to mention (in the description) that this new file is used in the next review.

- Joseph Wu


On Sept. 24, 2015, 11:18 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> -------
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>