Create pkg-config files from CMake by zackgalbreath · Pull Request #20211 · grpc/grpc

zackgalbreath · 2019-09-09T19:15:14Z

Fixes #10727

@nanahpang
@jtattermusch

linux-foundation-easycla · 2019-09-09T19:15:17Z

One or more committers are not authorized under a signed CLA as indicated below. Please click here to be authorized.

❌ Zack Galbreath (b6dd719, a4aac4c)

jtattermusch · 2019-09-10T07:36:36Z

@zackgalbreath I started the tests. In the meantime, you'll need to sign the CLA.

templates/CMakeLists.txt.template

jtattermusch · 2019-09-10T17:16:30Z

It would also be nice to add some check for the .pc files being setup correctly to one of our distribtests so that this gets continously tested.

jtattermusch · 2019-09-17T12:52:50Z

@zackgalbreath have you been able to figure out the CLA signing process?

templates/CMakeLists.txt.template

jtattermusch · 2019-09-17T12:57:34Z

@zackgalbreath can you please regenerate the CMakeLists.txt to resolve the merge conflict?

It would be good to expand our distribtests to make sure the generated .pc files are actually installed and that they work as expected.
The scripts being run by our distribtests are here: https://github.com/grpc/grpc/tree/master/test/distrib/cpp

zackgalbreath · 2019-09-23T18:03:18Z

examples/cpp/helloworld/Makefile

           -Wl,--no-as-needed -lgrpc++_reflection -Wl,--as-needed\
+           -laddress_sorting\
+           -lcares\
+           -lz\


As you can see, I had to add some libraries to the helloworld Makefile to get it to build. Please let me know if you would've expected this to build cleanly without these modifications.

I think the reason for the difference it that when built and installed via Makefile, address_sorting, cares and zlib are statically linked into libgrpc (as far as I can tell)

With cmake installation (as per the distribtest), we're actually building each of these libraries with gRPC_*_PROVIDER=package, so they end up installed as individual libraries (and thus need to be linked explicitly).

That actually begs the question that if the deps are installed as separate libraries, perhaps the pkgconfig files should know about these dependencies? (which is kind of the point of pkgconfig).

Discussed with @nicolasnoble offline -
the pkg config files need to carry information about what the dependencies are (gpr, address_sorting, cares, zlib) for them to be useful - that should make the original helloworld Makefile to work fine.

👍 I updated the CMake and Makefile created .pc files to indicate these dependencies. I also updated Requires.private to Requires so pkg-config will find grpc for you if you ask for grpc++.

jtattermusch · 2019-10-04T15:07:26Z

The EasyCLA check is looking good, glad that it finally worked for you!

zackgalbreath · 2019-10-07T16:47:41Z

Are these four failing checks something I should look into?

jtattermusch · 2019-10-09T15:06:30Z

tasks.distribtest.cpp_linux_x64_jessie_routeguide fails with
https://source.cloud.google.com/results/invocations/96da7424-2661-4830-be5a-536ef2842063/targets/github%2Fgrpc/tests

+ make
protoc -I ../../protos --cpp_out=. ../../protos/helloworld.proto
g++ -std=c++11 `pkg-config --cflags protobuf grpc`  -c -o helloworld.pb.o helloworld.pb.cc
protoc -I ../../protos --grpc_out=. --plugin=protoc-gen-grpc=`which grpc_cpp_plugin` ../../protos/helloworld.proto
g++ -std=c++11 `pkg-config --cflags protobuf grpc`  -c -o helloworld.grpc.pb.o helloworld.grpc.pb.cc
g++ -std=c++11 `pkg-config --cflags protobuf grpc`  -c -o greeter_client.o greeter_client.cc
g++ helloworld.pb.o helloworld.grpc.pb.o greeter_client.o -L/usr/local/lib `pkg-config --libs protobuf grpc++` -pthread -Wl,--no-as-needed -lgrpc++_reflection -Wl,--as-needed -ldl -o greeter_client
/usr/bin/ld: cannot find -lcares
/usr/bin/ld: cannot find -lz
collect2: error: ld returned 1 exit status
Makefile:44: recipe for target 'greeter_client' failed

The /usr/bin/ld: cannot find -lcares seems to be related to the Makefile changes in this PR

https://github.com/grpc/grpc/blob/master/test/distrib/cpp/run_distrib_test_routeguide.sh

jtattermusch · 2019-10-09T15:11:59Z

The above error seems to be happening because our Makefile build actually links all the dependent libraries statically (e.g. cares, libz) as part of libgrpc, so e.g. -lcares should not be used for the pkgconfig files generated by the Makefile.

CMakeLists.txt

Makefile

Change "Requires.private" to "Requires" so pkg-config can better provide gRPC's dependencies.

Use pkg-config to automatically find grpc as a dependency of grpc++.

jtattermusch

LGTM, thanks for the contribution!
I'll merge as soon once I doublecheck the test results.

jtattermusch · 2019-10-11T10:42:57Z

Known failures:
#20519
#20472

nanahpang · 2019-10-11T19:30:02Z

This change breaks linux/grpc_distribtests_standalone test. https://fusion.corp.google.com/projectanalysis/summary/KOKORO/prod%3Agrpc%2Fcore%2Fmaster%2Flinux%2Fgrpc_distribtests_standalone

jtattermusch · 2019-10-14T14:50:22Z

@nanahpang I think you're wrong.
According to the build history the distribtests were green for a few more builds after merging this PR but they broke once https://github.com/grpc/grpc/pull/20380/files got merged.

The first failing build is https://source.cloud.google.com/results/invocations/3323a229-9ae7-4c3a-abd3-3ef410cacd9d/targets

This PR got integrated earlier at https://source.cloud.google.com/results/invocations/e821ab54-3f6d-4ac1-859b-5e40a841a63c/targets

nanahpang · 2019-10-14T17:01:38Z

Yes, I have noticed that the failure happened when #20380 got merged. But the changes in #20380 doesn't modify pkg-config. Maybe that PR didn't sync with your changes. I will ask Mark to sync and rebuild that PR first.

nicolasnoble · 2019-10-15T19:41:20Z

LGTM, btw.

jtattermusch · 2019-10-16T14:59:15Z

@zackgalbreath it looks like we might be failing to reference openssl as one of the dependencies in the pkgconfig files. A seemingly unrelated PR is triggering the problem:
#20606 (comment)

https://source.cloud.google.com/results/invocations/a24056d4-44aa-439f-a253-dbf3163ab34d/targets/github%2Fgrpc/tests

zackgalbreath requested review from apolcyn, jtattermusch and nicolasnoble as code owners September 9, 2019 19:15

jtattermusch added the kokoro:run label Sep 10, 2019

grpc-kokoro removed the kokoro:run label Sep 10, 2019

jtattermusch self-assigned this Sep 10, 2019

jtattermusch added lang/c++ release notes: yes Indicates if PR needs to be in release notes labels Sep 10, 2019

jtattermusch reviewed Sep 10, 2019

View reviewed changes

templates/CMakeLists.txt.template Outdated Show resolved Hide resolved

zackgalbreath force-pushed the cmake-pkg-config branch from 543cd21 to c045366 Compare September 10, 2019 20:45

jtattermusch added the kokoro:run label Sep 17, 2019

jtattermusch reviewed Sep 17, 2019

View reviewed changes

templates/CMakeLists.txt.template Show resolved Hide resolved

zackgalbreath force-pushed the cmake-pkg-config branch from c045366 to b6dd719 Compare September 23, 2019 16:45

grpc-kokoro removed the kokoro:run label Sep 23, 2019

zackgalbreath force-pushed the cmake-pkg-config branch from c5f5d40 to a4aac4c Compare September 23, 2019 17:59

zackgalbreath commented Sep 23, 2019

View reviewed changes

jtattermusch added the kokoro:run label Oct 1, 2019

grpc-kokoro removed the kokoro:run label Oct 1, 2019

zackgalbreath force-pushed the cmake-pkg-config branch from a4aac4c to 60b9f7b Compare October 3, 2019 15:08

jtattermusch added the kokoro:run label Oct 4, 2019

grpc-kokoro removed the kokoro:run label Oct 4, 2019

jtattermusch reviewed Oct 9, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

jtattermusch reviewed Oct 9, 2019

View reviewed changes

Makefile Outdated Show resolved Hide resolved

zackgalbreath added 4 commits October 9, 2019 11:39

Create pkg-config files from CMake

7bb7506

Update Makefile rules for .pc file creation

d249dc9

Change "Requires.private" to "Requires" so pkg-config can better provide gRPC's dependencies.

Simplify helloworld makefile

17f95ab

Use pkg-config to automatically find grpc as a dependency of grpc++.

Add distrib test for CMake pkg-config

d6f4891

zackgalbreath force-pushed the cmake-pkg-config branch from 60b9f7b to d6f4891 Compare October 9, 2019 16:58

jtattermusch added the kokoro:run label Oct 10, 2019

grpc-kokoro removed the kokoro:run label Oct 10, 2019

add cmake_pkgconfig test to distribtest suite

1303e80

jtattermusch added the kokoro:run label Oct 11, 2019

grpc-kokoro removed the kokoro:run label Oct 11, 2019

jtattermusch approved these changes Oct 11, 2019

View reviewed changes

jtattermusch merged commit 3990d9c into grpc:master Oct 11, 2019

jtattermusch mentioned this pull request Oct 14, 2019

Revert "xds client bootstrap file" #20593

Merged

KyleFromKitware mentioned this pull request Oct 28, 2019

cmake: pkgconfig files not installed #10727

Closed

figroc mentioned this pull request Nov 9, 2019

grpc: add version 1.24.3 spack/spack#13658

Merged

zackgalbreath deleted the cmake-pkg-config branch December 12, 2019 18:53

lock bot locked as resolved and limited conversation to collaborators Mar 11, 2020

Conversation

zackgalbreath commented Sep 9, 2019

Uh oh!

linux-foundation-easycla bot commented Sep 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtattermusch commented Sep 10, 2019

Uh oh!

Uh oh!

jtattermusch commented Sep 10, 2019

Uh oh!

jtattermusch commented Sep 17, 2019

Uh oh!

Uh oh!

jtattermusch commented Sep 17, 2019

Uh oh!

zackgalbreath Sep 23, 2019

Choose a reason for hiding this comment

Uh oh!

jtattermusch Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtattermusch Oct 1, 2019

Choose a reason for hiding this comment

Uh oh!

jtattermusch Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

zackgalbreath Oct 3, 2019

Choose a reason for hiding this comment

Uh oh!

jtattermusch commented Oct 4, 2019

Uh oh!

zackgalbreath commented Oct 7, 2019

Uh oh!

jtattermusch commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jtattermusch commented Oct 9, 2019

Uh oh!

Uh oh!

Uh oh!

jtattermusch left a comment

Choose a reason for hiding this comment

Uh oh!

jtattermusch commented Oct 11, 2019

Uh oh!

nanahpang commented Oct 11, 2019

Uh oh!

jtattermusch commented Oct 14, 2019

Uh oh!

nanahpang commented Oct 14, 2019

Uh oh!

nicolasnoble commented Oct 15, 2019

Uh oh!

jtattermusch commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

linux-foundation-easycla bot commented Sep 9, 2019 •

edited

Loading

jtattermusch Oct 1, 2019 •

edited

Loading

jtattermusch commented Oct 9, 2019 •

edited

Loading