subject:"Re\: Review Request 66163\: Built storage local resource provider with CMake."

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-20 Thread Andrew Schwartzmeyer


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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 19, 2018, 7:34 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 19, 2018, 7:34 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/5/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-19 Thread Chun-Hung Hsiao


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

(Updated April 20, 2018, 2:34 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Enabled Windows Build.


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


Repository: mesos


Description
---

This patch adds CMake rules for compiling necessary source code for
building storage local resource provider and related tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 173089a1ebf80a6288431075770a807235524ac1 
  src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
  src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
  src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
  src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 


Diff: https://reviews.apache.org/r/66163/diff/5/

Changes: https://reviews.apache.org/r/66163/diff/4-5/


Testing
---

`sudo make check` with CMake


Thanks,

Chun-Hung Hsiao

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-19 Thread Chun-Hung Hsiao



> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 387-390 (patched)
> > 
> >
> > Do you know which targets actually require `ENABLE_GRPC` set as a 
> > pre-processor definition? Since it's new code being added, we should be 
> > able to resovle this TODO before committing.
> 
> Chun-Hung Hsiao wrote:
> For now it's just `src/resource_provider/local.cpp`. What's the best way 
> to achieve this? IMHO it's not that bad to make this global. ;)
> 
> Andrew Schwartzmeyer wrote:
> Change
> ```
> target_compile_definitions(mesos PUBLIC USE_CMAKE_BUILD_CONFIG)
> ```
> to
> ```
> target_compile_definitions(
>   mesos
>   PUBLIC USE_CMAKE_BUILD_CONFIG
>   PRIVATE ENABLE_GRPC)
> ```
> 
> But yeah, we still have `add_definitions(-DUSE_SSL_SOCKET=1)`, so this 
> can be a `TODO`.

Actually I'm not sure if having an explicit list of targets that the definition 
takes effects would reduce the cost of code maintanance or it would increase 
the cost. It seems to me that people that are unfamiliar with this list could 
easily run into a case where a definition doesn't take effects on a new target 
and they will spend a lot of time debugging it.


- Chun-Hung


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


On April 17, 2018, 3:20 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 17, 2018, 3:20 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-18 Thread Chun-Hung Hsiao



> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/CMakeLists.txt
> > Line 237 (original), 250-254 (patched)
> > 
> >
> > So `uri_disk_profile_adaptor` is, what, a module? that depends on (and 
> > links in `mesos`), but then `mesos-tess-interface` (which links in `mesos`) 
> > also links in `uri_disk_profile_adaptor`. So if things are being linked 
> > statically, `mesos-tests` would then then have `libmesos` twice? Sorry, I'm 
> > just trying to sort out the dependency graph in my head. Can you elaborate?
> 
> Chun-Hung Hsiao wrote:
> We have the same dependencies for `load_qos_controller`, 
> `fixed_resource_estimator` and `logrotate_container_logger` as well. All the 
> four modules depends on `libmesos` apperantly, and `mesos-tests-interface` 
> also depends on `libmesos`. And since the tests needs these modules, 
> `mesos-tests-interface` again depends on all of these modules.
> 
> I guess we could remove `mesos` above but it looks weird. Also I think 
> there should be no problem to "link" the same `libmesos` twice. This doesn't 
> violade the one-definition rule.
> 
> Andrew Schwartzmeyer wrote:
> Oh, I see. Keep in mind we don't support modules on Windows yet in 
> general... should this be under the NOT WIN32 target_link_libraries?

Hmm that also means the `disk_profile_adaptor_tests.cpp` should be 
conditionally compiled. Will do.


- Chun-Hung


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


On April 17, 2018, 3:20 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 17, 2018, 3:20 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-18 Thread Andrew Schwartzmeyer



> On April 12, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/CMakeLists.txt
> > Line 237 (original), 250-254 (patched)
> > 
> >
> > So `uri_disk_profile_adaptor` is, what, a module? that depends on (and 
> > links in `mesos`), but then `mesos-tess-interface` (which links in `mesos`) 
> > also links in `uri_disk_profile_adaptor`. So if things are being linked 
> > statically, `mesos-tests` would then then have `libmesos` twice? Sorry, I'm 
> > just trying to sort out the dependency graph in my head. Can you elaborate?
> 
> Chun-Hung Hsiao wrote:
> We have the same dependencies for `load_qos_controller`, 
> `fixed_resource_estimator` and `logrotate_container_logger` as well. All the 
> four modules depends on `libmesos` apperantly, and `mesos-tests-interface` 
> also depends on `libmesos`. And since the tests needs these modules, 
> `mesos-tests-interface` again depends on all of these modules.
> 
> I guess we could remove `mesos` above but it looks weird. Also I think 
> there should be no problem to "link" the same `libmesos` twice. This doesn't 
> violade the one-definition rule.

Oh, I see. Keep in mind we don't support modules on Windows yet in general... 
should this be under the NOT WIN32 target_link_libraries?


- Andrew


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


On April 16, 2018, 8:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 16, 2018, 8:20 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-18 Thread Andrew Schwartzmeyer



> On April 12, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 387-390 (patched)
> > 
> >
> > Do you know which targets actually require `ENABLE_GRPC` set as a 
> > pre-processor definition? Since it's new code being added, we should be 
> > able to resovle this TODO before committing.
> 
> Chun-Hung Hsiao wrote:
> For now it's just `src/resource_provider/local.cpp`. What's the best way 
> to achieve this? IMHO it's not that bad to make this global. ;)

Change
```
target_compile_definitions(mesos PUBLIC USE_CMAKE_BUILD_CONFIG)
```
to
```
target_compile_definitions(
  mesos
  PUBLIC USE_CMAKE_BUILD_CONFIG
  PRIVATE ENABLE_GRPC)
```

But yeah, we still have `add_definitions(-DUSE_SSL_SOCKET=1)`, so this can be a 
`TODO`.


> On April 12, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote:
> > src/resource_provider/storage/CMakeLists.txt
> > Lines 18 (patched)
> > 
> >
> > Nit: I've been aligning the `###`... but I know it's that important.
> 
> Chun-Hung Hsiao wrote:
> Oops sorry my bad.

Know it's _not_ that important (is what I meant to say)


- Andrew


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


On April 16, 2018, 8:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 16, 2018, 8:20 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-16 Thread Chun-Hung Hsiao



> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 387-390 (patched)
> > 
> >
> > Do you know which targets actually require `ENABLE_GRPC` set as a 
> > pre-processor definition? Since it's new code being added, we should be 
> > able to resovle this TODO before committing.

For now it's just `src/resource_provider/local.cpp`. What's the best way to 
achieve this? IMHO it's not that bad to make this global. ;)


> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > src/resource_provider/storage/CMakeLists.txt
> > Lines 18 (patched)
> > 
> >
> > Nit: I've been aligning the `###`... but I know it's that important.

Oops sorry my bad.


> On April 12, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/CMakeLists.txt
> > Line 237 (original), 250-254 (patched)
> > 
> >
> > So `uri_disk_profile_adaptor` is, what, a module? that depends on (and 
> > links in `mesos`), but then `mesos-tess-interface` (which links in `mesos`) 
> > also links in `uri_disk_profile_adaptor`. So if things are being linked 
> > statically, `mesos-tests` would then then have `libmesos` twice? Sorry, I'm 
> > just trying to sort out the dependency graph in my head. Can you elaborate?

We have the same dependencies for `load_qos_controller`, 
`fixed_resource_estimator` and `logrotate_container_logger` as well. All the 
four modules depends on `libmesos` apperantly, and `mesos-tests-interface` also 
depends on `libmesos`. And since the tests needs these modules, 
`mesos-tests-interface` again depends on all of these modules.

I guess we could remove `mesos` above but it looks weird. Also I think there 
should be no problem to "link" the same `libmesos` twice. This doesn't violade 
the one-definition rule.


- Chun-Hung


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


On April 17, 2018, 3:20 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 17, 2018, 3:20 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
>   src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-16 Thread Chun-Hung Hsiao


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

(Updated April 17, 2018, 3:20 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Partially addressed Andy's comments.


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


Repository: mesos


Description
---

This patch adds CMake rules for compiling necessary source code for
building storage local resource provider and related tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 08d154e6e4b718b6059afb70e0b0235d1725d72d 
  src/CMakeLists.txt 31af9eaf302122b844e7599944097ad1c3faae3a 
  src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
  src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
  src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 


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

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


Testing
---

`sudo make check` with CMake


Thanks,

Chun-Hung Hsiao

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-12 Thread Andrew Schwartzmeyer



> On April 12, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote:
> > src/examples/CMakeLists.txt
> > Lines 55-59 (patched)
> > 
> >
> > What about non-Linux but not Windows platforms, e.g. MacOS and FreeBSD?

Saw in prior revision that it explicitly is only Linux because it includes 
`linux/fs.hpp`, maybe change the note that it's not non-Windows, but explicitly 
Linux specific.


- Andrew


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


On April 11, 2018, 10:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 11, 2018, 10:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 
>   src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-12 Thread Andrew Schwartzmeyer

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

cmake/CompilationConfigure.cmake
Lines 387-390 (patched)

Do you know which targets actually require `ENABLE_GRPC` set as a 
pre-processor definition? Since it's new code being added, we should be able to 
resovle this TODO before committing.

src/CMakeLists.txt
Lines 387-388 (patched)

:(

src/examples/CMakeLists.txt
Lines 55-59 (patched)

What about non-Linux but not Windows platforms, e.g. MacOS and FreeBSD?

src/examples/CMakeLists.txt
Lines 93-98 (patched)

Ditto.

src/examples/CMakeLists.txt
Lines 97 (patched)

Nit: since it's already out of alignment due to being in a conditional, 
ther's no need to add the spaces to try to align `PRIVATE mesos grpc`.

src/resource_provider/storage/CMakeLists.txt
Lines 18 (patched)

Nit: I've been aligning the `###`... but I know it's that important.

src/tests/CMakeLists.txt
Line 237 (original), 250-254 (patched)

So `uri_disk_profile_adaptor` is, what, a module? that depends on (and 
links in `mesos`), but then `mesos-tess-interface` (which links in `mesos`) 
also links in `uri_disk_profile_adaptor`. So if things are being linked 
statically, `mesos-tests` would then then have `libmesos` twice? Sorry, I'm 
just trying to sort out the dependency graph in my head. Can you elaborate?

src/tests/CMakeLists.txt
Lines 337-341 (patched)

Maybe add a note that this is a binary and that's why it uses 
`add_dependencies`.

- Andrew Schwartzmeyer

On April 11, 2018, 10:15 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated April 11, 2018, 10:15 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 
>   src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/3/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-04-11 Thread Chun-Hung Hsiao


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

(Updated April 12, 2018, 5:15 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Rebased and addressed Jan's comments.


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


Repository: mesos


Description
---

This patch adds CMake rules for compiling necessary source code for
building storage local resource provider and related tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 
  src/CMakeLists.txt 6fc45dba8ee70b591be03ac483655c1844a0a6b9 
  src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
  src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
  src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 


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

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


Testing
---

`sudo make check` with CMake


Thanks,

Chun-Hung Hsiao

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-22 Thread Jan Schlicht


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




src/examples/CMakeLists.txt
Lines 55-58 (patched)


This won't compile on non-Linux systems (as `test_csi_plugin.cpp` includes 
`linux/fs.hpp`). Hence this has to be guarded by an `if (LINUX)`.



src/examples/CMakeLists.txt
Lines 93-98 (patched)


Needs an additional `if (LINUX)` guard, as explained above.


- Jan Schlicht


On March 21, 2018, 12:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated March 21, 2018, 12:47 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-20 Thread Mesos Reviewbot


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



Bad patch!

Reviews applied: [66163, 61118, 61096, 66162, 66161, 66160, 66159, 66158, 
66157, 66156, 66097, 66096, 66095, 66094]

Failed command: python support/apply-reviews.py -n -r 66094

Error:
2018-03-21 04:52:26 URL:https://reviews.apache.org/r/66094/diff/raw/ 
[9043/9043] -> "66094.patch" [1]
error: missing binary patch data for '3rdparty/grpc-1.10.0.tar.gz'
error: binary patch does not apply to '3rdparty/grpc-1.10.0.tar.gz'
error: 3rdparty/grpc-1.10.0.tar.gz: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/21964/console

- Mesos Reviewbot


On March 21, 2018, 7:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated March 21, 2018, 7:47 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-20 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 66094.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66094`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66163

Relevant logs:

- 
[apply-review-66094-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66163/logs/apply-review-66094-stdout.log):

```
error: missing binary patch data for '3rdparty/grpc-1.10.0.tar.gz'
error: binary patch does not apply to '3rdparty/grpc-1.10.0.tar.gz'
error: 3rdparty/grpc-1.10.0.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On March 20, 2018, 4:47 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated March 20, 2018, 4:47 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-20 Thread Chun-Hung Hsiao


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

(Updated March 20, 2018, 11:47 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds CMake rules for compiling necessary source code for
building storage local resource provider and related tests.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
  src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
  src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
  src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
  src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 


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

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


Testing
---

`sudo make check` with CMake


Thanks,

Chun-Hung Hsiao

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-20 Thread Mesos Reviewbot


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



Bad patch!

Reviews applied: [66163, 61118, 61096, 66162, 66161, 66160, 66159, 66158, 
66157, 66156, 66097, 66096, 66095, 66094]

Failed command: python support/apply-reviews.py -n -r 66094

Error:
2018-03-20 09:04:34 URL:https://reviews.apache.org/r/66094/diff/raw/ 
[9043/9043] -> "66094.patch" [1]
error: missing binary patch data for '3rdparty/grpc-1.10.0.tar.gz'
error: binary patch does not apply to '3rdparty/grpc-1.10.0.tar.gz'
error: 3rdparty/grpc-1.10.0.tar.gz: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/21950/console

- Mesos Reviewbot


On March 20, 2018, 12:59 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated March 20, 2018, 12:59 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

2018-03-19 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 66094.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66094`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66163

Relevant logs:

- 
[apply-review-66094-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66163/logs/apply-review-66094-stdout.log):

```
error: missing binary patch data for '3rdparty/grpc-1.10.0.tar.gz'
error: binary patch does not apply to '3rdparty/grpc-1.10.0.tar.gz'
error: 3rdparty/grpc-1.10.0.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On March 19, 2018, 11:59 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66163/
> ---
> 
> (Updated March 19, 2018, 11:59 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-8698
> https://issues.apache.org/jira/browse/MESOS-8698
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds CMake rules for compiling necessary source code for
> building storage local resource provider and related tests.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
>   src/CMakeLists.txt fb9e9d4cfc4a62830fe3065a139ae14401c0e52e 
>   src/examples/CMakeLists.txt e85eed607715d2b3ee596c3c0b51f16682491900 
>   src/resource_provider/storage/CMakeLists.txt PRE-CREATION 
>   src/tests/CMakeLists.txt ade5180f6e072112ad4836aa436e8545260d2ae6 
> 
> 
> Diff: https://reviews.apache.org/r/66163/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check` with CMake
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

Re: Review Request 66163: Built storage local resource provider with CMake.

17 matches

Site Navigation

Mail list logo

Footer information