mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Clemmer" <clemmer.alexan...@gmail.com>
Subject Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.
Date Tue, 22 Sep 2015 18:53:52 GMT


> On Sept. 22, 2015, 6:32 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/ProcessConfigure.cmake, lines 59-65
> > <https://reviews.apache.org/r/38542/diff/1/?file=1077780#file1077780line59>
> >
> >     What do you think about moving this `if` block into `Versions.cmake`?
> >     
> >     Pro: Versions are kept in one place.  If we unilaterally update to glog 0.3.4,
we'll only need to update it in one place.
> >     
> >     Con: Logic in the version file, which we don't really have a precedent for.

I think it would be best to keep it in `ProcessConfigure.cmake`. The reason is that then anyone
who uses libprocess need only `include(ProcessConfigure)` and the third-party build logic
is generated seemlessly for them. If you put this in `Versions.cmake`, then you'd have to
`include` both to fully configure and use libprocess, and you'd have to move `Versions.cmake`
back into the `libprocess` directory.

Besides that, we're going to upgrade glog to 0.3.5 at some point anyway, and in that case,
the conditional will disappear, and it will stop making much sense to have it in `Versions.cmake`.


- Alex


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


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake a5f8d399e151acad87bb72ecb1f7372b2c467423

>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e

> 
> Diff: https://reviews.apache.org/r/38542/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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message