Re: Review Request 71852: Used CMake's automatic parallelization in mesos-tidy setup.

2019-12-06 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [71852]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Dec. 6, 2019, 4:23 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71852/
> ---
> 
> (Updated Dec. 6, 2019, 4:23 p.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the `--parallel` flag for `cmake build` is only supported in more
> recent CMake versions we bump the installed version to the latest
> release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile 7d17a82c023c614016e771c305e3a114731eb832 
>   support/mesos-tidy/entrypoint.sh 7d2225d8f75998ee13acf0bd57c9483dfc7acd98 
> 
> 
> Diff: https://reviews.apache.org/r/71852/diff/2/
> 
> 
> Testing
> ---
> 
> Built a local image and tested it with a modified `support/mesos-tidy.sh`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71852: Used CMake's automatic parallelization in mesos-tidy setup.

2019-12-06 Thread Benjamin Bannier 


> On Dec. 6, 2019, 12:51 p.m., Benno Evers wrote:
> > support/mesos-tidy/entrypoint.sh
> > Line 38 (original), 38 (patched)
> > 
> >
> > From 
> > 
> > https://cmake.org/cmake/help/v3.12/release/3.12.html#command-line
> > 
> > it sounds like the `--parallel` option still wants a parameter to set 
> > the number of cores? If that is not required, maybe you could add a link to 
> > the cmake documentation in the commit message?

The release notes look incorrect to me. The documentation for this flag is 
here, 
https://cmake.org/cmake/help/latest/manual/cmake.1.html#index-0-envvar:CMAKE_BUILD_PARALLEL_LEVEL.
 While the documentation states when no explicit number is given the default 
parallelism level of the choosen build tool would be used, this still built 
fast for me (even though we build with `make` which by default does not 
parallelize). I added an explicit setting to make this less surprising ;)


- Benjamin


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


On Dec. 6, 2019, 5:23 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71852/
> ---
> 
> (Updated Dec. 6, 2019, 5:23 p.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the `--parallel` flag for `cmake build` is only supported in more
> recent CMake versions we bump the installed version to the latest
> release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile 7d17a82c023c614016e771c305e3a114731eb832 
>   support/mesos-tidy/entrypoint.sh 7d2225d8f75998ee13acf0bd57c9483dfc7acd98 
> 
> 
> Diff: https://reviews.apache.org/r/71852/diff/2/
> 
> 
> Testing
> ---
> 
> Built a local image and tested it with a modified `support/mesos-tidy.sh`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71852: Used CMake's automatic parallelization in mesos-tidy setup.

2019-12-06 Thread Benjamin Bannier 

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

(Updated Dec. 6, 2019, 5:23 p.m.)


Review request for mesos and Benno Evers.


Repository: mesos


Description
---

Since the `--parallel` flag for `cmake build` is only supported in more
recent CMake versions we bump the installed version to the latest
release.


Diffs (updated)
-

  support/mesos-tidy/Dockerfile 7d17a82c023c614016e771c305e3a114731eb832 
  support/mesos-tidy/entrypoint.sh 7d2225d8f75998ee13acf0bd57c9483dfc7acd98 


Diff: https://reviews.apache.org/r/71852/diff/2/

Changes: https://reviews.apache.org/r/71852/diff/1-2/


Testing
---

Built a local image and tested it with a modified `support/mesos-tidy.sh`.


Thanks,

Benjamin Bannier

Re: Review Request 71852: Used CMake's automatic parallelization in mesos-tidy setup.

2019-12-06 Thread Benno Evers 

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


Fix it, then Ship it!





support/mesos-tidy/entrypoint.sh
Line 38 (original), 38 (patched)


From 

https://cmake.org/cmake/help/v3.12/release/3.12.html#command-line

it sounds like the `--parallel` option still wants a parameter to set the 
number of cores? If that is not required, maybe you could add a link to the 
cmake documentation in the commit message?


- Benno Evers


On Dec. 3, 2019, 1:12 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71852/
> ---
> 
> (Updated Dec. 3, 2019, 1:12 p.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the `--parallel` flag for `cmake build` is only supported in more
> recent CMake versions we bump the installed version to the latest
> release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile 7d17a82c023c614016e771c305e3a114731eb832 
>   support/mesos-tidy/entrypoint.sh 7d2225d8f75998ee13acf0bd57c9483dfc7acd98 
> 
> 
> Diff: https://reviews.apache.org/r/71852/diff/1/
> 
> 
> Testing
> ---
> 
> Built a local image and tested it with a modified `support/mesos-tidy.sh`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71852: Used CMake's automatic parallelization in mesos-tidy setup.

2019-12-03 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [71852]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Dec. 3, 2019, 1:12 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71852/
> ---
> 
> (Updated Dec. 3, 2019, 1:12 p.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since the `--parallel` flag for `cmake build` is only supported in more
> recent CMake versions we bump the installed version to the latest
> release.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/Dockerfile 7d17a82c023c614016e771c305e3a114731eb832 
>   support/mesos-tidy/entrypoint.sh 7d2225d8f75998ee13acf0bd57c9483dfc7acd98 
> 
> 
> Diff: https://reviews.apache.org/r/71852/diff/1/
> 
> 
> Testing
> ---
> 
> Built a local image and tested it with a modified `support/mesos-tidy.sh`.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>