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/12/04 21:19:31 UTC

Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

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

(Updated Dec. 4, 2017, 1:19 p.m.)


Review request for mesos, Benjamin Bannier, Jeff Coffler, John Kordich, Joseph Wu, and Michael Park.


Changes
-------

Rebased.


Bugs: MESOS-5455
    https://issues.apache.org/jira/browse/MESOS-5455


Repository: mesos


Description
-------

This resolves MESOS-5455, and consolidates the `BUILD` variables into
one location.


Diffs (updated)
-----

  cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
  src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
  src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
  src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 


Diff: https://reviews.apache.org/r/63341/diff/4/

Changes: https://reviews.apache.org/r/63341/diff/3-4/


Testing
-------

Checked the emitted `build_config.hpp` under various conditions; flags are set correctly (and can still build of course).


Thanks,

Andrew Schwartzmeyer


Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63341/#review192766
-----------------------------------------------------------


Ship it!




Ship It!

- Joseph Wu


On Dec. 4, 2017, 1:19 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63341/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 1:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, John Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-5455
>     https://issues.apache.org/jira/browse/MESOS-5455
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This resolves MESOS-5455, and consolidates the `BUILD` variables into
> one location.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
>   src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
>   src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
>   src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 
> 
> 
> Diff: https://reviews.apache.org/r/63341/diff/4/
> 
> 
> Testing
> -------
> 
> Checked the emitted `build_config.hpp` under various conditions; flags are set correctly (and can still build of course).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Dec. 4, 2017, 1:36 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 376 (patched)
> > <https://reviews.apache.org/r/63341/diff/4/?file=1908336#file1908336line376>
> >
> >     I cannot see where `BUILD_FLAGS_RAW` is actually set. Is the intention of the user to set this?
> 
> Andrew Schwartzmeyer wrote:
>     This is where CMake gets weird. This command sets `BUILD_FLAGS_RAW` with the content of the `COMPILE_DEFINITIONS` property (for the directory, and it's not a variable so I can't use it directly). I'll add a comment to this effect.

https://cmake.org/cmake/help/v3.0/command/get_directory_property.html

That command is setting `BUILD_FLAGS_RAW` to the value of `COMPILE_DEFINITIONS` (which is a special variable whose value may change depending on the current directory, so it should be accessed in this way).


- Joseph


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


On Dec. 4, 2017, 1:19 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63341/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 1:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, John Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-5455
>     https://issues.apache.org/jira/browse/MESOS-5455
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This resolves MESOS-5455, and consolidates the `BUILD` variables into
> one location.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
>   src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
>   src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
>   src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 
> 
> 
> Diff: https://reviews.apache.org/r/63341/diff/4/
> 
> 
> Testing
> -------
> 
> Checked the emitted `build_config.hpp` under various conditions; flags are set correctly (and can still build of course).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

> On Dec. 4, 2017, 1:36 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 376 (patched)
> > <https://reviews.apache.org/r/63341/diff/4/?file=1908336#file1908336line376>
> >
> >     I cannot see where `BUILD_FLAGS_RAW` is actually set. Is the intention of the user to set this?

This is where CMake gets weird. This command sets `BUILD_FLAGS_RAW` with the content of the `COMPILE_DEFINITIONS` property (for the directory, and it's not a variable so I can't use it directly). I'll add a comment to this effect.


- Andrew


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


On Dec. 4, 2017, 1:19 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63341/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 1:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, John Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-5455
>     https://issues.apache.org/jira/browse/MESOS-5455
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This resolves MESOS-5455, and consolidates the `BUILD` variables into
> one location.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
>   src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
>   src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
>   src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 
> 
> 
> Diff: https://reviews.apache.org/r/63341/diff/4/
> 
> 
> Testing
> -------
> 
> Checked the emitted `build_config.hpp` under various conditions; flags are set correctly (and can still build of course).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>


Re: Review Request 63341: Set `BUILD_FLAGS` flag in CMake.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63341/#review192768
-----------------------------------------------------------




cmake/CompilationConfigure.cmake
Lines 376 (patched)
<https://reviews.apache.org/r/63341/#comment271022>

    I cannot see where `BUILD_FLAGS_RAW` is actually set. Is the intention of the user to set this?


- Benjamin Bannier


On Dec. 4, 2017, 10:19 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63341/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 10:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jeff Coffler, John Kordich, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-5455
>     https://issues.apache.org/jira/browse/MESOS-5455
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This resolves MESOS-5455, and consolidates the `BUILD` variables into
> one location.
> 
> 
> Diffs
> -----
> 
>   cmake/CompilationConfigure.cmake deb574200bceb605a60b536a396c98d9247c6018 
>   src/CMakeLists.txt 592489df070a0c4fdee814a4a7b613e62a544f88 
>   src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
>   src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 
> 
> 
> Diff: https://reviews.apache.org/r/63341/diff/4/
> 
> 
> Testing
> -------
> 
> Checked the emitted `build_config.hpp` under various conditions; flags are set correctly (and can still build of course).
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>