Re: Review Request 65347: Added missing protobuf include.

2018-01-30 Thread Till Toenshoff

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


Ship it!




Ship It!

- Till Toenshoff


On Jan. 26, 2018, 11:41 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65347/
> ---
> 
> (Updated Jan. 26, 2018, 11:41 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-8508
> https://issues.apache.org/jira/browse/MESOS-8508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using our bundled version of protobuf, this file is transitively
> included from other headers.
> 
> However, when building against olders versions of protobuf this fails,
> for example when building against the system-default protobuf 3.0
> on Ubuntu 17.04.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ff7239dd6b8f74119fdd97531d94af21fd0500c 
> 
> 
> Diff: https://reviews.apache.org/r/65347/diff/1/
> 
> 
> Testing
> ---
> 
> Built without bundled protobuf on an Ubuntu system.
> 
> Started internal CI run: 
> https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI/2675/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 65347: Added missing protobuf include.

2018-01-26 Thread Joseph Wu

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


Ship it!




Note that the reason why this doesn't compile in protobuf 3.0.x is due to how 
the c++ files are generated.  In protobuf 3.0.x (and 3.1.x and 3.2.x) generated 
code only includes the protobuf map headers if there is a map present in the 
.proto file:
https://github.com/google/protobuf/blob/3.0.x/src/google/protobuf/compiler/cpp/cpp_file.cc#L817-L827

>From 3.3.x onwards, all generated files include 
>`google/protobuf/generated_message_table_driven.h`, which in turn includes the 
>map headers:
https://github.com/google/protobuf/blob/3.3.x/src/google/protobuf/compiler/cpp/cpp_file.cc#L1006

In our case, the `common/protobuf_utils.hpp` includes `mesos/mesos.hpp`, which 
includes a generated protobuf.

- Joseph Wu


On Jan. 26, 2018, 3:41 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65347/
> ---
> 
> (Updated Jan. 26, 2018, 3:41 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using our bundled version of protobuf, this file is transitively
> included from other headers.
> 
> However, when building against olders versions of protobuf this fails,
> for example when building against the system-default protobuf 3.0
> on Ubuntu 17.04.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ff7239dd6b8f74119fdd97531d94af21fd0500c 
> 
> 
> Diff: https://reviews.apache.org/r/65347/diff/1/
> 
> 
> Testing
> ---
> 
> Built without bundled protobuf on an Ubuntu system.
> 
> Started internal CI run: 
> https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI/2675/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 65347: Added missing protobuf include.

2018-01-26 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 65347 was successfully built and tested.

Reviews applied: `['65347']`

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

- Mesos Reviewbot Windows


On Jan. 26, 2018, 11:41 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65347/
> ---
> 
> (Updated Jan. 26, 2018, 11:41 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using our bundled version of protobuf, this file is transitively
> included from other headers.
> 
> However, when building against olders versions of protobuf this fails,
> for example when building against the system-default protobuf 3.0
> on Ubuntu 17.04.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ff7239dd6b8f74119fdd97531d94af21fd0500c 
> 
> 
> Diff: https://reviews.apache.org/r/65347/diff/1/
> 
> 
> Testing
> ---
> 
> Built without bundled protobuf on an Ubuntu system.
> 
> Started internal CI run: 
> https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI/2675/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 65347: Added missing protobuf include.

2018-01-26 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 65347 was successfully built and tested.

Reviews applied: `['65347']`

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

- Mesos Reviewbot Windows


On Jan. 26, 2018, 11:41 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65347/
> ---
> 
> (Updated Jan. 26, 2018, 11:41 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using our bundled version of protobuf, this file is transitively
> included from other headers.
> 
> However, when building against olders versions of protobuf this fails,
> for example when building against the system-default protobuf 3.0
> on Ubuntu 17.04.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ff7239dd6b8f74119fdd97531d94af21fd0500c 
> 
> 
> Diff: https://reviews.apache.org/r/65347/diff/1/
> 
> 
> Testing
> ---
> 
> Built without bundled protobuf on an Ubuntu system.
> 
> Started internal CI run: 
> https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI/2675/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Review Request 65347: Added missing protobuf include.

2018-01-26 Thread Benno Evers

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

Review request for mesos.


Repository: mesos


Description
---

When using our bundled version of protobuf, this file is transitively
included from other headers.

However, when building against olders versions of protobuf this fails,
for example when building against the system-default protobuf 3.0
on Ubuntu 17.04.


Diffs
-

  src/common/protobuf_utils.hpp 0ff7239dd6b8f74119fdd97531d94af21fd0500c 


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


Testing
---

Built without bundled protobuf on an Ubuntu system.

Started internal CI run: 
https://jenkins.mesosphere.com/service/jenkins/job/mesos/job/Mesos_CI/2675/


Thanks,

Benno Evers