You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2019/03/26 05:13:05 UTC

Review Request 70302: Adjusted CSI v0 proto compilation.

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

Review request for mesos, Benjamin Bannier and Joseph Wu.


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


Repository: mesos


Description
-------

This patch makes the following adjustments so we can build CSI v0 and v1
in the future:

* Standardize placements for third-party proto files: `csi.proto` from
  CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
  `build/include/csi/v1` in the future. Dependent proto files should
  import `csi/v0/csi.proto` or `csi/v1/csi.proto`.

* Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
  future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
  Since we don't insteall CSI headers, for the installed `v0.hpp` and
  `v1.hpp` headers to work, users must ensure that the CSI headers can
  be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.


Diffs
-----

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
  3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
  3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
  include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
  src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
  src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
  src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
  src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
  src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
  src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
  src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
  src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
  src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
  src/tests/csi_utils_tests.cpp PRE-CREATION 
  src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
  src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 


Diff: https://reviews.apache.org/r/70302/diff/1/


Testing
-------

make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/#review214079
-----------------------------------------------------------



FAIL: Failed to apply the dependent review: 70216.

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

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3023/mesos-review-70302

Relevant logs:

- [apply-review-70216.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3023/mesos-review-70302/logs/apply-review-70216.log):

```
error: patch failed: src/resource_provider/storage/provider.cpp:2130
error: src/resource_provider/storage/provider.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On March 25, 2019, 10:13 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 25, 2019, 10:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On March 26, 2019, 3:44 p.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Line 1 (original), 1 (patched)
> > <https://reviews.apache.org/r/70302/diff/1/?file=2134142#file2134142line1>
> >
> >     I would suggest we move this patch in between https://reviews.apache.org/r/70248/ and https://reviews.apache.org/r/70247/.

This cannot be moved before r/72048 because it relies on that some `csi.proto` dependencies have been removed from r/70248. Dropping.


> On March 26, 2019, 3:44 p.m., Benjamin Bannier wrote:
> > include/csi/spec.hpp
> > Line 1 (original), 1 (patched)
> > <https://reviews.apache.org/r/70302/diff/1/?file=2134146#file2134146line1>
> >
> >     Let's explicitly include this file in `src/csi/volume_manager.hpp`.

Reordered the patches so no need to do the header inclusion. Let's keep `volume_manager.hpp` CSI-version agnostic.


- Chun-Hung


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


On March 26, 2019, 5:13 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 5:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

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


Fix it, then Ship it!




Looks good module two issues.


3rdparty/CMakeLists.txt
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/70302/#comment300233>

    I would suggest we move this patch in between https://reviews.apache.org/r/70248/ and https://reviews.apache.org/r/70247/.



include/csi/spec.hpp
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/70302/#comment300231>

    Let's explicitly include this file in `src/csi/volume_manager.hpp`.


- Benjamin Bannier


On March 26, 2019, 6:13 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 6:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/#review214077
-----------------------------------------------------------



Bad review!

Reviews applied: [70302, 70248, 70258, 70247, 70225, 70223, 70285, 70222, 70284, 70217, 70215, 70216, 70214, 70213, 70168, 70245, 70316, 70315, 70314, 70313]

Error:
2019-03-27 04:43:50 URL:https://reviews.apache.org/r/70168/diff/raw/ [65403/65403] -> "70168.patch" [1]
error: patch failed: src/resource_provider/storage/provider.cpp:2704
error: src/resource_provider/storage/provider.cpp: patch does not apply

- Mesos Reviewbot


On March 26, 2019, 5:13 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 5:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On April 2, 2019, 1:50 p.m., Benjamin Bannier wrote:
> > 3rdparty/csi.md
> > Lines 10-12 (patched)
> > <https://reviews.apache.org/r/70302/diff/3/?file=2136660#file2136660line10>
> >
> >     Thinking about this, let's not rebundle, but instead us the upstream tarball.
> >     
> >     ```
> >     curl -L https://github.com/container-storage-interface/spec/archive/v<X.Y.Z>.tar.gz -o csi-<X.Y.Z>.tar.gz
> >     ```
> >     
> >     We can then adjust the directory layout as part of our build.
> >     
> >     cmake: use e.g., `CONFIGURE_COMMAND mkdir -p ${CSI_V0_ROOT}/csi/v0 && mv ${CSI_V0_ROOT}/csi.proto ${CSI_V0_ROOT}/csi/v0` and `CONFIGURE_COMMAND mkdir -p ${CSI_V1_ROOT}/csi/v1 && mv ${CSI_V1_ROOT}/csi.proto ${CSI_V1_ROOT}/csi/v1`
> >     
> >     autotools:
> >     ```
> >     -ALL_LOCAL += $(CSI_V0)-stamp
> >     +$(CSI_V0)-build-stamp: $(CSI_V0)-stamp
> >     +       $(MKDIR_P) $(CSI_V0)/csi/v0 && mv spec-$(CSI_V0_VERSION)/csi.proto $(CSI_V0)/csi/v0
> >     +       touch $@
> >     +
> >     +ALL_LOCAL += $(CSI_V0)-build-stamp
> >     ```
> >     We also need to add `spec-$(CSI_V0_VERSION)` to `CLEAN_EXTRACTED` there.
> >     
> >     The same should be done for v1 later in the chain.

For autotools, I'm thinking about doing the following instead:
```
# NOTE: The CSI release tarball would be extracted to directory `spec-<X.Y.Z>`
# instead of `csi-<X.Y.Z>`. Therefore we make directory `csi-<X.Y.Z>` and
# extract the tarball there.
csi-%-stamp: csi-%.tar.gz
        $(MKDIR_P) csi-$*
        gzip -d -c $^ | (cd csi-$* && tar xf -)
        test ! -e $(srcdir)/csi-$*.patch || \
          patch -d csi-$*/spec-$* -p1 <$(srcdir)/csi-$*.patch
        touch $@
```
So we don't need to put `spec-$(CSI_V0_VERSION)` elsewhere.

Then, we can add the `build-stamp` rule as you suggested.


- Chun-Hung


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


On April 2, 2019, 6:11 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated April 2, 2019, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 should be in `csi/v0/` of the library root directory and that
>   from v1 should be in `csi/v1/` in the future. Dependent proto files
>   should import `csi/v0/csi.proto` or `csi/v1/csi.proto`. The procedure
>   to bundle CSI is described in `3rdparty/csi.md`.
> 
> * The generated files for CSI v0 is placed in `build/include/csi/v0`,
>   and those for CSI v1 will be in `build/include/csi/v1` in the future.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't install CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
>   3rdparty/csi.md PRE-CREATION 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On April 2, 2019, 1:50 p.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 705-706 (patched)
> > <https://reviews.apache.org/r/70302/diff/3/?file=2136664#file2136664line716>
> >
> >     This does not what you think it does, in particular the headers will not be installed in `csi/v0`, but in `$csidir` instead.
> >     
> >     You need to declare these headers separately, e.g.,
> >     ```
> >     +csi_v0dir = $(pkgincludedir)/csi/v0
> >     +nodist_csi_v0_HEADERS =                                                        \
> >     +  ../include/csi/v0/csi.grpc.pb.h                                      \
> >     +  ../include/csi/v0/csi.pb.h
> >     +
> >     ```
> >     
> >     Similarly later in the chain for the v1 headers.

Then would it be better if we set `csi_v0dir` to `$(includedir)/csi/v0` instead of `$(pkgincludedir/csi/v0)`? And we can also install `csi.proto` in the same place in `3rdparty/Makefile.am`.


- Chun-Hung


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


On April 2, 2019, 6:11 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated April 2, 2019, 6:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 should be in `csi/v0/` of the library root directory and that
>   from v1 should be in `csi/v1/` in the future. Dependent proto files
>   should import `csi/v0/csi.proto` or `csi/v1/csi.proto`. The procedure
>   to bundle CSI is described in `3rdparty/csi.md`.
> 
> * The generated files for CSI v0 is placed in `build/include/csi/v0`,
>   and those for CSI v1 will be in `build/include/csi/v1` in the future.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't install CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
>   3rdparty/csi.md PRE-CREATION 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

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




3rdparty/csi.md
Lines 10-12 (patched)
<https://reviews.apache.org/r/70302/#comment300490>

    Thinking about this, let's not rebundle, but instead us the upstream tarball.
    
    ```
    curl -L https://github.com/container-storage-interface/spec/archive/v<X.Y.Z>.tar.gz -o csi-<X.Y.Z>.tar.gz
    ```
    
    We can then adjust the directory layout as part of our build.
    
    cmake: use e.g., `CONFIGURE_COMMAND mkdir -p ${CSI_V0_ROOT}/csi/v0 && mv ${CSI_V0_ROOT}/csi.proto ${CSI_V0_ROOT}/csi/v0` and `CONFIGURE_COMMAND mkdir -p ${CSI_V1_ROOT}/csi/v1 && mv ${CSI_V1_ROOT}/csi.proto ${CSI_V1_ROOT}/csi/v1`
    
    autotools:
    ```
    -ALL_LOCAL += $(CSI_V0)-stamp
    +$(CSI_V0)-build-stamp: $(CSI_V0)-stamp
    +       $(MKDIR_P) $(CSI_V0)/csi/v0 && mv spec-$(CSI_V0_VERSION)/csi.proto $(CSI_V0)/csi/v0
    +       touch $@
    +
    +ALL_LOCAL += $(CSI_V0)-build-stamp
    ```
    We also need to add `spec-$(CSI_V0_VERSION)` to `CLEAN_EXTRACTED` there.
    
    The same should be done for v1 later in the chain.



src/Makefile.am
Lines 705-706 (patched)
<https://reviews.apache.org/r/70302/#comment300491>

    This does not what you think it does, in particular the headers will not be installed in `csi/v0`, but in `$csidir` instead.
    
    You need to declare these headers separately, e.g.,
    ```
    +csi_v0dir = $(pkgincludedir)/csi/v0
    +nodist_csi_v0_HEADERS =                                                        \
    +  ../include/csi/v0/csi.grpc.pb.h                                      \
    +  ../include/csi/v0/csi.pb.h
    +
    ```
    
    Similarly later in the chain for the v1 headers.


- Benjamin Bannier


On April 2, 2019, 8:11 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated April 2, 2019, 8:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 should be in `csi/v0/` of the library root directory and that
>   from v1 should be in `csi/v1/` in the future. Dependent proto files
>   should import `csi/v0/csi.proto` or `csi/v1/csi.proto`. The procedure
>   to bundle CSI is described in `3rdparty/csi.md`.
> 
> * The generated files for CSI v0 is placed in `build/include/csi/v0`,
>   and those for CSI v1 will be in `build/include/csi/v1` in the future.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't install CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
>   3rdparty/csi.md PRE-CREATION 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

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


Ship it!




Ship It!

- Benjamin Bannier


On April 3, 2019, 11:12 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated April 3, 2019, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 should be in `csi/v0/` of the library root directory and that
>   from v1 should be in `csi/v1/` in the future. Dependent proto files
>   should import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * The generated files for CSI v0 is placed in `build/include/csi/v0`,
>   and those for CSI v1 will be in `build/include/csi/v1` in the future.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
> 
> * Install the CSI v0 proto file and its generated headers to
>   `$PREFIX/include/csi/v0` in autotools. CSI v1 files will be installed
>   at `$PREFIX/include/csi/v1` in the future.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt f1c3114f809ed775e09252ec1f811f8504cbde17 
>   src/Makefile.am ea8e1760ffe7fd30d47176a37a5eabf6618bb365 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/
-----------------------------------------------------------

(Updated April 3, 2019, 9:12 p.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
-------

Fixed a build issue with GNU make 3.81 (which is used on MacOS, Ubuntu 14.04 and CentOS 6).


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


Repository: mesos


Description
-------

This patch makes the following adjustments so we can build CSI v0 and v1
in the future:

* Standardize placements for third-party proto files: `csi.proto` from
  CSI v0 should be in `csi/v0/` of the library root directory and that
  from v1 should be in `csi/v1/` in the future. Dependent proto files
  should import `csi/v0/csi.proto` or `csi/v1/csi.proto`.

* The generated files for CSI v0 is placed in `build/include/csi/v0`,
  and those for CSI v1 will be in `build/include/csi/v1` in the future.

* Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
  future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.

* Install the CSI v0 proto file and its generated headers to
  `$PREFIX/include/csi/v0` in autotools. CSI v1 files will be installed
  at `$PREFIX/include/csi/v1` in the future.


Diffs (updated)
-----

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
  3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
  3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
  3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
  include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
  src/CMakeLists.txt f1c3114f809ed775e09252ec1f811f8504cbde17 
  src/Makefile.am ea8e1760ffe7fd30d47176a37a5eabf6618bb365 
  src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
  src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
  src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
  src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
  src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
  src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
  src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
  src/tests/csi_utils_tests.cpp PRE-CREATION 
  src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
  src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 


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

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


Testing
-------

make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/
-----------------------------------------------------------

(Updated April 3, 2019, 12:48 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
-------

Addressed Benjamin's comments.


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


Repository: mesos


Description (updated)
-------

This patch makes the following adjustments so we can build CSI v0 and v1
in the future:

* Standardize placements for third-party proto files: `csi.proto` from
  CSI v0 should be in `csi/v0/` of the library root directory and that
  from v1 should be in `csi/v1/` in the future. Dependent proto files
  should import `csi/v0/csi.proto` or `csi/v1/csi.proto`.

* The generated files for CSI v0 is placed in `build/include/csi/v0`,
  and those for CSI v1 will be in `build/include/csi/v1` in the future.

* Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
  future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.

* Install the CSI v0 proto file and its generated headers to
  `$PREFIX/include/csi/v0` in autotools. CSI v1 files will be installed
  at `$PREFIX/include/csi/v1` in the future.


Diffs (updated)
-----

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
  3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
  3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
  3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
  include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
  src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
  src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3 
  src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
  src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
  src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
  src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
  src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
  src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
  src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
  src/tests/csi_utils_tests.cpp PRE-CREATION 
  src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
  src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 


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

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


Testing
-------

make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 70302: Adjusted CSI v0 bundling and proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/
-----------------------------------------------------------

(Updated April 2, 2019, 6:11 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
-------

Addressed Benjamin's comments.


Summary (updated)
-----------------

Adjusted CSI v0 bundling and proto compilation.


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


Repository: mesos


Description (updated)
-------

This patch makes the following adjustments so we can build CSI v0 and v1
in the future:

* Standardize placements for third-party proto files: `csi.proto` from
  CSI v0 should be in `csi/v0/` of the library root directory and that
  from v1 should be in `csi/v1/` in the future. Dependent proto files
  should import `csi/v0/csi.proto` or `csi/v1/csi.proto`. The procedure
  to bundle CSI is described in `3rdparty/csi.md`.

* The generated files for CSI v0 is placed in `build/include/csi/v0`,
  and those for CSI v1 will be in `build/include/csi/v1` in the future.

* Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
  future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
  Since we don't install CSI headers, for the installed `v0.hpp` and
  `v1.hpp` headers to work, users must ensure that the CSI headers can
  be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.


Diffs (updated)
-----

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
  3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
  3rdparty/csi-0.2.0.tar.gz eea1cbd47eaf955ca94c579135e95cb4d750356a 
  3rdparty/csi.md PRE-CREATION 
  3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
  include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
  src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
  src/Makefile.am 7c2131a9daf16d49e8e8d75b12f019e5c4df1da3 
  src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
  src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
  src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
  src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
  src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
  src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
  src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
  src/tests/csi_utils_tests.cpp PRE-CREATION 
  src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
  src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 


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

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


Testing
-------

make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.

> On March 28, 2019, 10:06 a.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 489-492 (patched)
> > <https://reviews.apache.org/r/70302/diff/2/?file=2134667#file2134667line495>
> >
> >     We don't typically directly copy 3rdparty sources into our build tree; let's not start doing it here.
> >     
> >     Let's instead build our artifacts by directly referencing the unpacked artifacts. We can adjust the macro to do just that. Above approach also fails in `make distcheck` as it places files in the build tree which are not removed by any `clean` rule.

It depends if we want to allow our own proto to refer to these protos in the future. If we want to allow that, the problem is that it won't work with protoc.

Consider that we have `src/csi/v0_state.proto` and `src/csi/v1_state.proto`, both import `csi.proto` of respective version, and the directory layout is as follows:
```
build/3rdparty/csi_v0-0.2.0/csi.proto
build/3rdparty/csi_v1-1.1.0/csi.proto
```
Apprantly we cannot do `protoc -Ibuild/3rdparty/csi_v0-0.2.0/ -Ibuild/3rdparty/csi_v1-1.1.0/`. Instead, we might have to write specific rules to compile these two protos, like:
```
csi/v0_%.pb.h csi/v0_%.pb.cc: csi/v0_%.proto
        $(PROTOC) $(PROTOCFLAGS) -I$(CSI_V0) --cpp_out=. $^
        
csi/v1_%.pb.h csi/v1_%.pb.cc: csi/v1_%.proto
        $(PROTOC) $(PROTOCFLAGS) -I$(CSI_V1) --cpp_out=. $^
```
We'll need to do a similar workaround in our cmake rules.

Now, workaround protoc is actually a small problem. The bigger problem is that, by doing the above, the generated code would both include `csi.pb.h`.
Again, we cannot do `g++ -Ibuild/include/csi/v0 -Ibuild/include/csi/v1`, so we have to also write specific rules for compiling the generated code.

The alternative I proposed here, is that we first place `csi.proto`s with the following directory layout:
```
build/include/csi/v0/csi.proto
build/include/csi/v1/csi.proto
```
Then, in `src/csi/v0_state.proto` we do `import "csi/v0/csi.proto";`, and in `src/csi/v1_state.proto` we do `import "csi/v1/csi.proto`.
As a result, `csi/v0/csi.pb.h` will be included in the generated files of the former, and `csi/v1/csi.pb.h` will be included in that of the latter.

That said, since we now have introduced unversioned protos, we currently don't have any proto depending on the CSI proto of a specific version. But, we'll need a resolution unless we are sure that there will never be such a need.

WDYT?


- Chun-Hung


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


On March 27, 2019, 3:27 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 3:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am bcafe48b2105575371464a29783bc6f3f1c2cf8d 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

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




src/CMakeLists.txt
Line 123 (original), 118 (patched)
<https://reviews.apache.org/r/70302/#comment300354>

    If we use this here let's also init the variable above; else remove the usage here.



src/CMakeLists.txt
Line 126 (original), 121 (patched)
<https://reviews.apache.org/r/70302/#comment300355>

    Ditto.



src/Makefile.am
Lines 489-492 (patched)
<https://reviews.apache.org/r/70302/#comment300357>

    We don't typically directly copy 3rdparty sources into our build tree; let's not start doing it here.
    
    Let's instead build our artifacts by directly referencing the unpacked artifacts. We can adjust the macro to do just that. Above approach also fails in `make distcheck` as it places files in the build tree which are not removed by any `clean` rule.


- Benjamin Bannier


On March 27, 2019, 4:27 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 27, 2019, 4:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am bcafe48b2105575371464a29783bc6f3f1c2cf8d 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/
-----------------------------------------------------------

(Updated March 27, 2019, 3:27 p.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

This patch makes the following adjustments so we can build CSI v0 and v1
in the future:

* Standardize placements for third-party proto files: `csi.proto` from
  CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
  `build/include/csi/v1` in the future. Dependent proto files should
  import `csi/v0/csi.proto` or `csi/v1/csi.proto`.

* Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
  future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
  Since we don't insteall CSI headers, for the installed `v0.hpp` and
  `v1.hpp` headers to work, users must ensure that the CSI headers can
  be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.


Diffs (updated)
-----

  3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
  3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
  3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
  3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
  include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
  src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
  src/Makefile.am bcafe48b2105575371464a29783bc6f3f1c2cf8d 
  src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
  src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
  src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
  src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
  src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
  src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
  src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
  src/tests/csi_utils_tests.cpp PRE-CREATION 
  src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
  src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 


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

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


Testing
-------

make check


Thanks,

Chun-Hung Hsiao


Re: Review Request 70302: Adjusted CSI v0 proto compilation.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70302/#review214031
-----------------------------------------------------------



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['70245', '70168', '70213', '70214', '70216', '70215', '70217', '70284', '70222', '70285', '70223', '70225', '70247', '70258', '70248', '70302']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3016/mesos-review-70302

Relevant logs:

- [mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3016/mesos-review-70302/logs/mesos-tests-cmake.log):

```
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501): warning C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479): warning C4101: 'addrstr': unreferenced local variable [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170): warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496): warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256): warning C4090: 'function': different 'const' qualifiers [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166): warning C4716: 'pthread_cond_broadcast': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205): warning C4716: 'pthread_cond_wait': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
         d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]


       "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
       "D:\DCOS\mesos\src\mesos-protobufs.vcxproj" (default target) (16) ->
       "D:\DCOS\mesos\src\copy_bin_include_csi_v0_proto.vcxproj" (default target) (23) ->
       (CustomBuild target) -> 
         C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(209,5): error MSB6006: "cmd.exe" exited with code 1. [D:\DCOS\mesos\src\copy_bin_include_csi_v0_proto.vcxproj]

    172 Warning(s)
    1 Error(s)

Time Elapsed 00:00:53.63
```

- Mesos Reviewbot Windows


On March 26, 2019, 5:13 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70302/
> -----------------------------------------------------------
> 
> (Updated March 26, 2019, 5:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9624
>     https://issues.apache.org/jira/browse/MESOS-9624
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the following adjustments so we can build CSI v0 and v1
> in the future:
> 
> * Standardize placements for third-party proto files: `csi.proto` from
>   CSI v0 will be in `build/include/csi/v0/` and that from v0 will be in
>   `build/include/csi/v1` in the future. Dependent proto files should
>   import `csi/v0/csi.proto` or `csi/v1/csi.proto`.
> 
> * Moved `include/csi/spec.hpp` to `include/mesos/csi/v0.hpp`. In the
>   future, CSI v1 proto headers will be in `include/mesos/csi/v1.hpp`.
>   Since we don't insteall CSI headers, for the installed `v0.hpp` and
>   `v1.hpp` headers to work, users must ensure that the CSI headers can
>   be found through `csi/v0/csi*.pb.h` and `csi/v1/csi*.pb.h`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 1999dd20964da96bc5acfbd47cb80d4ca6f734b9 
>   3rdparty/Makefile.am 98a2623b34132885279bea6135c0da4ffc8c59cf 
>   3rdparty/cmake/Versions.cmake 972c706a18bfce45af6c2632b326606052888b02 
>   3rdparty/versions.am 24381073f4c77d92c5ba62f54bb2194bf041ee6c 
>   include/csi/spec.hpp 19d9445fe1da7be6e41b484b5a78dcd10e5ece52 
>   src/CMakeLists.txt 3397c3b1d4e8a7900b2e5f870679cc7aa30b4be2 
>   src/Makefile.am d451d7cabe3bf5d4f5039cfac5de1b03ef891d07 
>   src/cmake/MesosProtobuf.cmake 09074d77a1df5076c6e51292523ce994a77a88f2 
>   src/csi/client.hpp c2583cf4c0d2abc38d65ddc801cae3696a8080a9 
>   src/csi/rpc.hpp b2502ceb319638038b4d151965f6226db675f96b 
>   src/csi/utils.hpp 9145c6795c3ecdde5de5859a852763fe9aeb1ddf 
>   src/examples/test_csi_plugin.cpp 73a6c43e72afec0dd124b0fe2f8ef0e45acb307f 
>   src/resource_provider/storage/disk_profile.proto 1c97e9c62fb3154d6048d0e55352a1276adcbca8 
>   src/resource_provider/storage/disk_profile_utils.hpp 8a83a15ba555ce66bbb86b8df72178bce17a615a 
>   src/tests/csi_utils_tests.cpp PRE-CREATION 
>   src/tests/disk_profile_adaptor_tests.cpp 0ccbc79d7ffb82a68b7ed5aeab930bcd8e6e770e 
>   src/tests/mock_csi_plugin.hpp 6897fbc878f1e2f5b9e9c402b09358c187af79a0 
> 
> 
> Diff: https://reviews.apache.org/r/70302/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>