mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 68319: Put binaries into root folder in libprocess.
Date Mon, 20 Aug 2018 22:19:22 GMT


> On Aug. 20, 2018, 11:18 a.m., Andrew Schwartzmeyer wrote:
> > I have to ask: why? I've personally thought it would be nicer to remove this rather
unexpected logic (from a CMake standpoint) from Mesos. Why add it to stout and libprocess?
> 
> Benjamin Bannier wrote:
>     Thanks for engaging. The main idea would be to make the various binaries easier to
run, e.g., when manually executing tests. IMO the alternative to the patches proposed here
would be to all put them into a shared, _global_ folder -- many projects use bin/ for that.
> 
> Andrew Schwartzmeyer wrote:
>     We could do that in an `install` step, which seems to be CMake's "canonical" method
of aggregating the binaries.
>     
>     Re: manually executing tests: there is also `ctest`. So you can use ` ctest -R Stout`
to run the `StoutTests` regardless of where it is (which is nice because you also don't have
to remember that it is `stout-tests.exe` on Windows). Though passing like `--gtest_filter`
etc. I'm not so sure about...
> 
> Benjamin Bannier wrote:
>     Fair enough. I thought this might be useful, e.g., when running tests with parallel
test runners or for the mentioned filtering, but I agree with you that adding this extra complexity
to the build setup isn't really worth it (it might even confused multiconfig generators).
>     
>     As an example, llvm _does_ put produced binaries into a dedicated file, but they
have a dedicated function to deal with multiconfig generators instead of directly setting
this global variable.
>     
>     I'll drop the chain.

I do like where you were going with it though. What we should do is get rid of this part of
cruft from the Mesos CMake files, and make sure `make install` gives us a folder of stuff
we want (though I imagine actually that we would want to ignore tests).


- Andrew


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


On Aug. 20, 2018, 2:53 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68319/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2018, 2:53 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Put binaries into root folder in libprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/CMakeLists.txt 5bba8e2c59ea3d7c37d186a2273bcaad6ffb5c46 
>   3rdparty/libprocess/src/tests/CMakeLists.txt a03a77eb5e289b4daac0bbd414dc17c8acc848dc

> 
> 
> Diff: https://reviews.apache.org/r/68319/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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