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

2017-12-04 Thread Joseph Wu


> On Dec. 4, 2017, 1:36 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 376 (patched)
> > 
> >
> > 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.

2017-12-04 Thread Andrew Schwartzmeyer


> On Dec. 4, 2017, 1:36 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 376 (patched)
> > 
> >
> > 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.

2017-12-04 Thread Benjamin Bannier

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




cmake/CompilationConfigure.cmake
Lines 376 (patched)


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
> 
>



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

2017-12-04 Thread Joseph Wu

---
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.

2017-12-04 Thread Andrew Schwartzmeyer

---
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.

2017-11-06 Thread Andrew Schwartzmeyer

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

(Updated Nov. 6, 2017, 10:34 a.m.)


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


Changes
---

Added guard to avoid having to always define `BUILD_JAVA_JVM_LIBRARY`.


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 b0e223121f24ac0dc604dc2c639158b817eec535 
  src/common/build.cpp 4192b89cbee1c9d7a75213f55b189565cd8a10f1 
  src/common/build_config.hpp.in ac7059ab2aa54011197b9e1a46f9d7fd39ac39c6 


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

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


Testing
---

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


Thanks,

Andrew Schwartzmeyer