mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@mesosphere.io>
Subject Re: Review Request 65997: Added `PROTOC_SPEC_GENERATE` helper for cmake.
Date Wed, 14 Mar 2018 02:20:44 GMT


> On March 13, 2018, 6:54 p.m., Andrew Schwartzmeyer wrote:
> > src/cmake/MesosProtobuf.cmake
> > Lines 174-176 (patched)
> > <https://reviews.apache.org/r/65997/diff/2/?file=1973227#file1973227line174>
> >
> >     This is probably something for later, but these essentially hard-coded directories
have never sat well with me.

Yeah this is kinda unfortunate. It would be better if the CSI package itself comes with its
own makefile to build the proto files, and I might work on a PR to the CSI repo for this in
the future. As a workaround for now, I think this hard-coded directory structure is good enough
for us to handle similar cases we may have in the future.


- Chun-Hung


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


On March 9, 2018, 10:44 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65997/
> -----------------------------------------------------------
> 
> (Updated March 9, 2018, 10:44 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and Joseph
Wu.
> 
> 
> Bugs: MESOS-8657
>     https://issues.apache.org/jira/browse/MESOS-8657
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> PROTOC_SPEC_GENERATE is a convenience function that will:
>   (1) Compile .proto files found in the third-party specification
>       library under its include directory and place the generated files
>       in the build folder, under the `include/` directory, or with the
>       option `INTERNAL` the `src/` directory.
>   (2) Append to list variables `PUBLIC_PROTO_PATH` or
>       `INTERNAL_PROTO_PATH` the fully qualified path to the library's
>       include directory depending on options passed in.
>   (3) Append to list variables `PUBLIC_PROTOBUF_INCLUDE_DIR` or
>       `INTERNAL_PROTOBUF_INCLUDE_DIR` (depending on options passed in)
>       the fully qualified path to the directory where the files are
>       generated.
>   (4) Append to list variables `PUBLIC_PROTOBUF_SRC` or
>       `INTERNAL_PROTOBUF_SRC` (depending on options passed in) the fully
>       qualified path to the generated .cc file.
> Where the exports in (2)(3)(4) are *side effects* and modifies the
> variables in the parent scope.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 0c135034982a6a4844509cd0e9b2cc41804333ef 
>   src/cmake/MesosProtobuf.cmake ef90c15a6f2afaedaf7559c4a411098924528505 
> 
> 
> Diff: https://reviews.apache.org/r/65997/diff/2/
> 
> 
> Testing
> -------
> 
> `make check` with cmake.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message