Re: Review Request 67025: Fixed the link order for gRPC in CMake.

[Andrew Schwartzmeyer]

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


Ship it!




Examined this with Chun over DM. Brad King (of CMake) has stated at least in 
mailing list threads that the order of the libraries specified here are (or at 
least should be) preserved when linking.

- Andrew Schwartzmeyer


On May 8, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 8, 2018, 7:28 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

[Chun-Hung Hsiao]

> On May 9, 2018, 10:35 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Line 1009 (original), 1009 (patched)
> > 
> >
> > Why are we moving `libgrpc++` before `libgrpc` here in addition to 
> > moving `libgpr` to the back?
> > 
> > I see both the C++ and C libraries depend on `libgpr` so it should 
> > definitely be on the back to allow correct static links; I see no 
> > dependencies between `libgprc++` and `libgprc` though so we might as well 
> > stick to lexicographic order.

`libgrpc` is not gRPC's C wrapper. It's the core library. And despite that the 
current build doesn't generate linkage between `libgrpc` and `libgrpc++`, the 
C++ wrappers do include the core library headers, so it seems safer and more 
future-proof to me to keep this order. Dropping this.


- Chun-Hung


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


On May 9, 2018, 2:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67017, 67018, 66996, 66997, 67024, 67025]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On May 8, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 8, 2018, 7:28 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67025 was successfully built and tested.

Reviews applied: `['67017', '67018', '66996', '66997', '67024', '67025']`

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

- Mesos Reviewbot Windows


On May 9, 2018, 2:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/CMakeLists.txt
Line 1009 (original), 1009 (patched)


Why are we moving `libgrpc++` before `libgrpc` here in addition to moving 
`libgpr` to the back?

I see both the C++ and C libraries depend on `libgpr` so it should 
definitely be on the back to allow correct static links; I see no dependencies 
between `libgprc++` and `libgprc` though so we might as well stick to 
lexicographic order.


- Benjamin Bannier


On May 9, 2018, 4:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 4:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 67025: Fixed the link order for gRPC in CMake.

[Chun-Hung Hsiao]

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

Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.


Repository: mesos


Description
---

Fixed the link order for gRPC in CMake.


Diffs
-

  3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 


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


Testing
---

sudo ninja check


Thanks,

Chun-Hung Hsiao