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

0.52.0-rc.0: Yoga Value Conversion Issue preventing build #17274

Closed
jaysig opened this issue Dec 19, 2017 · 76 comments
Closed

0.52.0-rc.0: Yoga Value Conversion Issue preventing build #17274

jaysig opened this issue Dec 19, 2017 · 76 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@jaysig
Copy link

jaysig commented Dec 19, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Steps to Reproduce

(Write your steps here:)

  1. Try to build app in xCode
  2. Receive Error Implicit conversion loses integer precision: 'size_type' (aka 'unsigned long') to 'const uint32_t' (aka 'const unsigned int')
    line 208 in YGNodePrint.cpp const uint32_t childCount = node->children.size();

Not sure if this is related but Line 201 and 293 of Yoga.cpp are also throwing logic errors of Null pointer argument in call to memory copy function

Expected Behavior

I expected my app to work. It had hung up at the splash screen with no errors except the yoga logic errors and I assumed the newer version would resolve them.

Actual Behavior

The app is unable to build due to the yoga issue and xCodes says there are two other logic errors in Yoga.

Reproducible Demo

I would imagine this would just show up in any new project; however, any suggested steps to resolve it would be appreciated.

@shameemz
Copy link

Facing the same issue after upgrading to 0.52.0-rc.0.

@jcady
Copy link

jcady commented Dec 20, 2017

I'm seeing this same issue in Xcode 9.2 with 0.52.0-rc.0.

@hpb0412
Copy link

hpb0412 commented Jan 2, 2018

The same thing happened for me on Xcode 9.2 (9C40b) with yoga 0.52.0-rc.0 (after update to react-native@0.52.0-rc.0)

@blackxored
Copy link

It is not fixed on master either.

@gabrielbull
Copy link
Contributor

gabrielbull commented Jan 8, 2018

Version 0.52.0 was released with this bug still present. I bet there's going to be an influx of people having this issue now. We really should have resolved this issue before releasing 0.52.0 😬.

@gabrielbull
Copy link
Contributor

Let's bring in @priteshrnandgaonkar into this discussion as he's the one who worked on the file causing issues.

@turnrye
Copy link
Contributor

turnrye commented Jan 8, 2018

b08a912 would be the commit I believe

@priteshrnandgaonkar
Copy link
Contributor

I ran RNTester shipped with RC. It built and ran successfully. The messages which are mentioned above are just warnings, those shouldn't be the cause of the problem. Although I will fix those warnings

@fungilation
Copy link

fungilation commented Jan 9, 2018

@gabrielbull I confirm Xcode build stalled at this exact same line with same error, in YGNodePrint.cpp, on RN 0.52.0. How/Why did RN release ship with a glaring show stopper again like this is a good question. 😭

Any idea for a workaround?

@LeVadim
Copy link

LeVadim commented Jan 9, 2018

Same issue here, it won't build due to this error

@kontinuity
Copy link

kontinuity commented Jan 9, 2018


❌  ...node_modules/react-native/ReactCommon/yoga/yoga/YGNodePrint.cpp:208:46: implicit conversion loses integer precision: 'size_type' (aka 'unsigned long') to 'const uint32_t' (aka 'const unsigned int') [-Werror,-Wshorten-64-to-32]

  const uint32_t childCount = node->children.size();
              ^~~~~~


▸ Compiling yoga-dummy.m
▸ Compiling Yoga.cpp
** ARCHIVE FAILED **


The following build commands failed:
	CompileC ...Library/Developer/Xcode/DerivedData/xxxx-eouawjjkefabkkexnsxilamhdmzd/Build/Intermediates.noindex/ArchiveIntermediates/xxxx-Release/IntermediateBuildFilesPath/Pods.build/Release-iphoneos/yoga.build/Objects-normal/arm64/YGNodePrint.o ...node_modules/react-native/ReactCommon/yoga/yoga/YGNodePrint.cpp normal arm64 c++ com.apple.compilers.llvm.clang.1_0.compiler

The build fails due to this fatal error.

@hanweixing
Copy link

Got this too. Mark this issue, expect solutions soon.

@bkitduy
Copy link

bkitduy commented Jan 9, 2018

Fix please :(. I'm facing this issue (React 0.52.0 - yoga 0.52.0)

@hitbear518
Copy link

Same issue. Integrated with existing app, upgraded from 0.50.4

@raviatflexiloans
Copy link

Facing the same issues. Getting following error:

error: implicit conversionloses integer precision: 'size_type' (aka 'unsigned long') to 'uint32_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
return node->children.size();

facebook-github-bot pushed a commit to facebook/yoga that referenced this issue Jan 9, 2018
Summary: There were warnings of castings and null pointer handling in yoga.cpp. This diff fixes the warnings. The issue was brought up here facebook/react-native#17274

Reviewed By: emilsjolander

Differential Revision: D6675111

fbshipit-source-id: 884659fabb05033b4d43d3aa6629e22481d39b7e
facebook-github-bot pushed a commit to facebook/litho that referenced this issue Jan 9, 2018
Summary: There were warnings of castings and null pointer handling in yoga.cpp. This diff fixes the warnings. The issue was brought up here facebook/react-native#17274

Reviewed By: emilsjolander

Differential Revision: D6675111

fbshipit-source-id: 884659fabb05033b4d43d3aa6629e22481d39b7e
@allthetime
Copy link

How can I incorporate these new changes?

@vovkasm
Copy link
Contributor

vovkasm commented Jan 9, 2018

People!

Dev process

This issue was opened before 0.52.0 release, how and why release happened? What was/will be done to not repeat this in future?

Technical

The messages which are mentioned above are just warnings, those shouldn't be the cause of the problem.

They are not "just warnings", they signs of holes in logic or design, inconsistency in type systems or other flawless. But this other story, same as transit yoga to C++... there are so many pitfalls only to allow inlining of methods :-/

I had to say this.
C/C++ code in RN will be compiled on various platforms with many-many various settings. E.g. android with two build systems (buck/ndk, right?), ios with two build systems: standard, by inclusion of React.xcodeproj etc... and cocoapods, cocoapods generates own xcodeproj files, so settings for C/C++ files are different.
Take all these variants, I can suggest to:

  1. Make C/C++ code maximum clean and warning free.
  2. Identify all build variants/systems and test build automatically with these variants.
  3. Make it clear in docs about required build settings and/or create some single place to store this settings and tool to update it in build files?

@vovkasm
Copy link
Contributor

vovkasm commented Jan 9, 2018

Addition to previous:

To workaround -Werror settings for yoga (only RN integrated with cocoapods), add this to Podfile:

post_install do |installer|
    installer.pods_project.targets.each do |target|
        if target.name == 'yoga'
            target.build_configurations.each do |config|
                config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'NO'
            end
        end
    end
end

But I make it only to find #17027 :-(

@Jono20201
Copy link

Same issue.

@guysegal
Copy link

guysegal commented Jan 9, 2018

I'm also getting Yoga related errors on iOS compilation, see:
screen shot 2018-01-09 at 19 15 05

and:

screen shot 2018-01-09 at 19 16 19

is it related?

@vovkasm
Copy link
Contributor

vovkasm commented Jan 9, 2018

@guysegal this most probably not related, this is more like integration problems. If you use cocoapods, may be this helps. If you use vanilla react-native project, I can suggest to generate new empty project and compare Xcode project settings, most notable schema settings.

@ma-pe
Copy link

ma-pe commented Jan 9, 2018

Downgrading to React-Native 0.50 worked for me as a "work-around".

edit: 0.51 also works fine.

@fungilation
Copy link

fungilation commented Jan 9, 2018

Downgrading to React-Native 0.50.0 worked for me as a work-around.

Considering I upgraded from RN 0.51, that's not a workaround, that's giving up. 😭

@gabrielbull
Copy link
Contributor

@fungilation Haha, you can downgrade to 0.51 as well, this issue is specific to 0.52.0. We'll need to wait for 0.52.1 to update.

@fungilation
Copy link

@vovkasm, that's unrelated. I already have this in my Podfile:

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

@gabrielbull this is most appropriate.

@dluksza
Copy link

dluksza commented Jan 9, 2018

In my case, I also need to disable GCC_WARN_64_TO_32_BIT_CONVERSION for yoga and now my project compiles and runs perfectly.

The full workaround for this issue if you are using cocoapods:

post_install do |installer|
    installer.pods_project.targets.each do |target|
        if target.name == 'yoga'
            target.build_configurations.each do |config|
                config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'NO'
                config.build_settings['GCC_WARN_64_TO_32_BIT_CONVERSION'] = 'NO'
            end
        end
    end
end

@msand
Copy link
Contributor

msand commented Jan 18, 2018

@tonygriv @itinance Make sure your Podfile contains both the tvOS bug workaround:
https://github.com/msand/SVGPodTest/blob/fe45f88a936181e6ecaddeb68268d33268b56121/ios/Podfile#L19
and the workaround for this bug:
https://github.com/msand/SVGPodTest/blob/fe45f88a936181e6ecaddeb68268d33268b56121/ios/Podfile#L56-L65

@antonsivogrivov
Copy link

@msand, adding this

config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'NO'
config.build_settings['GCC_WARN_64_TO_32_BIT_CONVERSION'] = 'NO'

in podfile helps me, but is it a good solution?

@msand
Copy link
Contributor

msand commented Jan 19, 2018

@tonygriv It complies, but, No. I think the upcoming fix is no good either. They should stick to the same number of bits everywhere, and not cast it, losing precision is guaranteed to cause bugs when someone puts a large enough input. There should be checks for too large inputs, independent of the chosen integer precision. And if 64 bit is allowed somewhere, it should be used everywhere along its read-write dependency graph.

@OceanHorn
Copy link

PR #17722 could also solve.

@ahmetilhann
Copy link

Example for podfile

# Uncomment the next line to define a global platform for your project
# platform :ios, '9.0'

target 'podExample' do
  # Uncomment the next line if you're using Swift or would like to use dynamic frameworks
  # use_frameworks!

  # Pods for podExample

  # Your 'node_modules' directory is probably in the root of your project,
  # but if not, adjust the `:path` accordingly
  pod 'React', :path => '../node_modules/react-native', :subspecs => [
    'Core',
    'DevSupport', # Include this to enable In-App Devmenu if RN >= 0.43
    'RCTText',
    'RCTNetwork',
    'RCTWebSocket', # needed for debugging
    # Add any other subspecs you want to use in your project
  ]
  
  # Explicitly include Yoga if you are using RN >= 0.42.0
  pod 'yoga', :path => '../node_modules/react-native/ReactCommon/yoga'

  # Third party deps podspec link
  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'

end

post_install do |installer|
  installer.pods_project.targets.each do |target|
      if target.name == 'yoga'
          target.build_configurations.each do |config|
              config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'NO'
              config.build_settings['GCC_WARN_64_TO_32_BIT_CONVERSION'] = 'NO'
          end
      end
  end
end

@willbattel
Copy link

Error is still present in 0.52.1. Any updates on an actual solution and not just temporary workarounds? 0.52 has been out for a while now and this issue is causing trouble for quite a few people.

@hugomosh
Copy link

I came here due to the react-native-maps iOS installation and this manage to do the work. And It works until I clean the project and did pod install again : )

# Uncomment the next line to define a global platform for your project
# platform :ios, '9.0'

target 'leoparkcityappv0' do
    rn_path = '../node_modules/react-native'
    rn_maps_path = '../node_modules/react-native-maps'
  
    #pod 'yoga', path: "#{rn_path}/ReactCommon/yoga/yoga.podspec" #From react-native-maps
    # Explicitly include Yoga if you are using RN >= 0.42.0
    pod 'yoga', :path => '../node_modules/react-native/ReactCommon/yoga'

    pod 'React', path: rn_path, subspecs: [
      'Core',
      'RCTActionSheet',
      'RCTAnimation',
      'RCTGeolocation',
      'RCTImage',
      'RCTLinkingIOS',
      'RCTNetwork',
      'RCTSettings',
      'RCTText',
      'RCTVibration',
      'RCTWebSocket',
      'BatchedBridge',
      'tvOS', # https://github.com/facebook/react-native/issues/17274#issuecomment-356427445
    ]
  
    pod 'react-native-maps', path: rn_maps_path
  
    #pod 'GoogleMaps'  # Remove this line if you don't want to support Google Maps on iOS
    #pod 'react-native-google-maps', path: rn_maps_path  # Remove this line if you don't want to support Google Maps on iOS
  end
  
  post_install do |installer|
    installer.pods_project.targets.each do |target|
        if target.name == 'yoga'
            # Workaround: react-native v0.52 bug issue #17274
            # node_modules/react-native/ReactCommon/yoga/yoga/YGNodePrint.cpp:208:46: Implicit conversion loses integer
            # precision: 'size_type' (aka 'unsigned long') to 'const uint32_t' (aka 'const unsigned int')
            # https://github.com/facebook/react-native/issues/17274#issuecomment-356363557
            target.build_configurations.each do |config|
                config.build_settings['GCC_TREAT_WARNINGS_AS_ERRORS'] = 'NO'
                config.build_settings['GCC_WARN_64_TO_32_BIT_CONVERSION'] = 'NO'
            end
        end
      if target.name == 'react-native-google-maps'
        target.build_configurations.each do |config|
          config.build_settings['CLANG_ENABLE_MODULES'] = 'No'
        end
      end
      if target.name == "React"
        target.remove_from_project
      end
     
    end
  end

  

@grabbou
Copy link
Contributor

grabbou commented Jan 25, 2018

@msand will do it right away.

@maxkomarychev
Copy link
Contributor

@grabbou is there any reason why yoga sources are distributed within RN and not just referenced as dependency from it's origin repo?

@grabbou
Copy link
Contributor

grabbou commented Jan 26, 2018

The main reason (as far as I am aware) is that at Facebook, everything lives in a single repository and so, all projects depend on "master".

When RN gets released in the open source, we have to "lock" its version so that we don't ship master-dependant build onto npm. To do so, one would need to make sure these two get released at the same time.

That's been the case of few issues with metro-bundler.

CC: @emilsjolander you might have a better idea here :)

@grabbou
Copy link
Contributor

grabbou commented Jan 26, 2018

The commit has been cherry-picked and will be landing soon. Now, that the original issue has been fixed, I will close this one.

For any other, Yoga-specific requests (like this fix being inappropriate or not the most optimal way), please try opening a solution directly on Yoga repository.

@grabbou grabbou closed this as completed Jan 26, 2018
@msand
Copy link
Contributor

msand commented Jan 26, 2018

@grabbou Excellent, thank you!

@guysegal
Copy link

@grabbou just to be sure - after the fix do I still need hacks in the podfile to be able to compile?

@grabbou
Copy link
Contributor

grabbou commented Jan 26, 2018

No - the code should be gone and so, no need to mute the warnings anymore :)

@maxkomarychev
Copy link
Contributor

maxkomarychev commented Jan 26, 2018

I installed latest release with "react-native": "git+https://github.com/facebook/react-native#v0.52.2" (since it's not available via npm yet)

And now I'm getting this:

❌  /Users/***/node_modules/react-native/ReactCommon/yoga/yoga/YGNodePrint.cpp:208:59: no member named 'getChildren' in 'YGNode'

  const uint32_t childCount = static_cast<uint32_t>(node->getChildren().size());
                                                    ~~~~  ^


▸ Compiling Yoga.cpp

❌  /Users/***/node_modules/react-native/ReactCommon/yoga/yoga/Yoga.cpp:474:38: no member named 'getChildren' in 'YGNode'

  return static_cast<uint32_t>(node->getChildren().size());
                               ~~~~  ^

can someone confirm you can compile?

@maxkomarychev
Copy link
Contributor

Ok false alarm, I removed node_modules and installed everything from scratch. Able to compile now.

@grabbou
Copy link
Contributor

grabbou commented Jan 26, 2018 via email

@3degreeszw
Copy link

1.$ npm i -g react-native-git-upgrade
2.inside your react-native project folder run
$ react-native-git-upgrade
3.delete ios/build, ios/Pods
4.$ cd ios && pod install
5.$ quite metro bundler if running
6.$ npm run ios

@jayasimhaprasad
Copy link

@3degreeszw your steps worked perfectly, thank you for posting it here

@hegelstad
Copy link

You are a savior @3degreeszw!

I had a problem with adding react-native-maps on top of react-native-firebase.

@HemantRMali
Copy link

HemantRMali commented Mar 10, 2018

Downgraded to "react-native": "^0.51.0", it seems stable version, and works fine
before that, i was using "react-native": "^0.54.0"

@sadeshp
Copy link

sadeshp commented Mar 27, 2018

@grabbou The issue I am facing is adding use_framework! in podfile resulting into compile time error.
I am not able to build the project.

I am getting below errors.
#import <yoga/Yoga> -------> Could not build module Yoga.
#include -------> algorithm file not found.

I am using react-native 0.54.3 which is latest.

Does it mean, yoga is not supported when we use use_framework!?

vincentriemer pushed a commit to vincentriemer/yoga-dom that referenced this issue May 9, 2018
Summary: There were warnings of castings and null pointer handling in yoga.cpp. This diff fixes the warnings. The issue was brought up here facebook/react-native#17274

Reviewed By: emilsjolander

Differential Revision: D6675111

fbshipit-source-id: 884659fabb05033b4d43d3aa6629e22481d39b7e
@facebook facebook locked as resolved and limited conversation to collaborators Jan 26, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jan 26, 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.
Projects
None yet
Development

No branches or pull requests