BREAKING: iOS: Fix case sensitivity build warning in Xcode 8.3

[rigdern]

Breaking Change Notes

To adapt to the breaking change, app developers that are consuming React Native through CocoaPods must update their Podfile to refer to yoga with a lowercase "y". In other words:

pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga'

Must be changed to (note the lowercase "y"):

pod 'yoga', :path => '../node_modules/react-native/ReactCommon/yoga'

Symptom

If you consume React Native as a CocoaPod and consume a third-party React Native module not as a CocoaPod, you will receive a nonportable-include-path warning in Xcode >= 8.3.

Details

Xcode 8.3 introduced -Wnonportable-include-path which triggers if you import a header using different casing than the path to the header on disk. This triggers when consuming React Native as a CocoaPod from a library that isn't a CocoaPod. React Native imports Yoga using paths like this:

#import <yoga/Yoga.h>

Which means Yoga's headers are expected to be located in a directory called "yoga" with a lowercase "y". However, when React Native is a CocoaPod the Yoga headers for non-CocoaPods end up in the directory "$(BUILT_PRODUCTS_DIR)/Yoga".

To fix this such that Yoga's headers end up in "$(BUILT_PRODUCTS_DIR)/yoga" (note the lowercase "y"), I've changed Yoga's podspec name to have a lowercase "y".

Test Plan

Created a test app where React Native is consumed as a CocoaPod. Added the react-native-maps library to the project and configured it to not be consumed through CocoaPods. Verified that the app compiles properly without the nonportable-include-path warnings.

Adam Comella
Microsoft Corp.

[pull-bot]

📄 Thanks for your contribution to the docs!

Attention: @facebook/react-native

Generated by 🚫 dangerJS

[shergin]

Hmm... The official name of the library in terms of Cocoapods is "Yoga". 🤔
https://cocoapods.org/pods/Yoga
cc @emilsjolander

[rigdern]

@alloy Do you have any opinions on this fix? Ideally, we could control the path with module_name but that doesn't work for static libraries. I filed CocoaPods/CocoaPods#6683 about 4 months ago but it hasn't been fixed yet.

This PR which changes the spec.name is the best solution I could come up with.

[ide]

There's a property called header_dir that might help. Have you looked into that?

[rigdern]

@ide thanks for the suggestion. header_dir didn't help. I tried a couple of things.

spec.header_dir = 'yoga' placed the headers in Headers/Public/Yoga/yoga.

spec.header_dir = '../yoga' placed the headers in Headers/Public/yoga which seemed promising.

However, the headers always seemed to get copied into $(BUILT_PRODUCTS_DIR)/Yoga regardless of the value of header_dir but I need them to be copied into $(BUILT_PRODUCTS_DIR)/yoga (lowercase y).

[alloy]

@rigdern I think changing the name is a good fix 👍

[shergin]

¯\_(ツ)_/¯
cc @javache

[facebook-github-bot]

@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[javache]

This causes the podspec test to fail:

 -> yoga (1000.0.0.React)
    - ERROR | name: The name of the spec should match the name of the file.

https://travis-ci.org/facebook/react-native/jobs/268332170

[rigdern]

@javache to fix this, should we rename the Yoga.podspec file to be all lowercase? If so, we'll need to do two separate commits. Changing the casing of a file is tricky with git. Do you want to do this or should I send 2 PRs? If you want me to do it, the earliest I can do it is Monday.

[alloy]

@rigdern Changing case works in 1 commit if you first rename to something else completely. E.g. git mv Yoga.podspec Yoga.podspec.bak && git mv Yoga.podspec.bak yoga.podspec.

[rigdern]

@alloy if that's in 1 commit, I thought git would drop the intermediate file and interpret that as a rename from Yoga.podspec to yoga.podspec which would be problematic.

[javache]

Trying to fix in #15657

There's still another breakage in the podspec test where the umbrella header tries to pull in an Objective-C++ header.

[alloy]

@rigdern hmm, maybe I’m just misremembering.

[alloy]

@javache Are you able to fix it or is it not obvious?

[javache]

I don't have an immediate clue what to do about the umbrella header. As a short-term fix we could just exclude this header (RCTManagedPointer.h) from the public headers, or gate it with #if __cplusplus, since it's unlikely used outside FB.

If you have any ideas to fix it, that would be very much appreciated.

[alloy]

Not from the top of my head. I’ve been wanting to create a prototype of a specs repo for the various libraries inside this repo (I have verified that that principal works), which should get rid of many issues related to having distinct libraries (rather than actual subspecs). But I’m still swamped by upgrading our app to Relay Modern, which I expect to take (on and off) a few more weeks. If your stopgap solution works, I suggest you do that for now.