mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 53756: CMake: Added logrotate container logger module to the build.
Date Mon, 21 Nov 2016 19:24:46 GMT


> On Nov. 17, 2016, 9:19 a.m., Alex Clemmer wrote:
> > src/slave/container_loggers/CMakeLists.txt, line 25
> > <https://reviews.apache.org/r/53756/diff/2/?file=1564715#file1564715line25>
> >
> >     For my own education, this must be shared, rather than `MESOS_DEFAULT_LIBRARY_LINKAGE`?
Or am I misunderstanding?

All the modules (including example modules) must be linked as shared libraries.  It doesn't
make sense to link them any other way, even if the default linking strategy is `STATIC`. 
That's why all modules are omitted by default on Windows.


> On Nov. 17, 2016, 9:19 a.m., Alex Clemmer wrote:
> > src/slave/container_loggers/CMakeLists.txt, line 30
> > <https://reviews.apache.org/r/53756/diff/2/?file=1564715#file1564715line30>
> >
> >     Seems like it might be easier to just `if` out the line that includes this `CMakeLists.txt`?

It's easier to reason about the build structure if all the `CMakeLists.txt` files are included.
 i.e. Anyone working on source code in the same directory just has to modify this one CMakeLists.txt,
instead of checking in all parent directories for potential inclusion.

The one exception for this is if there are platform specific folders, such as the `stout/os/windows/*`
folders.  (We'll be removing the Windows-only exclusion of tests shortly after this review
chain too.)


- Joseph


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


On Nov. 15, 2016, 2:56 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53756/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2016, 2:56 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
> -------
> 
> This review replaces: https://reviews.apache.org/r/49874/
> 
> 
> Diffs
> -----
> 
>   src/slave/CMakeLists.txt 33120ace79bce449329a7cc4b7ef264d2867fc13 
>   src/slave/cmake/SlaveConfigure.cmake b339239761a5de321d65b92376dae69c339bee5c 
>   src/slave/container_loggers/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53756/diff/
> 
> 
> Testing
> -------
> 
> cmake ..
> make
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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