mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <bbann...@apache.org>
Subject Re: Review Request 71241: Added a `distcheck` target to the cmake build.
Date Tue, 03 Sep 2019 13:52:38 GMT


> On Sept. 3, 2019, 3:08 p.m., Benno Evers wrote:
> > cmake/distcheck.sh
> > Lines 25 (patched)
> > <https://reviews.apache.org/r/71241/diff/1/?file=2159995#file2159995line25>
> >
> >     I don't think we should unconditionally override the users environment like
this in a support script - there's no guarantee that `clang` or `ccache` are even installed.
> >     
> >     If we have to specify `CMAKE_CXX_COMPILER` etc. at all, we should probably use
`${CXX}` as default value and make sure that cmake sets the correct values when invoking the
script.

Thanks for catching this. This isn't only not nice, but shouldn't have been there at all;
I simply forgot implementing this part.

What I do now is to pass a shell variable `DISTCHECK_CMAKE_ARGS` (modelled after autoconf's
`DISTCHECK_CONFIGURE_ARGS`) which can be filled with the kinds of arguments I passed above.

```
cmake .. ${DISTCHECK_CMAKE_FLAGS}
```


- Benjamin


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


On Sept. 3, 2019, 3:52 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71241/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2019, 3:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds a `distcheck` target to the cmake build which mimics the
> target of the same name provided by the autotools build. `distcheck`
> depends on the `dist` target to create a distribution archive and then
> ensures that with the distributed sources the `check` targets succeeds.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt dc50dd457585c14a456ca710da4a16a0d45ef17a 
>   cmake/distcheck.sh PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71241/diff/2/
> 
> 
> Testing
> -------
> 
> `ninja distcheck` passes, no artifacts left around but tarball.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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