Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add header search paths to protobuf-c++ spec #10024

Merged

Conversation

Niranjan-B
Copy link
Contributor

The Problem:
Consuming Protobuf runtime in Swift via "Protobuf-C++ podspec" fails compilation as it isn't able to resolve the header imports.

Solution:
Adding "HEADER_SEARCH_PATHS" to the "pod_target_xcconfig" flag will help "Protobuf-C++" pod discover the required headers.

NOTE:- Current obj-c builds are linting "Protobuf.podspec" and not "Protobuf-C++.podspec", hence they are all green.

@google-cla
Copy link

google-cla bot commented May 23, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@shaod2 shaod2 requested a review from thomasvl May 24, 2022 16:59
@thomasvl thomasvl removed their request for review May 24, 2022 17:00
@thomasvl
Copy link
Contributor

Core protobuf-team manages this podspec, so I'm not sure if this would be a good addition or not.

@deannagarcia
Copy link
Member

Thanks for the contribution! Can you explain why this file needs the header argument but Protobuf.podspec doesn't?

@thomasvl thomasvl added c++ and removed objective-c labels May 24, 2022
@Niranjan-B
Copy link
Contributor Author

Thanks for reviewing @deannagarcia.

I think it is to do with the way headers are imported in obj-c files (obj-c runtime is shipped using Protobuf.podspec) vs c++ files (c++ runtime is shipped using Protobuf-C++.podspec).

Protobuf.podspec -
I think we don't need header search paths since the imports do not use angled brackets. This tells the compiler to look for headers in the same directory first.
Eg. of an import - #import "GPBAny.pbobjc.h" in GPBAny.pbobjc.m

Protobuf-C++.podspec -
Since the imports in C++ files includes angled brackets (Eg. #include <google/protobuf/message_lite.h>), clang searches for those headers in the system directory or include path list. Since we want the compiler to use headers that are shipped with the framework, we would have to specify the directory in the include path list. This is achieved using the HEADER_SEARCH_PATHS flag.

@deannagarcia
Copy link
Member

Thanks for the explanation! Once the tests pass, I will get this merged.

@deannagarcia deannagarcia merged commit 6641a42 into protocolbuffers:main May 25, 2022
@Niranjan-B
Copy link
Contributor Author

Hi @deannagarcia, do you have an ETA on when this commit would be picked up for the next release, please?

@Niranjan-B
Copy link
Contributor Author

Hi @deannagarcia, sorry for tagging you again. I've been waiting quite some time for this fix. Could this be included as part of v21.2 Please?

cc: @thomasvl, @esorot

@fowles fowles requested a review from esorot June 25, 2022 20:49
@esorot
Copy link
Contributor

esorot commented Jun 27, 2022

Apologies, but this was missed in the v21.2 build. This will have to wait for our next release.

@Niranjan-B
Copy link
Contributor Author

Hi @esorot, When would the next release be out? Or could I cherry-pick this commit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants