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 2016/11/28 18:46:50 UTC
Re: Review Request 53753: CMake: Added variable for default linking
strategy.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53753/#review157092
-----------------------------------------------------------
cmake/MesosConfigure.cmake (line 71)
<https://reviews.apache.org/r/53753/#comment227512>
Hmm. I think this is not available in CMake 2.8.0, which is the minimum version of CMake we support? In the documentation for 2.8.7[1] it's not clear to me that this variable is implemented, but in 2.8.11, it is[2]. Originally we decided to support 2.8.0 because the Ubuntu CMake packages going back to version Ubuntu 12 support CMake 2.8.0 or later. These days it looks like 12 supports CMake 2.8.7, while 14 supports 2.8.11. So, we could bump the version here, but then we should update `cmake_minimum_required`. We should do this with the conscious understanding that we're not supporting 12's default CMake installation, which makes the barrier to entry higher.
[1] https://cmake.org/cmake/help/v2.8.7/cmake.html
[2] https://cmake.org/cmake/help/v2.8.11/cmake.html
- Alex Clemmer
On Nov. 15, 2016, 3:08 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53753/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2016, 3:08 a.m.)
>
>
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
>
>
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `MESOS_DEFAULT_LIBRARY_LINKAGE` changes all Mesos libraries to
> the use specified linking strategy (static or shared). This does
> not affect third-party libraries, which have their own requirements.
>
> On Posix, the default is shared linkage. On Windows the default
> is static linkage.
>
> This review replaces: https://reviews.apache.org/r/49862/
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d
> cmake/MesosConfigure.cmake de04389b016b22bdff69e94d37fad4eedbda5874
> src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f
> src/slave/qos_controllers/CMakeLists.txt 87c92af21c012655c201c01cd4ba5ff912555119
> src/slave/resource_estimators/CMakeLists.txt 17b149f734ea9dc8ac4c5dd45bdb8312faf4cc77
>
> Diff: https://reviews.apache.org/r/53753/diff/
>
>
> Testing
> -------
>
> cmake ..
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 53753: CMake: Added variable for default linking
strategy.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Nov. 28, 2016, 6:46 p.m., Alex Clemmer wrote:
> > cmake/MesosConfigure.cmake, line 71
> > <https://reviews.apache.org/r/53753/diff/1/?file=1563833#file1563833line71>
> >
> > Hmm. I think this is not available in CMake 2.8.0, which is the minimum version of CMake we support? In the documentation for 2.8.7[1] it's not clear to me that this variable is implemented, but in 2.8.11, it is[2]. Originally we decided to support 2.8.0 because the Ubuntu CMake packages going back to version Ubuntu 12 support CMake 2.8.0 or later. These days it looks like 12 supports CMake 2.8.7, while 14 supports 2.8.11. So, we could bump the version here, but then we should update `cmake_minimum_required`. We should do this with the conscious understanding that we're not supporting 12's default CMake installation, which makes the barrier to entry higher.
> >
> > [1] https://cmake.org/cmake/help/v2.8.7/cmake.html
> > [2] https://cmake.org/cmake/help/v2.8.11/cmake.html
>
> Joseph Wu wrote:
> Here's what I'm considering:
>
> 1) By setting `CMAKE_POSITION_INDEPENDENT_CODE` globally, we can reduce logic in 3 existing places where we build an intermediate library. I have some plans (in my head) to increase the number of intermediate libraries so that we don't have a gigantic libmesos.
> 2) 2.8.10 is the earliest version which includes the `CMAKE_POSITION_INDEPENDENT_CODE` option. Increasing the burden for using these old distros is not exactly a problem, as cmake works on those old distros. There are already barriers for all the old distros, CentOS 6 especially (which requires you to download rpms from some really arbitrary locations).
>
> In total, I'm in favor of bumping up the required CMake version.
I'm completely fine with bumping the version. Just wanted to make sure we make the decision consciously. :)
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53753/#review157092
-----------------------------------------------------------
On Nov. 15, 2016, 3:08 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53753/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2016, 3:08 a.m.)
>
>
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
>
>
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `MESOS_DEFAULT_LIBRARY_LINKAGE` changes all Mesos libraries to
> the use specified linking strategy (static or shared). This does
> not affect third-party libraries, which have their own requirements.
>
> On Posix, the default is shared linkage. On Windows the default
> is static linkage.
>
> This review replaces: https://reviews.apache.org/r/49862/
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d
> cmake/MesosConfigure.cmake de04389b016b22bdff69e94d37fad4eedbda5874
> src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f
> src/slave/qos_controllers/CMakeLists.txt 87c92af21c012655c201c01cd4ba5ff912555119
> src/slave/resource_estimators/CMakeLists.txt 17b149f734ea9dc8ac4c5dd45bdb8312faf4cc77
>
> Diff: https://reviews.apache.org/r/53753/diff/
>
>
> Testing
> -------
>
> cmake ..
> make
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 53753: CMake: Added variable for default linking
strategy.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Nov. 28, 2016, 10:46 a.m., Alex Clemmer wrote:
> > cmake/MesosConfigure.cmake, line 71
> > <https://reviews.apache.org/r/53753/diff/1/?file=1563833#file1563833line71>
> >
> > Hmm. I think this is not available in CMake 2.8.0, which is the minimum version of CMake we support? In the documentation for 2.8.7[1] it's not clear to me that this variable is implemented, but in 2.8.11, it is[2]. Originally we decided to support 2.8.0 because the Ubuntu CMake packages going back to version Ubuntu 12 support CMake 2.8.0 or later. These days it looks like 12 supports CMake 2.8.7, while 14 supports 2.8.11. So, we could bump the version here, but then we should update `cmake_minimum_required`. We should do this with the conscious understanding that we're not supporting 12's default CMake installation, which makes the barrier to entry higher.
> >
> > [1] https://cmake.org/cmake/help/v2.8.7/cmake.html
> > [2] https://cmake.org/cmake/help/v2.8.11/cmake.html
Here's what I'm considering:
1) By setting `CMAKE_POSITION_INDEPENDENT_CODE` globally, we can reduce logic in 3 existing places where we build an intermediate library. I have some plans (in my head) to increase the number of intermediate libraries so that we don't have a gigantic libmesos.
2) 2.8.10 is the earliest version which includes the `CMAKE_POSITION_INDEPENDENT_CODE` option. Increasing the burden for using these old distros is not exactly a problem, as cmake works on those old distros. There are already barriers for all the old distros, CentOS 6 especially (which requires you to download rpms from some really arbitrary locations).
In total, I'm in favor of bumping up the required CMake version.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53753/#review157092
-----------------------------------------------------------
On Nov. 14, 2016, 7:08 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53753/
> -----------------------------------------------------------
>
> (Updated Nov. 14, 2016, 7:08 p.m.)
>
>
> Review request for mesos, Alex Clemmer and Joris Van Remoortere.
>
>
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The `MESOS_DEFAULT_LIBRARY_LINKAGE` changes all Mesos libraries to
> the use specified linking strategy (static or shared). This does
> not affect third-party libraries, which have their own requirements.
>
> On Posix, the default is shared linkage. On Windows the default
> is static linkage.
>
> This review replaces: https://reviews.apache.org/r/49862/
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt 770a3828a6e0ffaa4f185392fdc1a2152446449d
> cmake/MesosConfigure.cmake de04389b016b22bdff69e94d37fad4eedbda5874
> src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f
> src/slave/qos_controllers/CMakeLists.txt 87c92af21c012655c201c01cd4ba5ff912555119
> src/slave/resource_estimators/CMakeLists.txt 17b149f734ea9dc8ac4c5dd45bdb8312faf4cc77
>
> Diff: https://reviews.apache.org/r/53753/diff/
>
>
> Testing
> -------
>
> cmake ..
> make
>
>
> Thanks,
>
> Joseph Wu
>
>