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

[CocoaPods] Podspec doesn't set up header search paths for <CSSLayout> imports #9014

Closed
ide opened this issue Jul 25, 2016 · 13 comments
Closed
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.

Comments

@ide
Copy link
Contributor

ide commented Jul 25, 2016

RN includes CSSLayout files using angle brackets like #import <CSSLayout/CSSLayout.h>, which don't work with CocoaPods right now since CP flattens the headers by default. We either need to figure out how to preserve the directory hierarchy just for React/CSSLayout or preserve the entire directory hierarchy and set up the header search path to be recursive I think.

@Gladkov-Art
Copy link

Gladkov-Art commented Aug 9, 2016

It seems that I have similar issue related to importing CSSLayout.
I can't build React pod. Compiling of CSSNodeList.c fails with fatal error: 'CSSLayout/CSSLayout.h' file not found.
Probably my setup will be useful:
package.json:

{
  "name": "Chat",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "start": "node node_modules/react-native/local-cli/cli.js start"
  },
  "dependencies": {
    "react": "~15.2.1",
    "react-native": "~0.31.0",
    "react-native-gifted-chat": "0.0.3"
  }
}

podfile:

target 'Chat' do
   use_frameworks!

   pod 'React', :path => './node_modules/react-native', :subspecs => [
    'Core',
    'RCTText',
    'RCTWebSocket',
    'RCTImage',
  ]
end

I have latest stable version of XCode (7.3.1), cocoapods v1.0.1 and new swift project.
I've tried to delete Pods/, node_modules/, Podfile.lock and install everything again.

I reproduced the issue on my office mac.
BTW, downgrading to 0.30.0 solves it.

@cknitt
Copy link

cknitt commented Aug 10, 2016

Same problem here, occurs only when use_frameworks! is set in the Podfile (which is required when using pods implemented in Swift).

Downgrading to 0.30 worked for me, too.

@guysegal
Copy link

Same here .We have react native project with Swift and the error started to occur on 0.31.
We also downgraded to 0.30 and it solved it..

@moosch
Copy link

moosch commented Aug 18, 2016

+1 I did the same as @guysegal and it's fine

@robhogan
Copy link
Contributor

This looks to be down to a Cocoapods/Xcode bug, but I haven't found a clean solution yet - CocoaPods/CocoaPods#4605

@sdcooke
Copy link

sdcooke commented Aug 18, 2016

It's not clean but we've worked around this using an npm postinstall script that rewrites imports: replace(/#(include|import) <CSSLayout\/([A-Za-z]+)\.h>/, '#$1 "$2.h"'); (#import <CSSLayout/CSSlayout.h> becomes #import "CSSLayout.h")

Definitely not clean or ideal and may or may not work depending on how you're using React in your codebase. Thought I'd post here just in case it helps anyone else.

@guysegal
Copy link

Seems like its a blocker for a lot of projects to upgrade to 0.31..

@robhogan
Copy link
Contributor

After doing some digging I think @sdcooke's solution is the right one, so I've submitted a PR to get it changed at source.

In the mean time, I'm using "postinstall": "find ./node_modules/react-native \\( -name *.h -o -name *.m \\) -print0 | xargs -0 sed -i '' -e 's:<CSSLayout/\\(.*\\)>:\"\\1\":g'"

ghost pushed a commit that referenced this issue Aug 24, 2016
Summary:
This PR changes `#include <CSSLayout/*.h>` to  `#include "*.h"` within the `CSSLayout` directory.

Rationale: Quote includes are preferred for user (aka local/project) includes, whereas angle includes are preferred for standard libraries and external frameworks. In particular, XCode 7.1+ will not search user paths (even the current directory) when angle brackets are used unless "Always search user paths" is enabled - it is off by default and [Apple recommend](https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW110) that it is only enabled for backwards compatibility.

I think this is the best fix for #9014, and seems like good practice in any case.
Closes facebook/yoga#217

Reviewed By: majak

Differential Revision: D3764132

Pulled By: emilsjolander

fbshipit-source-id: c8a6e8d19db71455922e3ba8f6c72bd66018fa84
ghost pushed a commit to facebook/yoga that referenced this issue Aug 24, 2016
Summary:
This PR changes `#include <CSSLayout/*.h>` to  `#include "*.h"` within the `CSSLayout` directory.

Rationale: Quote includes are preferred for user (aka local/project) includes, whereas angle includes are preferred for standard libraries and external frameworks. In particular, XCode 7.1+ will not search user paths (even the current directory) when angle brackets are used unless "Always search user paths" is enabled - it is off by default and [Apple recommend](https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW110) that it is only enabled for backwards compatibility.

I think this is the best fix for facebook/react-native#9014, and seems like good practice in any case.
Closes #217

Reviewed By: majak

Differential Revision: D3764132

Pulled By: emilsjolander

fbshipit-source-id: c8a6e8d19db71455922e3ba8f6c72bd66018fa84
@senpng
Copy link

senpng commented Sep 1, 2016

In Swift Project.
Try "postinstall": "find ./node_modules/react-native \\( -name *.h -o -name *.m \\) -print0 | xargs -0 sed -i '' -e 's:<CSSLayout/\\(.*\\)>:\"\\1\":g'"
And build error,
qq20160901-0

package.json
qq20160901-1

@senpng
Copy link

senpng commented Sep 1, 2016

I tried to solve this problem。
I was such a modification:
package.json

{
  "name": "xxxxxxx",
  "version": "1.0.0",
  "description": "",
  "main": "",
  "scripts": {
    "start": "node node_modules/react-native/local-cli/cli.js start",
    "bundle": "react-native bundle --platform ios --entry-file index.ios.js --bundle-output bundle/main.ios.jsbundle --assets-dest bundle/ --dev false",
    "postinstall": "find ./node_modules/react-native \\( -name *.h -o -name *.m \\) -print0 | xargs -0 sed -i '' -e 's:<CSSLayout/\\(.*\\)>:\"\\1\":g'"
  },
  "author": "senpng@qq.com",
  "license": "ISC",
  "dependencies": {
    "react": "^15.3.0",
    "react-native": "^0.32.0",
    "react-native-fs": "^2.0.1-rc.2"
  }
}

the most important is "postinstall": "find ./node_modules/react-native \\( -name *.h -o -name *.m \\) -print0 | xargs -0 sed -i '' -e 's:<CSSLayout/\\(.*\\)>:\"\\1\":g'" (thinks @rh389 )

Edit node_modules/react-native/React.podspec File:
line 46

 s.subspec 'CSSLayout' do |ss|
    ss.source_files        = "React/CSSLayout/**/*.{c,h}"
    ss.header_mappings_dir = "React"
  end

changed to

 s.subspec 'CSSLayout' do |ss|
    ss.source_files        = "React/CSSLayout/**/*.{c,h}"
  end

Before pod install

@robhogan
Copy link
Contributor

robhogan commented Sep 2, 2016

Good find - that seems to be because CocoaPods will usually copy all headers to the same directory, but specifying React as a header_mappings_dir overrides that behaviour to preserve all paths under React, so has quite a wide impact (@alloy may have more insight). #9544 works too by removing the subspec completely, and I guess adding a recursive user header search path would also work (you could do that from your own Podfile or as a workspace setting).

@alloy
Copy link
Contributor

alloy commented Sep 2, 2016

#9544 is definitely the good long-term solution, in the interim there are many stopgap solutions. Yours would work, as would @rh389’s:

I guess adding a recursive user header search path would also work (you could do that from your own Podfile

Which can be done with a post_install hook https://guides.cocoapods.org/syntax/podfile.html#post_install.

@ghost ghost closed this as completed in 6e216d2 Sep 6, 2016
mikelambert pushed a commit to mikelambert/react-native that referenced this issue Sep 19, 2016
Summary:
Include CSSLayout headers in the same way as other project headers, ie `#import <CSSLayout/CSSLayout.h>` becomes `#import "CSSLayout.h"`. CSSLayout is not a framework or system dependency, so shouldn't (AFAIK) be included with angle brackets. Doing so breaks framework builds, such as when RN is used as a pod in a swift project.

In combination with facebook/yoga#217 this fixes facebook#9014 (specifically swift cocoapods projects). There is then no need for a separate CSSLayout pod subspec.

Tests run on the RN project in isolation (with changes inside `CSSLayout` itself also applied) and against a dummy swift project with RN included as a pod.

NB: This effectively reverts facebook#9015 and may break non-swift cocoapods projects unless facebook/yoga#217 is merged and synced first.

Update: As discussed with alloy and emilsjolander, wrap these imports in a preprocess
Closes facebook#9544

Differential Revision: D3821791

Pulled By: javache

fbshipit-source-id: d27ac8be9ce560d03479b43d3db740cd196c24da
spikebrehm pushed a commit to airbnb/react-native that referenced this issue Sep 20, 2016
Summary:
Include CSSLayout headers in the same way as other project headers, ie `#import <CSSLayout/CSSLayout.h>` becomes `#import "CSSLayout.h"`. CSSLayout is not a framework or system dependency, so shouldn't (AFAIK) be included with angle brackets. Doing so breaks framework builds, such as when RN is used as a pod in a swift project.

In combination with facebook/yoga#217 this fixes facebook#9014 (specifically swift cocoapods projects). There is then no need for a separate CSSLayout pod subspec.

Tests run on the RN project in isolation (with changes inside `CSSLayout` itself also applied) and against a dummy swift project with RN included as a pod.

NB: This effectively reverts facebook#9015 and may break non-swift cocoapods projects unless facebook/yoga#217 is merged and synced first.

Update: As discussed with alloy and emilsjolander, wrap these imports in a preprocess
Closes facebook#9544

Differential Revision: D3821791

Pulled By: javache

fbshipit-source-id: d27ac8be9ce560d03479b43d3db740cd196c24da
spikebrehm pushed a commit to airbnb/react-native that referenced this issue Sep 21, 2016
Summary:
This PR changes `#include <CSSLayout/*.h>` to  `#include "*.h"` within the `CSSLayout` directory.

Rationale: Quote includes are preferred for user (aka local/project) includes, whereas angle includes are preferred for standard libraries and external frameworks. In particular, XCode 7.1+ will not search user paths (even the current directory) when angle brackets are used unless "Always search user paths" is enabled - it is off by default and [Apple recommend](https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW110) that it is only enabled for backwards compatibility.

I think this is the best fix for facebook#9014, and seems like good practice in any case.
Closes facebook/yoga#217

Reviewed By: majak

Differential Revision: D3764132

Pulled By: emilsjolander

fbshipit-source-id: c8a6e8d19db71455922e3ba8f6c72bd66018fa84
spikebrehm pushed a commit to airbnb/react-native that referenced this issue Sep 21, 2016
Summary:
Include CSSLayout headers in the same way as other project headers, ie `#import <CSSLayout/CSSLayout.h>` becomes `#import "CSSLayout.h"`. CSSLayout is not a framework or system dependency, so shouldn't (AFAIK) be included with angle brackets. Doing so breaks framework builds, such as when RN is used as a pod in a swift project.

In combination with facebook/yoga#217 this fixes facebook#9014 (specifically swift cocoapods projects). There is then no need for a separate CSSLayout pod subspec.

Tests run on the RN project in isolation (with changes inside `CSSLayout` itself also applied) and against a dummy swift project with RN included as a pod.

NB: This effectively reverts facebook#9015 and may break non-swift cocoapods projects unless facebook/yoga#217 is merged and synced first.

Update: As discussed with alloy and emilsjolander, wrap these imports in a preprocess
Closes facebook#9544

Differential Revision: D3821791

Pulled By: javache

fbshipit-source-id: d27ac8be9ce560d03479b43d3db740cd196c24da
gilbox pushed a commit to gilbox/react-native that referenced this issue Nov 23, 2016
Summary:
This PR changes `#include <CSSLayout/*.h>` to  `#include "*.h"` within the `CSSLayout` directory.

Rationale: Quote includes are preferred for user (aka local/project) includes, whereas angle includes are preferred for standard libraries and external frameworks. In particular, XCode 7.1+ will not search user paths (even the current directory) when angle brackets are used unless "Always search user paths" is enabled - it is off by default and [Apple recommend](https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW110) that it is only enabled for backwards compatibility.

I think this is the best fix for facebook#9014, and seems like good practice in any case.
Closes facebook/yoga#217

Reviewed By: majak

Differential Revision: D3764132

Pulled By: emilsjolander

fbshipit-source-id: c8a6e8d19db71455922e3ba8f6c72bd66018fa84
gilbox pushed a commit to airbnb/react-native that referenced this issue Nov 23, 2016
Summary:
This PR changes `#include <CSSLayout/*.h>` to  `#include "*.h"` within the `CSSLayout` directory.

Rationale: Quote includes are preferred for user (aka local/project) includes, whereas angle includes are preferred for standard libraries and external frameworks. In particular, XCode 7.1+ will not search user paths (even the current directory) when angle brackets are used unless "Always search user paths" is enabled - it is off by default and [Apple recommend](https://developer.apple.com/library/mac/documentation/DeveloperTools/Reference/XcodeBuildSettingRef/1-Build_Setting_Reference/build_setting_ref.html#//apple_ref/doc/uid/TP40003931-CH3-SW110) that it is only enabled for backwards compatibility.

I think this is the best fix for facebook#9014, and seems like good practice in any case.
Closes facebook/yoga#217

Reviewed By: majak

Differential Revision: D3764132

Pulled By: emilsjolander

fbshipit-source-id: c8a6e8d19db71455922e3ba8f6c72bd66018fa84
@hitbear518
Copy link

hitbear518 commented Dec 4, 2016

Can anyone tell me how should I add a recursive user header search path for workaround before pr merged?

sjmueller referenced this issue Dec 10, 2016
Summary:
To make React Native play nicely with our internal build infrastructure we need to properly namespace all of our header includes.

Where previously you could do `#import "RCTBridge.h"`, you must now write this as `#import <React/RCTBridge.h>`. If your xcode project still has a custom header include path, both variants will likely continue to work, but for new projects, we're defaulting the header include path to `$(BUILT_PRODUCTS_DIR)/usr/local/include`, where the React and CSSLayout targets will copy a subset of headers too. To make Xcode copy headers phase work properly, you may need to add React as an explicit dependency to your app's scheme and disable "parallelize build".

Reviewed By: mmmulani

Differential Revision: D4213120

fbshipit-source-id: 84a32a4b250c27699e6795f43584f13d594a9a82
@hramos hramos added Help Wanted :octocat: Issues ideal for external contributors. and removed Help Wanted :octocat: Issues ideal for external contributors. labels Mar 8, 2018
@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Help Wanted :octocat: Issues ideal for external contributors. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.