You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/02/25 00:31:38 UTC
Review Request 57052: CMake: Add `config.hpp.in` for `BUILD_TIME`
variables.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/
-----------------------------------------------------------
Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
Bugs: MESOS-7172
https://issues.apache.org/jira/browse/MESOS-7172
Repository: mesos
Description
-------
Commit c7fc1377b introduced a bug that prevented any incremental
recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
every build was seen as having a root dependency (the flags)
changed, causing a rebuild of every single file (including
dependencies).
This patch introduces a CMake `configure_file` directive which
takes in `config.hpp.in` and emits `config.hpp` with the
`BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
It also sets `HAVE_CONFIG_H` which `build.cpp` uses to `#include`
the configuration file. The result is that the date, time, and user
are set at the point of configuration (invocation of CMake) instead
of at build, thus allowing for incremental rebuilds.
As `config.hpp` is auto-generated by the CMake configuration,
it is ignored by Git.
Diffs
-----
cmake/CompilationConfigure.cmake ed727e6a679e718f88f158faba9fecc3061ac700
src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
src/common/config.hpp.in PRE-CREATION
support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25
Diff: https://reviews.apache.org/r/57052/diff/
Testing
-------
cmake; make && make check on Linux
cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Line 291 (original), 291 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652318#file1652318line291>
> >
> > Not yours, but looks like this one actually doesn't work on Posix.
> >
> > It ends up as:
> > ```
> > #define BUILD_TIME "%s"
> > ```
> >
> > I'll fix it while I'm modifying this part of the build system:
> > ```
> > execute_process(
> > COMMAND date +%s
> > OUTPUT_VARIABLE BUILD_TIME
> > OUTPUT_STRIP_TRAILING_WHITESPACE
> > )
> > ```
I believe this is a CMake versioning issue. I had to explicitly call `cmake3` for this to work (but AFAIK we don't "officially" support 2.x, I say that because we already make use of a lot of 3.x features).
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 297 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652318#file1652318line297>
> >
> > Would be better with a comment :D
> > ```
> > # Emit the BUILD_DATE, BUILD_TIME, and BUILD_USER variables into a file.
> > # This will be updated each time `cmake` is run.
> > ```
Oh, yes it would :D
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 299 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652318#file1652318line299>
> >
> > Just for symmetry, let's put this in
> >
> > "${CMAKE_BINARY_DIR}/src/common/build_config.hpp"
I'm fine with that if you were. I went with include/mesos as that's the only currently generated folder inside build/include. Definitely prefer include/common.
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Line 308 (original), 312 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652318#file1652318line312>
> >
> > What do you feel about "USE_CMAKE_BUILD_CONFIG" instead? "HAVE_BUILD_CONFIG_H" seems too much like a header guard.
No preference. `HAVE_CONFIG_H` is very common (the `HAVE_FEATURE` is itself a common pattern), but I don't care one way or another.
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > src/common/build.cpp
> > Lines 26 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652319#file1652319line26>
> >
> > Would help to have a nice beefy comment :)
> > ```
> > // NOTE: On CMake, instead of defining `BUILD_DATE|TIME|USER` as
> > // compiler flags, we instead emit a header file with the definitions.
> > // This facilitates incremental builds as the compiler flags will
> > // no longer change with every invocation of the build.
> > // TODO(josephw): After deprecating autotools, remove this guard.
> > ```
I knew there was something I was forgetting.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167198
-----------------------------------------------------------
On March 1, 2017, 12:42 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated March 1, 2017, 12:42 a.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single source in Mesos.
>
> This patch introduces a CMake `configure_file` directive which
> takes in `build_config.hpp.in` and emits `build_config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_BUILD_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/build_config.hpp.in PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/57052/diff/2/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
> On March 1, 2017, 2:13 a.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Line 291 (original), 291 (patched)
> > <https://reviews.apache.org/r/57052/diff/2/?file=1652318#file1652318line291>
> >
> > Not yours, but looks like this one actually doesn't work on Posix.
> >
> > It ends up as:
> > ```
> > #define BUILD_TIME "%s"
> > ```
> >
> > I'll fix it while I'm modifying this part of the build system:
> > ```
> > execute_process(
> > COMMAND date +%s
> > OUTPUT_VARIABLE BUILD_TIME
> > OUTPUT_STRIP_TRAILING_WHITESPACE
> > )
> > ```
>
> Andrew Schwartzmeyer wrote:
> I believe this is a CMake versioning issue. I had to explicitly call `cmake3` for this to work (but AFAIK we don't "officially" support 2.x, I say that because we already make use of a lot of 3.x features).
Yup 2.8.10's `string` command [does not support `TIMESTAMP`] (https://cmake.org/cmake/help/v2.8.10/cmake.html#command:string). I'll get an issue filed; but we need to move the non-Windows minimum required version up to CMake 3.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167198
-----------------------------------------------------------
On March 1, 2017, 10:18 p.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated March 1, 2017, 10:18 p.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single source in Mesos.
>
> This patch introduces a CMake `configure_file` directive which
> takes in `build_config.hpp.in` and emits `build_config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `USE_CMAKE_BUILD_CONFIG` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/build_config.hpp.in PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/57052/diff/3/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167198
-----------------------------------------------------------
Ship it!
I'll make the changes and ship this if you agree with the renaming (see below).
cmake/CompilationConfigure.cmake (line 291)
<https://reviews.apache.org/r/57052/#comment239367>
Not yours, but looks like this one actually doesn't work on Posix.
It ends up as:
```
#define BUILD_TIME "%s"
```
I'll fix it while I'm modifying this part of the build system:
```
execute_process(
COMMAND date +%s
OUTPUT_VARIABLE BUILD_TIME
OUTPUT_STRIP_TRAILING_WHITESPACE
)
```
cmake/CompilationConfigure.cmake (line 297)
<https://reviews.apache.org/r/57052/#comment239369>
Would be better with a comment :D
```
# Emit the BUILD_DATE, BUILD_TIME, and BUILD_USER variables into a file.
# This will be updated each time `cmake` is run.
```
cmake/CompilationConfigure.cmake (line 299)
<https://reviews.apache.org/r/57052/#comment239368>
Just for symmetry, let's put this in
"${CMAKE_BINARY_DIR}/src/common/build_config.hpp"
cmake/CompilationConfigure.cmake (line 312)
<https://reviews.apache.org/r/57052/#comment239366>
What do you feel about "USE_CMAKE_BUILD_CONFIG" instead? "HAVE_BUILD_CONFIG_H" seems too much like a header guard.
src/common/build.cpp (line 26)
<https://reviews.apache.org/r/57052/#comment239371>
Would help to have a nice beefy comment :)
```
// NOTE: On CMake, instead of defining `BUILD_DATE|TIME|USER` as
// compiler flags, we instead emit a header file with the definitions.
// This facilitates incremental builds as the compiler flags will
// no longer change with every invocation of the build.
// TODO(josephw): After deprecating autotools, remove this guard.
```
- Joseph Wu
On Feb. 28, 2017, 4:42 p.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2017, 4:42 p.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single source in Mesos.
>
> This patch introduces a CMake `configure_file` directive which
> takes in `build_config.hpp.in` and emits `build_config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_BUILD_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/build_config.hpp.in PRE-CREATION
>
> Diff: https://reviews.apache.org/r/57052/diff/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167283
-----------------------------------------------------------
Patch looks great!
Reviews applied: [57052]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 1, 2017, 12:42 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated March 1, 2017, 12:42 a.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single source in Mesos.
>
> This patch introduces a CMake `configure_file` directive which
> takes in `build_config.hpp.in` and emits `build_config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_BUILD_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/build_config.hpp.in PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/57052/diff/2/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/
-----------------------------------------------------------
(Updated March 1, 2017, 10:18 p.m.)
Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
Changes
-------
Update per Joe's comments; also add license and header guard to `build_config.hpp`.
Bugs: MESOS-7172
https://issues.apache.org/jira/browse/MESOS-7172
Repository: mesos
Description (updated)
-------
Commit c7fc1377b introduced a bug that prevented any incremental
recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
every build was seen as having a root dependency (the flags)
changed, causing a rebuild of every single source in Mesos.
This patch introduces a CMake `configure_file` directive which
takes in `build_config.hpp.in` and emits `build_config.hpp` with the
`BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
It also sets `USE_CMAKE_BUILD_CONFIG` which `build.cpp` uses to `#include`
the configuration file. The result is that the date, time, and user
are set at the point of configuration (invocation of CMake) instead
of at build, thus allowing for incremental rebuilds.
Diffs (updated)
-----
cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
src/common/build_config.hpp.in PRE-CREATION
Diff: https://reviews.apache.org/r/57052/diff/3/
Changes: https://reviews.apache.org/r/57052/diff/2-3/
Testing
-------
cmake; make && make check on Linux
cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 57052: CMake: Add `build_config.hpp.in` for
`BUILD_TIME` variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/
-----------------------------------------------------------
(Updated March 1, 2017, 12:42 a.m.)
Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
Changes
-------
Emit build_config.hpp into build directory, instead of source directory. Rename config to build_config.
Summary (updated)
-----------------
CMake: Add `build_config.hpp.in` for `BUILD_TIME` variables.
Bugs: MESOS-7172
https://issues.apache.org/jira/browse/MESOS-7172
Repository: mesos
Description (updated)
-------
Commit c7fc1377b introduced a bug that prevented any incremental
recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
every build was seen as having a root dependency (the flags)
changed, causing a rebuild of every single source in Mesos.
This patch introduces a CMake `configure_file` directive which
takes in `build_config.hpp.in` and emits `build_config.hpp` with the
`BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
It also sets `HAVE_BUILD_CONFIG_H` which `build.cpp` uses to `#include`
the configuration file. The result is that the date, time, and user
are set at the point of configuration (invocation of CMake) instead
of at build, thus allowing for incremental rebuilds.
Diffs (updated)
-----
cmake/CompilationConfigure.cmake 7808d1a0086a7999991af8dcb2dd796b9424c7d5
src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
src/common/build_config.hpp.in PRE-CREATION
Diff: https://reviews.apache.org/r/57052/diff/
Testing
-------
cmake; make && make check on Linux
cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 57052: CMake: Add `config.hpp.in` for `BUILD_TIME`
variables.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Feb. 28, 2017, 10:05 a.m., Andrew Schwartzmeyer wrote:
> > support/gitignore, line 36
> > <https://reviews.apache.org/r/57052/diff/1/?file=1647633#file1647633line36>
> >
> > Ah, note to myself: update the output to go only into `build` instead of into `${CMAKE_SOURCE_DIR}`.
`${CMAKE_BINARY_DIR}` is the magic variable you'll want to use.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167109
-----------------------------------------------------------
On Feb. 24, 2017, 4:31 p.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2017, 4:31 p.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single file (including
> dependencies).
>
> This patch introduces a CMake `configure_file` directive which
> takes in `config.hpp.in` and emits `config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
> As `config.hpp` is auto-generated by the CMake configuration,
> it is ignored by Git.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake ed727e6a679e718f88f158faba9fecc3061ac700
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/config.hpp.in PRE-CREATION
> support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25
>
> Diff: https://reviews.apache.org/r/57052/diff/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `config.hpp.in` for `BUILD_TIME`
variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
> On Feb. 28, 2017, 6:05 p.m., Andrew Schwartzmeyer wrote:
> > support/gitignore, line 36
> > <https://reviews.apache.org/r/57052/diff/1/?file=1647633#file1647633line36>
> >
> > Ah, note to myself: update the output to go only into `build` instead of into `${CMAKE_SOURCE_DIR}`.
>
> Joseph Wu wrote:
> `${CMAKE_BINARY_DIR}` is the magic variable you'll want to use.
Yes indeed it is; sorry, a bit busy with other stuff today.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167109
-----------------------------------------------------------
On Feb. 25, 2017, 12:31 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2017, 12:31 a.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single file (including
> dependencies).
>
> This patch introduces a CMake `configure_file` directive which
> takes in `config.hpp.in` and emits `config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
> As `config.hpp` is auto-generated by the CMake configuration,
> it is ignored by Git.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake ed727e6a679e718f88f158faba9fecc3061ac700
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/config.hpp.in PRE-CREATION
> support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25
>
> Diff: https://reviews.apache.org/r/57052/diff/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 57052: CMake: Add `config.hpp.in` for `BUILD_TIME`
variables.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57052/#review167109
-----------------------------------------------------------
cmake/CompilationConfigure.cmake (line 298)
<https://reviews.apache.org/r/57052/#comment239242>
Joseph Wu brought up a point that this _may_ not update the output file `config.hpp` on each invocation of CMake, as `config.hpp.in` itself doesn't change.
However, I double checked and with CMake 3.7.1 at least, it does indeed behave as I expected. Each direction invocation of `cmake` reruns the CMake command `configure_file` which has different input due to `BUILD_TIME` changing, and so updates the file.
While the semantics _slightly_ change from 'compilation time' to 'configuration time', we agree this is acceptable.
support/gitignore (line 36)
<https://reviews.apache.org/r/57052/#comment239243>
Ah, note to myself: update the output to go only into `build` instead of into `${CMAKE_SOURCE_DIR}`.
- Andrew Schwartzmeyer
On Feb. 25, 2017, 12:31 a.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57052/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2017, 12:31 a.m.)
>
>
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
>
>
> Bugs: MESOS-7172
> https://issues.apache.org/jira/browse/MESOS-7172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Commit c7fc1377b introduced a bug that prevented any incremental
> recompilation. By defining the `BUILD_TIME` in `MESOS_CPPFLAGS`,
> every build was seen as having a root dependency (the flags)
> changed, causing a rebuild of every single file (including
> dependencies).
>
> This patch introduces a CMake `configure_file` directive which
> takes in `config.hpp.in` and emits `config.hpp` with the
> `BUILD_TIME`, `BUILD_DATE`, and `BUILD_USER` variables defined.
> It also sets `HAVE_CONFIG_H` which `build.cpp` uses to `#include`
> the configuration file. The result is that the date, time, and user
> are set at the point of configuration (invocation of CMake) instead
> of at build, thus allowing for incremental rebuilds.
>
> As `config.hpp` is auto-generated by the CMake configuration,
> it is ignored by Git.
>
>
> Diffs
> -----
>
> cmake/CompilationConfigure.cmake ed727e6a679e718f88f158faba9fecc3061ac700
> src/common/build.cpp 090b59fb22bfc00516d65f501f8f18cd85a5b4cd
> src/common/config.hpp.in PRE-CREATION
> support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25
>
> Diff: https://reviews.apache.org/r/57052/diff/
>
>
> Testing
> -------
>
> cmake; make && make check on Linux
> cmake; VS build stout-tests and mesos-tests, then repeat and see no compilation take place a second time. Also support\windows-build.bat twice.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>