You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2019/12/03 13:12:45 UTC
Review Request 71852: Used CMake's automatic parallelization in
mesos-tidy setup.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71852/
-----------------------------------------------------------
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.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 71852: Used CMake's automatic parallelization in
mesos-tidy setup.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Dec. 6, 2019, 12:51 p.m., Benno Evers wrote:
> > support/mesos-tidy/entrypoint.sh
> > Line 38 (original), 38 (patched)
> > <https://reviews.apache.org/r/71852/diff/1/?file=2181662#file2181662line38>
> >
> > 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.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/71852/#comment306929>
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.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
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.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
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