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:20:06 UTC

Review Request 38756: CMake: Add support for compiling the agent with CMake.

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

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


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


Repository: mesos


Description
-------

CMake: Add support for compiling the agent with CMake.


Diffs
-----

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/AgentConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Sept. 25, 2015, 10:39 a.m., Joseph Wu wrote:
> > src/CMakeLists.txt, lines 34-38
> > <https://reviews.apache.org/r/38756/diff/1/?file=1084616#file1084616line34>
> >
> >     This seems macro-able.
> >     
> >     A macro which would let you do something like:
> >     ```
> >     BUILD_PROTO(MESOS)
> >     BUILD_PROTO(V1_MESOS)
> >     BUILD_PROTO(AUTHENTICATION)
> >     ...
> >     ```
> 
> Alex Clemmer wrote:
>     Hmm, I see your point about there being a lot of duplicated code, so let me bounce this idea off of you. I think we will end up having to have two protoc-generating functions, like `PROTOC_TO_MESOS_INCLUDE_DIR(V1_MESOS)` and `PROTOC_TO_MESOS_SRC_DIR(MESSAGES)`.
>     
>     The reason is that there are actually two flavors of `protoc` being generated here -- one where we are generating protobuf code and putting it in `include/` and one where we are putting it in `src/`. This means practically that at least two things need to be able to change: (1) `protoc` flags need to change to tell where to put the generated code, and (2) `DEPENDS` needs to change to make sure we depend on build targets that will generate that folder before we try to dump the generated code there.
>     
>     So, at the very least, I think `BUILD_PROTO` would have to look something like: `BUILD_PROTO(MESOS, make_bin_include_dir, ${MESOS_BIN_INCLUDE_DIR})`, but on top of that, we also have other baked-in assumptions like what the working dir is, and what the rest of the `-I` flags are. By splitting it up, it seems like these assumptions are much more explicit, which is attractive to me.
>     
>     Thoughts?

I think having two flavors of macros (`PROTOC_TO_MESOS_INCLUDE_DIR`/`PROTOC_TO_MESOS_SRC_DIR`) would be good and explicit.  (<- my preference)

If that isn't trivial with how the `TO_INCLUDE_DIR`/`TO_SRC_DIR` variables are defined, then the multiple-argument macro would be nice too.


- Joseph


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


On Sept. 24, 2015, 11:20 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 5:39 p.m., Joseph Wu wrote:
> > src/CMakeLists.txt, lines 34-38
> > <https://reviews.apache.org/r/38756/diff/1/?file=1084616#file1084616line34>
> >
> >     This seems macro-able.
> >     
> >     A macro which would let you do something like:
> >     ```
> >     BUILD_PROTO(MESOS)
> >     BUILD_PROTO(V1_MESOS)
> >     BUILD_PROTO(AUTHENTICATION)
> >     ...
> >     ```

Hmm, I see your point about there being a lot of duplicated code, so let me bounce this idea off of you. I think we will end up having to have two protoc-generating functions, like `PROTOC_TO_MESOS_INCLUDE_DIR(V1_MESOS)` and `PROTOC_TO_MESOS_SRC_DIR(MESSAGES)`.

The reason is that there are actually two flavors of `protoc` being generated here -- one where we are generating protobuf code and putting it in `include/` and one where we are putting it in `src/`. This means practically that at least two things need to be able to change: (1) `protoc` flags need to change to tell where to put the generated code, and (2) `DEPENDS` needs to change to make sure we depend on build targets that will generate that folder before we try to dump the generated code there.

So, at the very least, I think `BUILD_PROTO` would have to look something like: `BUILD_PROTO(MESOS, make_bin_include_dir, ${MESOS_BIN_INCLUDE_DIR})`, but on top of that, we also have other baked-in assumptions like what the working dir is, and what the rest of the `-I` flags are. By splitting it up, it seems like these assumptions are much more explicit, which is attractive to me.

Thoughts?


- Alex


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


On Sept. 25, 2015, 6:20 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 6:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 5:39 p.m., Joseph Wu wrote:
> > src/CMakeLists.txt, lines 34-38
> > <https://reviews.apache.org/r/38756/diff/1/?file=1084616#file1084616line34>
> >
> >     This seems macro-able.
> >     
> >     A macro which would let you do something like:
> >     ```
> >     BUILD_PROTO(MESOS)
> >     BUILD_PROTO(V1_MESOS)
> >     BUILD_PROTO(AUTHENTICATION)
> >     ...
> >     ```
> 
> Alex Clemmer wrote:
>     Hmm, I see your point about there being a lot of duplicated code, so let me bounce this idea off of you. I think we will end up having to have two protoc-generating functions, like `PROTOC_TO_MESOS_INCLUDE_DIR(V1_MESOS)` and `PROTOC_TO_MESOS_SRC_DIR(MESSAGES)`.
>     
>     The reason is that there are actually two flavors of `protoc` being generated here -- one where we are generating protobuf code and putting it in `include/` and one where we are putting it in `src/`. This means practically that at least two things need to be able to change: (1) `protoc` flags need to change to tell where to put the generated code, and (2) `DEPENDS` needs to change to make sure we depend on build targets that will generate that folder before we try to dump the generated code there.
>     
>     So, at the very least, I think `BUILD_PROTO` would have to look something like: `BUILD_PROTO(MESOS, make_bin_include_dir, ${MESOS_BIN_INCLUDE_DIR})`, but on top of that, we also have other baked-in assumptions like what the working dir is, and what the rest of the `-I` flags are. By splitting it up, it seems like these assumptions are much more explicit, which is attractive to me.
>     
>     Thoughts?
> 
> Joseph Wu wrote:
>     I think having two flavors of macros (`PROTOC_TO_MESOS_INCLUDE_DIR`/`PROTOC_TO_MESOS_SRC_DIR`) would be good and explicit.  (<- my preference)
>     
>     If that isn't trivial with how the `TO_INCLUDE_DIR`/`TO_SRC_DIR` variables are defined, then the multiple-argument macro would be nice too.

I solved both these issues with a pretty substantial changeset. Bascially instead of creating two macros for the issue above and two macros for the below, I just made them one macro. It's way cleaner and inuitive.


- Alex


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


On Sept. 25, 2015, 10:14 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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



src/CMakeLists.txt (lines 34 - 38)
<https://reviews.apache.org/r/38756/#comment157844>

    This seems macro-able.
    
    A macro which would let you do something like:
    ```
    BUILD_PROTO(MESOS)
    BUILD_PROTO(V1_MESOS)
    BUILD_PROTO(AUTHENTICATION)
    ...
    ```



src/cmake/MesosProtobuf.cmake (lines 17 - 22)
<https://reviews.apache.org/r/38756/#comment157845>

    Also looks macro-able.
    
    i.e.
    ```
    DEFINE_PROTO_SOURCES(MESOS    /mesos/mesos)
    DEFINE_PROTO_SOURCES(V!_MESOS /mesos/v1/mesos)
    ...
    ```


- Joseph Wu


On Sept. 24, 2015, 11:20 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

Ship it!


Also tested on OSX.

i.e.
```
mkdir cbuild
cd cbuild
cmake ..
make -j8
make test
```

- Joseph Wu


On Sept. 25, 2015, 4:19 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 4:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

Ship it!


Ship It!

- Joris Van Remoortere


On Sept. 26, 2015, 10:57 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2015, 10:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38756/#review100766
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752, 38753, 38754, 38755, 38756]

All tests passed.

- Mesos ReviewBot


On Sept. 26, 2015, 10:57 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2015, 10:57 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

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


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


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


Repository: mesos


Description
-------

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-----

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

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


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


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


Repository: mesos


Description
-------

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-----

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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


This doesn't compiler for me on Ubuntu 15.04
I will work with you offline to figure this out.


CMakeLists.txt (line 23)
<https://reviews.apache.org/r/38756/#comment157996>

    26


- Joris Van Remoortere


On Sept. 25, 2015, 11:19 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 11:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38756/#review100728
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752, 38753, 38754, 38755, 38756]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 11:19 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 11:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

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


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


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


Repository: mesos


Description
-------

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-----

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 11:16 p.m., Joseph Wu wrote:
> > For me, `mkdir build && cd build && cmake .. && make`.
> > 
> > Gave me an error (OSX):
> > 
> > ```
> > Scanning dependencies of target mesos-0.25.0
> > /Applications/Xcode.app/Contents/Developer/usr/bin/make -f src/CMakeFiles/mesos-0.25.0.dir/build.make src/CMakeFiles/mesos-0.25.0.dir/build
> > /usr/local/Cellar/cmake/3.2.3/bin/cmake -E cmake_progress_report /Users/josephwu/mesos2/build-cmake/CMakeFiles 
> > 
> > ...
> > 
> > [ 77%] Building CXX object src/CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o
> > cd /Users/josephwu/mesos2/build-cmake/src && /usr/local/bin/c++   -DLIBDIR=\"WARNINGDONOTUSEME\" -DPICOJSON_USE_INT64 -DPKGLIBEXECDIR=\"WARNINGDONOTUSEME\" -DVERSION=\"WARNINGDONOTUSEME\" -D__STDC_FORMAT_MACROS -std=c++11 -g -I/Users/josephwu/mesos2/include -I/Users/josephwu/mesos2/include/mesos -I/Users/josephwu/mesos2/build-cmake/include -I/Users/josephwu/mesos2/build-cmake/src -I/Users/josephwu/mesos2/src -I/Users/josephwu/mesos2/3rdparty/libprocess/3rdparty/../include -I/Users/josephwu/mesos2/3rdparty/libprocess/3rdparty/stout/include -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/boost-1.53.0/src/boost-1.53.0 -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog-0.3.3-lib/lib/include -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/picojson-1.3.0/src/picojson-1.3.0 -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/protobuf-2.5.0/src/protobuf-2.5.0-lib/lib/include -I/Users/josephwu/mesos2/buil
 d-cmake/3rdparty/zookeeper-3.4.5/src/zookeeper-3.4.5-lib/lib/include/zookeeper    -o CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o -c /Users/josephwu/mesos2/src/slave/http.cpp
> > In file included from /Users/josephwu/mesos2/src/slave/http.cpp:25:
> > /Users/josephwu/mesos2/include/mesos/executor/executor.hpp:25:10: fatal error: 'executor/executor.pb.h' file not found
> > #include <executor/executor.pb.h>
> >          ^
> > 1 error generated.
> > make[2]: *** [src/CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o] Error 1
> > make[1]: *** [src/CMakeFiles/mesos-0.25.0.dir/all] Error 2
> > make: *** [all] Error 2
> > ```
> > 
> > I wonder if the macro change broke something.

I accidentally moved that part into the master build WIP commit. It should be fixed now.


- Alex


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


On Sept. 25, 2015, 11:19 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 11:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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


For me, `mkdir build && cd build && cmake .. && make`.

Gave me an error (OSX):

```
Scanning dependencies of target mesos-0.25.0
/Applications/Xcode.app/Contents/Developer/usr/bin/make -f src/CMakeFiles/mesos-0.25.0.dir/build.make src/CMakeFiles/mesos-0.25.0.dir/build
/usr/local/Cellar/cmake/3.2.3/bin/cmake -E cmake_progress_report /Users/josephwu/mesos2/build-cmake/CMakeFiles 

...

[ 77%] Building CXX object src/CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o
cd /Users/josephwu/mesos2/build-cmake/src && /usr/local/bin/c++   -DLIBDIR=\"WARNINGDONOTUSEME\" -DPICOJSON_USE_INT64 -DPKGLIBEXECDIR=\"WARNINGDONOTUSEME\" -DVERSION=\"WARNINGDONOTUSEME\" -D__STDC_FORMAT_MACROS -std=c++11 -g -I/Users/josephwu/mesos2/include -I/Users/josephwu/mesos2/include/mesos -I/Users/josephwu/mesos2/build-cmake/include -I/Users/josephwu/mesos2/build-cmake/src -I/Users/josephwu/mesos2/src -I/Users/josephwu/mesos2/3rdparty/libprocess/3rdparty/../include -I/Users/josephwu/mesos2/3rdparty/libprocess/3rdparty/stout/include -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/boost-1.53.0/src/boost-1.53.0 -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/glog-0.3.3/src/glog-0.3.3-lib/lib/include -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/picojson-1.3.0/src/picojson-1.3.0 -I/Users/josephwu/mesos2/build-cmake/3rdparty/libprocess/3rdparty/protobuf-2.5.0/src/protobuf-2.5.0-lib/lib/include -I/Users/josephwu/mesos2/build-cm
 ake/3rdparty/zookeeper-3.4.5/src/zookeeper-3.4.5-lib/lib/include/zookeeper    -o CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o -c /Users/josephwu/mesos2/src/slave/http.cpp
In file included from /Users/josephwu/mesos2/src/slave/http.cpp:25:
/Users/josephwu/mesos2/include/mesos/executor/executor.hpp:25:10: fatal error: 'executor/executor.pb.h' file not found
#include <executor/executor.pb.h>
         ^
1 error generated.
make[2]: *** [src/CMakeFiles/mesos-0.25.0.dir/slave/http.cpp.o] Error 1
make[1]: *** [src/CMakeFiles/mesos-0.25.0.dir/all] Error 2
make: *** [all] Error 2
```

I wonder if the macro change broke something.

- Joseph Wu


On Sept. 25, 2015, 3:14 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 3:14 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

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


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


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


Repository: mesos


Description
-------

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-----

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/AgentConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Artem Harutyunyan <ar...@mesosphere.io>.

> On Sept. 25, 2015, 2:36 p.m., Artem Harutyunyan wrote:
> >

LGTM.


- Artem


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


On Sept. 24, 2015, 11:20 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 9:36 p.m., Artem Harutyunyan wrote:
> > src/CMakeLists.txt, line 1
> > <https://reviews.apache.org/r/38756/diff/1/?file=1084616#file1084616line1>
> >
> >     I think this file belongs in src/slave. And from top level src/ dir we should just be pulling CMakeLists.txt files from component directories. What do you think?

Personally, I think We should do that only if the goal is to build a libagent binary.

If we don't plan on doing that, I don't think it buys us much. For example, if you look at the WIP commit that builds the master (https://github.com/hausdorff/mesos/commits/cmake_agent), we just have a bunch of variables like `MASTER_SRC` that break up the source by component (master, isolator, etc.) and compile it all at once. It's pretty clean, and moving each of those things to its own file really only means that you have to go to another file to see what `AGENT_SRC` has in it.

BUT, if we plan to ship libagent, libmaster, etc., then it does make sense, because they're all individual things.


- Alex


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


On Sept. 25, 2015, 10:14 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Artem Harutyunyan <ar...@mesosphere.io>.

> On Sept. 25, 2015, 2:36 p.m., Artem Harutyunyan wrote:
> > src/CMakeLists.txt, line 1
> > <https://reviews.apache.org/r/38756/diff/1/?file=1084616#file1084616line1>
> >
> >     I think this file belongs in src/slave. And from top level src/ dir we should just be pulling CMakeLists.txt files from component directories. What do you think?
> 
> Alex Clemmer wrote:
>     Personally, I think We should do that only if the goal is to build a libagent binary.
>     
>     If we don't plan on doing that, I don't think it buys us much. For example, if you look at the WIP commit that builds the master (https://github.com/hausdorff/mesos/commits/cmake_agent), we just have a bunch of variables like `MASTER_SRC` that break up the source by component (master, isolator, etc.) and compile it all at once. It's pretty clean, and moving each of those things to its own file really only means that you have to go to another file to see what `AGENT_SRC` has in it.
>     
>     BUT, if we plan to ship libagent, libmaster, etc., then it does make sense, because they're all individual things.

For now slave is going to be the only thing in that file anyway, so we can defer this conversation to later, when we start adding more components. But I am pretty sure that this file will become pretty big pretty soon :-). Dropping this for now.


- Artem


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


On Sept. 25, 2015, 4:19 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 4:19 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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



src/CMakeLists.txt (line 1)
<https://reviews.apache.org/r/38756/#comment157860>

    I think this file belongs in src/slave. And from top level src/ dir we should just be pulling CMakeLists.txt files from component directories. What do you think?



src/CMakeLists.txt (lines 123 - 125)
<https://reviews.apache.org/r/38756/#comment157859>

    Extra newlines.



src/slave/cmake/AgentConfigure.cmake (line 1)
<https://reviews.apache.org/r/38756/#comment157951>

    Can we keep calling this slave for the time being? It will take a while until we get to file and directory rename (it's pretty low in Ben's list).


- Artem Harutyunyan


On Sept. 24, 2015, 11:20 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 7:06 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752]
> > 
> > Failed command: ./support/apply-review.sh -n -r 38752
> > 
> > Error:
> >  2015-09-25 07:06:43 URL:https://reviews.apache.org/r/38752/diff/raw/ [1426/1426] -> "38752.patch" [1]
> > Successfully applied: CMake: Transition Mesos to use new third-party build scripts.
> > 
> > CMake: Transition Mesos to use new third-party build scripts.
> > 
> > 
> > Review: https://reviews.apache.org/r/38752
> > No files to lint
> > 
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > mesos:
> >   cmake/MesosConfigure.cmake
> > libprocess:
> >   3rdparty/libprocess/3rdparty/CMakeLists.txt
> > Failed to commit patch

Whoa, really? How did that happen??? Sorry folks! :S


- Alex


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


On Sept. 25, 2015, 6:20 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 6:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

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

> On Sept. 25, 2015, 7:06 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752]
> > 
> > Failed command: ./support/apply-review.sh -n -r 38752
> > 
> > Error:
> >  2015-09-25 07:06:43 URL:https://reviews.apache.org/r/38752/diff/raw/ [1426/1426] -> "38752.patch" [1]
> > Successfully applied: CMake: Transition Mesos to use new third-party build scripts.
> > 
> > CMake: Transition Mesos to use new third-party build scripts.
> > 
> > 
> > Review: https://reviews.apache.org/r/38752
> > No files to lint
> > 
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > mesos:
> >   cmake/MesosConfigure.cmake
> > libprocess:
> >   3rdparty/libprocess/3rdparty/CMakeLists.txt
> > Failed to commit patch
> 
> Alex Clemmer wrote:
>     Whoa, really? How did that happen??? Sorry folks! :S
> 
> Joseph Wu wrote:
>     I used Artem's script to pull in this chain :D
>     https://reviews.apache.org/r/38705/
>     
>     It looks like 38752 doesn't `apply` in general.  (Not just because of this separation issue.)

I rebased the reviews against the master. Does that solve the issue?


- Alex


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


On Sept. 25, 2015, 10:14 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:14 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Sept. 25, 2015, 12:06 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752]
> > 
> > Failed command: ./support/apply-review.sh -n -r 38752
> > 
> > Error:
> >  2015-09-25 07:06:43 URL:https://reviews.apache.org/r/38752/diff/raw/ [1426/1426] -> "38752.patch" [1]
> > Successfully applied: CMake: Transition Mesos to use new third-party build scripts.
> > 
> > CMake: Transition Mesos to use new third-party build scripts.
> > 
> > 
> > Review: https://reviews.apache.org/r/38752
> > No files to lint
> > 
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > mesos:
> >   cmake/MesosConfigure.cmake
> > libprocess:
> >   3rdparty/libprocess/3rdparty/CMakeLists.txt
> > Failed to commit patch
> 
> Alex Clemmer wrote:
>     Whoa, really? How did that happen??? Sorry folks! :S

I used Artem's script to pull in this chain :D
https://reviews.apache.org/r/38705/

It looks like 38752 doesn't `apply` in general.  (Not just because of this separation issue.)


- Joseph


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


On Sept. 24, 2015, 11:20 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2015, 11:20 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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 38756: CMake: Add support for compiling the agent with CMake.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38756/#review100553
-----------------------------------------------------------


Bad patch!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752]

Failed command: ./support/apply-review.sh -n -r 38752

Error:
 2015-09-25 07:06:43 URL:https://reviews.apache.org/r/38752/diff/raw/ [1426/1426] -> "38752.patch" [1]
Successfully applied: CMake: Transition Mesos to use new third-party build scripts.

CMake: Transition Mesos to use new third-party build scripts.


Review: https://reviews.apache.org/r/38752
No files to lint

ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
mesos:
  cmake/MesosConfigure.cmake
libprocess:
  3rdparty/libprocess/3rdparty/CMakeLists.txt
Failed to commit patch

- Mesos ReviewBot


On Sept. 25, 2015, 6:20 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38756/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 6:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3385
>     https://issues.apache.org/jira/browse/MESOS-3385
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> CMake: Add support for compiling the agent with CMake.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
>   src/CMakeLists.txt PRE-CREATION 
>   src/cmake/MesosProtobuf.cmake PRE-CREATION 
>   src/slave/cmake/AgentConfigure.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38756/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
> 
>