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

Protobuf CocoaPod depends upon umbrella header ordering #4403

Closed
paulb777 opened this issue Mar 21, 2018 · 5 comments
Closed

Protobuf CocoaPod depends upon umbrella header ordering #4403

paulb777 opened this issue Mar 21, 2018 · 5 comments
Assignees

Comments

@paulb777
Copy link
Contributor

paulb777 commented Mar 21, 2018

CocoaPods will fail to build the Protobuf CocoaPod if the explicitly pathed headers are ordered before the wild carded headers in the generated umbrella header file.

This is exposed in the unreleased CocoaPods master branch. CocoaPods may change back to the old behavior, but it would probably be good to make the Protobuf CocoaPod less fragile.

Details and example at CocoaPods/CocoaPods#7518.

@paulb777
Copy link
Contributor Author

Part of the problem seems to be a circular dependency chain between Any.pbobjc.h and GPBWellKnownTypes.h. Other strange things are GPBProtocolBuffers.h being kind of a duplicate umbrella header and header sequencing to test the protoc versioning.

@thomasvl
Copy link
Contributor

This sounds a little like #4301, is it the same thing?

From a quick look at the linked issue, that seems to say it is an ordering problem, but it isn't clear that protos has and ordering problem vs. something causing the imports to be found via different paths, so the #import doesn't collapse into a single definition?

@paulb777
Copy link
Contributor Author

It's similar to #4301, triggered by a different cause, with likely the same solution.

In this case, because the pre-release version of CocoaPods generated the umbrella header with a different order, the Protobuf module failed to be created during the build because of the order requirements.

The CocoaPods team decided that even though relying upon header order is not technically correct, that it wasn't an acceptable change for a minor CocoaPods release update and decided to maintain the requested order with CocoaPods/CocoaPods#7539 that got in before the first 1.5.0 beta went out.

So there is not a near term CocoaPods risk.

To reproduce :

  • install from CocoaPods normally
  • open Pods/Target Support Files/Protobuf/Protobuf-umbrella.h
  • move the "Any.pbobjc.h" through "Wrappers.pbobjc.h" in front of the other headers
  • build

@thomasvl
Copy link
Contributor

Other strange things are GPBProtocolBuffers.h being kind of a duplicate umbrella header

I'm not sure how to resolve this one. Cocoapods isn't the only packaging system, we need to provide an umbrella for others (Carthage, direct source usage, etc).

header sequencing to test the protoc versioning

Not sure I follow this one, this should just mean the defines have to exist before we import other headers, no?

@paulb777
Copy link
Contributor Author

If you create your own module_map with your own umbrella header - https://guides.cocoapods.org/syntax/podspec.html#module_map, then CocoaPods shouldn't generate one.

thomasvl added a commit to thomasvl/protobuf that referenced this issue Mar 30, 2018
Cut down on what is imported into the generated .pbobjc.h files to only
what is really needed. This should help compile times a little by not
making clang go fetch headers that aren't really needed.

BREAKING CHANGE: For anyone that only ever included .pbobjc.h files, but
then expected to be able to access some less common things like UnknownFields,
Descriptor based field access, etc. The code using those features needs to
include the specific GPB headers needed or just include GPBProtocolBuffers.h.

This also resolves some issues with include orders around the the WKTs.

Fixes protocolbuffers#4301
Fixes protocolbuffers#4403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants