Test RNTesterPods on CI with use_frameworks! enabled

[jtreanor]

Summary

This adds a test_ios_frameworks job to CircleCI to test the RNTesterPods project with use_frameworks! enabled. It will ensure the issue in #25349 is not reintroduced as suggested in #25619 (comment).

Changelog

[iOS] [Internal] - Added CircleCI job for testing RNTesterPods with use_frameworks! enabled.

Test Plan

Tests seem to be failing on master at the moment but you can see that the new job builds successfully here. You can confirm it installs the pods with use_frameworks! by seeing that Installing pods with use_frameworks! is at the start of the log for the Generate RNTesterPods Workspace step.

[pull-bot]

Messages

📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message. CATEGORY may be: General iOS Android JavaScript Internal (for changes that do not need to be called out in the release notes) TYPE may be: Added, for new features. Changed, for changes in existing functionality. Deprecated, for soon-to-be removed features. Removed, for now removed features. Fixed, for any bug fixes. Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against 7910c91

[jtreanor]

This just adds a second test run with frameworks enabled to the test_ios job. @fkgozali would you prefer this to be a separate parallel job (such as test_ios_frameworks) or is it better here?

[jtreanor]

Actually, I think a separate job probably is better. Let me know if you would like me to change it back.

[fkgozali]

I'll have to defer to @hramos for the actual tradeoff discussion. To me, putting it in one place is easier to maintain (because it's basically testing exactly the same thing, just with framework output).

So for now, let's get this in as is and we can adjust it if needed in the future.

[jtreanor]

Its worth pointing out that fixing #23561 could potentially make this redundant. process-podspecs.sh validates the podspecs with pod lib lint both for frameworks and libraries. When that is working again its arguable that a full test run of RNTesterPods for use_frameworks! would not be needed.

A quick look suggests it shouldn't be too hard to fix process-podspecs.sh. I'll take a look next week.

[fkgozali]

A quick look suggests it shouldn't be too hard to fix process-podspecs.sh.

@hramos: do we still use that script? I think ios CI builds are fully using cocoapods already?

[facebook-github-bot]

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

[jtreanor]

I think the suggestion in that issue is to get it working again. Even if the repo uses CocoaPods extensively it could be useful to validate the podspecs.

[fkgozali]

The CI builds are failing, do you know if it's related to this change?

https://circleci.com/gh/facebook/react-native/103930?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link
https://circleci.com/gh/facebook/react-native/103932?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

[jtreanor]

@fkgozali Not related. master is currently failing with the same error.

[react-native-bot]

This pull request was successfully merged by @jtreanor in bbde55e.

^{When will my fix make it into a release? | Upcoming Releases}

[fkgozali]

@facebook-github-bot facebook-github-bot closed this in bbde55e 1 hour ago

Not sure why the bot didn't close this PR automatically... Anyway, closing!

[facebook-github-bot]

Something went wrong executing that command, @hramos could you take a look?

[grabbou]

The tests are now failing on 0.61-stable branch (https://circleci.com/gh/facebook/react-native/107763) with the following:

❌ Undefined symbols for architecture x86_64
Symbol: _OBJCCLASS$__RCTTypedModuleConstants
Referenced from: objc-class-ref in RCTPlatform.o

I believe this may be related to the CocoaPods integration - does anyone happen to know where to dig this deeper?

[jtreanor]

I can take a look @grabbou. I think this should be relatively straightforward to fix.

[jtreanor]

Okay, so I did a git bisect and it seems that the test failure was first introduced by 2956eb2.

It seems these tests have been failing on master for a while 😞 I'm looking into how much effort it will take to fix.

[grabbou]

Great, thanks! Make sure to check 0.61-stable too, I've reverted one commit there to make other lanes work as well. In the worst case scenario, I think what we can do is we can just revert this commit until a better solution is found.

…

On Wed, 21 Aug 2019 at 16:26, James Treanor ***@***.***> wrote: Okay, so I did a git bisect and it seems that the test failure was first introduced by 2956eb2 <2956eb2> . It seems these tests have been failing on master for a while 😞 I'm looking into how much effort it will take to fix. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#25818?email_source=notifications&email_token=AASZZRUPTHOF3QTH42DLHXLQFVGBNA5CNFSM4IGZHA6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4Z2R6Y#issuecomment-523479291>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASZZRXBKQ6GQ7R3LV63D2TQFVGBNANCNFSM4IGZHA6A> .

[fkgozali]

cc @osdnk

@grabbou / @jtreanor that commit seems likely culprit. To revert it, you'll have to revert 28e18e4 beforehand (they're related change)

[jtreanor]

Great, thanks! Make sure to check 0.61-stable too, I've reverted one commit
there to make other lanes work as well.

Yeah, I've confirmed the problem on 0.61-stable.

@grabbou / @jtreanor that commit seems likely culprit. To revert it, you'll have to revert 28e18e4 beforehand (they're related change)

Thanks @fkgozali! I'll try that out. The alternative approach which has worked for me is to add s.static_framework = true to all of the podspecs as we discussed in #25619 (comment).

[jtreanor]

Reverting those commits fixed the RCTPlatform build error but uncovered a number of other build errors (see below) that seem to be caused by bf78d79. That didn't revert cleanly.

Undefined symbols for architecture x86_64:
  "_RCTGetImageData", referenced from:
      ___45-[RCTImageStoreManager storeImage:withBlock:]_block_invoke in RCTImageStoreManager.o
      ___47-[RCTImageStoreManager(Deprecated) storeImage:]_block_invoke in RCTImageStoreManager.o
  "_OBJC_CLASS_$_RCTImageCache", referenced from:
      objc-class-ref in RCTImageLoader.o
  "_RCTImageHasAlpha", referenced from:
      ___43-[RCTImageLoader sendRequest:withDelegate:]_block_invoke in RCTImageLoader.o
  "_RCTTransformImage", referenced from:
      ___75-[RCTImageEditingManager cropImage:cropData:successCallback:errorCallback:]_block_invoke in RCTImageEditingManager.o
      RCTResizeImageIfNeeded(UIImage*, CGSize, double, RCTResizeMode) in RCTImageLoader.o
  "_RCTGetImageMetadata", referenced from:
      ___50-[RCTImageLoader getImageSizeForURLRequest:block:]_block_invoke in RCTImageLoader.o
  "_RCTDecodeImageWithData", referenced from:
      ___80-[RCTImageLoader decodeImageData:size:scale:clipped:resizeMode:completionBlock:]_block_invoke_3.264 in RCTImageLoader.o
  "_RCTTransformFromTargetRect", referenced from:
      ___75-[RCTImageEditingManager cropImage:cropData:successCallback:errorCallback:]_block_invoke in RCTImageEditingManager.o
      RCTResizeImageIfNeeded(UIImage*, CGSize, double, RCTResizeMode) in RCTImageLoader.o
  "_RCTTargetRect", referenced from:
      ___75-[RCTImageEditingManager cropImage:cropData:successCallback:errorCallback:]_block_invoke in RCTImageEditingManager.o
      RCTResizeImageIfNeeded(UIImage*, CGSize, double, RCTResizeMode) in RCTImageLoader.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@fkgozali It seems that this problem will continue to occur with CoreModules. What are your thoughts on using s.static_framework = true in the podspecs to resolve it?

[fkgozali]

What are your thoughts on using s.static_framework = true in the podspecs to resolve it?

I think this is fine to do to unblock. I won't have cycle to debug further for the next week or so, so let's do this quick fix.

[grabbou]

I have reverted #25816 on the 0.61-stable branch. However, from what I understand from the linked PR, the issue may still occur within existing apps that depend on React podspecs and have use_frameworks turned on.

I think it makes sense to note this explicitly in the first release candidate and that we are working on providing a better solution to this problem.

[jtreanor]

Thanks @grabbou, you beat me to it.

However, from what I understand from the linked PR, the issue may still occur within existing apps that depend on React podspecs and have use_frameworks turned on.

The issue will likely be for third party libraries which have podspecs. It would now be required to have s.static_framework = true for use_frameworks! to work.

I think it makes sense to note this explicitly in the first release candidate and that we are working on providing a better solution to this problem.

That makes sense.

[jtreanor]

Okay, I have found a simple solution that does not require s.static_framework = true. We just needed to update the CoreModules podspec. I've opened a PR in #26151 to target the 0.61-stable branch.