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

Fabric should be excluded from Podspec build #18683

Closed
3 tasks done
slycoder opened this issue Apr 4, 2018 · 16 comments
Closed
3 tasks done

Fabric should be excluded from Podspec build #18683

slycoder opened this issue Apr 4, 2018 · 16 comments
Labels
Resolution: Locked This issue was locked by the bot. Resolution: PR Submitted A pull request with a fix has been provided.

Comments

@slycoder
Copy link

slycoder commented Apr 4, 2018

Version 0.55 of React Native includes an experimental Fabric library which has some issues building in a Podspec environment. In particular, it is pulled into Core via the source_files wildcard and directly includes headers from folly --- however without folly_compiler_flags being set on the Core subspec, this causes build breakage.

Environment

OS: macOS High Sierra 10.13.3
Node: 9.6.1
Yarn: 1.5.1
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.3 Build version 9E145
Android Studio: 3.0 AI-171.4443003

Packages: (wanted => installed)
react: 16.3.1 => 16.3.1
react-native: 0.55.0 => 0.55.0

Steps to Reproduce

Create a Pods based project. Here's an example Podfile snippet:

pod 'yoga', path: '../node_modules/react-native/ReactCommon/yoga'
pod 'DoubleConversion', podspec: '../node_modules/react-native/third-party-podspecs/DoubleConversion.podspec'
pod 'glog', podspec: '../node_modules/react-native/third-party-podspecs/glog.podspec'
pod 'Folly', podspec: '../node_modules/react-native/third-party-podspecs/Folly.podspec'
pod 'React', path: '../node_modules/react-native', :subspecs => [
  'Core',
  'CxxBridge',
  'DevSupport',
  'RCTWebSocket',
  'RCTGeolocation',
  'RCTLinkingIOS',
  'RCTSettings',
  'RCTText',
  'RCTVibration',
  'RCTImage',
  'RCTAnimation',
  'RCTActionSheet',
  'RCTCameraRoll',
]

Expected Behavior

The project builds.

Actual Behavior

An error is thrown while building complaining that folly/folly-config.h is not found:

In file included from <redacted>/node_modules/react-native/React/Fabric/RCTFabricUIManagerWrapper.mm:10:
In file included from <redacted>/node_modules/react-native/ReactCommon/fabric/FabricUIManager.h:10:
In file included from <redacted>/ios/Pods/Folly/folly/dynamic.h:77:
In file included from <redacted>/ios/Pods/Folly/folly/Range.h:22:
In file included from <redacted>/ios/Pods/Folly/folly/FBString.h:41:
In file included from <redacted>/ios/Pods/Folly/folly/Portability.h:23:
<redacted>/ios/Pods/Folly/folly/portability/Config.h:20:10: fatal error: 'folly/folly-config.h' file not found
#include <folly/folly-config.h>
         ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Inspection of the compile command being executed confirms that -DFOLLY_NO_CONFIG is NOT being set.

Adding

                              "React/Fabric/*"

to the exclude_files section of the Core subspec fixes the issue.

Gyran added a commit to Gyran/react-native that referenced this issue Apr 4, 2018
@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest release, v0.54?

Thank you for your contributions.

@slycoder
Copy link
Author

slycoder commented Apr 4, 2018

Domo arigato, Mr React-native-bot-o, but isn't v0.55 > v0.54?

@hramos
Copy link
Contributor

hramos commented Apr 4, 2018

0.55.0 has some issues and we are asking people to stay on 0.54.x for now, which is why the latest release (github.com/facebook/react-native/releases) is no longer 0.55.

It's alright to ignore the bot for now.

@hramos hramos added Resolution: PR Submitted A pull request with a fix has been provided. and removed ⏪Old Version labels Apr 4, 2018
campsafari pushed a commit to exozet/react-native that referenced this issue Apr 11, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this issue Apr 11, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
LukeDurrant pushed a commit to LukeDurrant/react-native that referenced this issue Apr 11, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
evilDave pushed a commit to evilDave/react-native that referenced this issue Apr 13, 2018
@peacechen
Copy link

@hramos
React Native's official site shows 0.55 as current stable
https://facebook.github.io/react-native/

@shafy
Copy link

shafy commented Apr 16, 2018

This error still persists for me with 0.55.2. Should it be fixed?

@hung-yueh
Copy link

Looks like this issue still there if you do upgrade from old react-native version, so 0.55 shouldn't be the stable version at this point. Should we re-open this issue? or create a new one?

@youhaowei
Copy link

add React/Fabric/* as mentioned above fixed the issue, just wondering if this would make to the next version or not.

@shafy
Copy link

shafy commented Apr 17, 2018

How do I add it to exclude_files. Could you show me your Podfile?

grabbou pushed a commit that referenced this issue Apr 17, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in #18683 and got it to build.

Fixes #18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes #18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
@youhaowei
Copy link

@shafy, I just edited the node_modules/react-native/React.podspec, and reran pod install. Not pretty but worked.

@shafy
Copy link

shafy commented Apr 17, 2018

Oh ok, thanks!

@grabbou
Copy link
Contributor

grabbou commented Apr 17, 2018 via email

jacobp100 pushed a commit to OurStarClub/react-native that referenced this issue Apr 27, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
@YYCoder1963
Copy link

add "React/Fabric/*" did not work for me, Are there any other solutions??

@khorark
Copy link

khorark commented May 24, 2018

@lyayun, I have version 0.55.4 and problem solution for me. But 0.55.3 I had problem.

@objHua
Copy link

objHua commented May 24, 2018

the release to 0.55.4, but there were still problems。
image

@georgecr0ss
Copy link

@lyayun have you searched for this flag -DFOLLY_NO_CONFIG ? If it's set remove it

macdoum1 pushed a commit to macdoum1/react-native that referenced this issue Jun 28, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
woshi82 pushed a commit to ant-worker/react-native that referenced this issue Jul 10, 2018
Summary:
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

React native does not build on iOS when using Podspec environment. I tried the solution proposed in facebook#18683 and got it to build.

Fixes facebook#18683

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[IOS] [BUGFIX] [React.podspec] - Exclude `React/Fabric/*` from Core subspec
<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18688

Differential Revision: D7505267

Pulled By: hramos

fbshipit-source-id: 8e7138ee966d08209a74193e418feaecceefb86a
@YVvanessa
Copy link

@shafy, I just edited the node_modules/react-native/React.podspec, and reran pod install. Not pretty but worked.

like this👇👇??
but there were still problems😯😯

s.subspec "Core" do |ss|
ss.dependency "yoga", "#{package["version"]}.React"
ss.source_files = "React//*.{c,h,m,mm,S,cpp}"
ss.exclude_files = "
/tests/",
"IntegrationTests/
",
"React/DevSupport/",
"React/Inspector/
",
"ReactCommon/yoga/",
"React/Cxx
/",
"React/Fabric/**/
",
"React/Fabric/*"

@facebook facebook locked as resolved and limited conversation to collaborators Apr 4, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Apr 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot. Resolution: PR Submitted A pull request with a fix has been provided.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

14 participants