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

[RNMobile] Upgrade CI to use Xcode 12 #26283

Merged
merged 17 commits into from
Oct 28, 2020
Merged

Conversation

ceyhun
Copy link
Member

@ceyhun ceyhun commented Oct 19, 2020

Description

Fixes #26188

How has this been tested?

  • Check if all GitHub Actions workflows are green.
  • Run e2e tests locally using Xcode 12:
    1. Make sure command line is using Xcode 12: sudo xcode-select --switch /Applications/Xcode_12_path.app
    2. npm run native test:e2e:ios:local

@ceyhun ceyhun added [Type] Build Tooling Issues or PRs related to build tooling Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) labels Oct 19, 2020
@github-actions
Copy link

github-actions bot commented Oct 19, 2020

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/api-fetch/index.js 3.45 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.94 kB 0 B
build/block-library/editor.css 8.94 kB 0 B
build/block-library/index.js 146 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 837 B 0 B
build/block-library/theme.css 838 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.81 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 772 B 0 B
build/data/index.js 8.77 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.46 kB 0 B
build/edit-navigation/index.js 11.2 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.37 kB 0 B
build/edit-post/style.css 6.36 kB 0 B
build/edit-site/index.js 22 kB 0 B
build/edit-site/style-rtl.css 3.84 kB 0 B
build/edit-site/style.css 3.83 kB 0 B
build/edit-widgets/index.js 26.4 kB 0 B
build/edit-widgets/style-rtl.css 3.09 kB 0 B
build/edit-widgets/style.css 3.09 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 43.1 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 7.7 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13.2 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@ceyhun ceyhun requested a review from guarani October 20, 2020 14:46
@ceyhun ceyhun marked this pull request as ready for review October 20, 2020 14:46
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When running the e2e tests locally I get:

error Failed to build iOS project. We ran "xcodebuild" command but it exited with error code 65. To debug build logs further, consider building your app with Xcode.app, by opening GutenbergDemo.xcworkspace. Run CLI with --verbose flag for more details.

Which could be issue #25882, but which was fixed in PR #26062 (and worked for me previously with this fix).

I tried running npm run postinstall as suggested in the above mentioned PR, but same result.
Then I switched back to the master branch and saw the same issue there as well when running npm run native test:e2e:ios:local. Notably, running npm run native ios on master succeeded, but didn't fire up the packager as it used to (running npm run native start in one window and npm run native ios in another window worked, and it's how I've been running Gutenberg for the last few months.

I tried:

  1. Deleting ~/.cocoapods and cd packages/react-native-editor/ios && bundle exec pod install
  2. cd packages/react-native-editor/ios && rm -rf Pods, deleted Xcode derived data, and reran npm run native test:e2e:ios:local in the Gutenberg root folder

No luck. So I'll have to come back tomorrow for some more digging, but feel free to add another dev here to get a second look at this PR since this looks to be an issue with my machine, not this PR.

@ceyhun
Copy link
Member Author

ceyhun commented Oct 22, 2020

I think I reproduced the issue you were having. Was it maybe this one:

Can't find the 'node' binary to build the React Native bundle.  If you have a non-standard Node.js installation, select your project in Xcode, find  'Build Phases' - 'Bundle React Native code and images' and change NODE_BINARY to an  absolute path to your node executable. You can find it by invoking 'which node' in the terminal.

Strange that this happens only when using the react-native run-ios command to build and not when running from inside Xcode (even when using FORCE_BUNDLING flag) or when using xcodebuild directly.

I updated the scripts to use xcodebuild directly and also removed some duplicate ones that we won't need.

For the metro server not launching on build: The Build Phases in the Xcode project doesn't include the phase to launch metro anymore, this I think was on purpose because the metro server needs to run from different roots depending on which gutenberg version is being executed (gutenberg or gutenberg-mobile one) and I'm not sure how to achieve that with the script living in the Xcode project.

@ceyhun ceyhun requested a review from guarani October 22, 2020 15:57
@ceyhun ceyhun mentioned this pull request Oct 23, 2020
6 tasks
@guarani
Copy link
Contributor

guarani commented Oct 26, 2020

I think I reproduced the issue you were having. Was it maybe this one:

Can't find the 'node' binary to build the React Native bundle.  If you have a non-standard Node.js installation, select your project in Xcode, find  'Build Phases' - 'Bundle React Native code and images' and change NODE_BINARY to an  absolute path to your node executable. You can find it by invoking 'which node' in the terminal.

I don't see that message in the output of npm run native test:e2e:ios:local, so it looks like it's a different issue.

For the metro server not launching on build: The Build Phases in the Xcode project doesn't include the phase to launch metro anymore, this I think was on purpose because the metro server needs to run from different roots depending on which gutenberg version is being executed (gutenberg or gutenberg-mobile one) and I'm not sure how to achieve that with the script living in the Xcode project.

I see — I've been running the packager separately anyway, but good to know the reason why this changed.


I'm upgrading from Xcode 12.0 to Xcode 12.1 to see if that fixes the issue I'm having, since I'm still getting the same from #26283 (review) when using the latest commit from this branch.

@ceyhun
Copy link
Member Author

ceyhun commented Oct 27, 2020

I don't see that message in the output of npm run native test:e2e:ios:local, so it looks like it's a different issue.

@guarani Can you maybe share more details about the error, maybe the full output if that's possible?

@etoledom Can you please give this a try on your machine to see if you're having issues too?

@etoledom
Copy link
Contributor

etoledom commented Oct 27, 2020

My first try with npm run native test:e2e:ios:local resulted on a error code 65 (even though the description was different)

Second try:

  • git clean -fdx
  • Xcode Clean Build Folder
  • npm i
  • npm run native test:e2e:ios:local

And Build Success! 🎉
But all tests failed 😱

Third try:

  • I noticed that now the iOS version for local is 14.0, I have Xcode 12.1 that comes with the 14.1 sims, so I changed the platformVersion: '14.0' def to platformVersion: '14.1' and now everything works! 🎉 🎉
  • I could have alternatively downloaded the 14.0 sims, it should work this way too.

One thing, after doing the second try, I got a huge diff on package-lock.json. This could also be a difference on node version (or something like that).
Just to be sure:

$ node --version
v12.18.4

@ceyhun
Copy link
Member Author

ceyhun commented Oct 27, 2020

Thanks for checking @etoledom, awesome that it worked 🎉

One thing, after doing the second try, I got a huge diff on package-lock.json. This could also be a difference on node version (or something like that).

I couldn't reproduce this. I have node v12.19.0 and npm 6.14.8. There's a gutenberg-mobile PR too: wordpress-mobile/gutenberg-mobile#2744 and there the CI didn't find any issues with package-lock.json changes as well.

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local E2E tests are ✅ on my machine and CI seems to be happy too 🎉
Nice job @ceyhun 🙏

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It worked after following the tips from #26283 (comment) (I guess my mistake was only removing the Pods folder instead of git clean -fdx to remove all untracked files).
Still on Xcode 12.0 (not 12.1), all tests passed locally! 🎆

@ceyhun
Copy link
Member Author

ceyhun commented Oct 27, 2020

That's great, thanks @guarani 🎉 Can you please check the gb-mobile PR too: wordpress-mobile/gutenberg-mobile#2744 and if there aren't any errors, I'll go ahead and merge these two!

@ceyhun
Copy link
Member Author

ceyhun commented Oct 28, 2020

Merging this despite failing End-to-End Tests / Admin - 1 as it's failing on current master as well.

@ceyhun ceyhun merged commit 148e2b2 into master Oct 28, 2020
@ceyhun ceyhun deleted the rnmobile/upgrade-ci-to-xcode-12 branch October 28, 2020 14:13
@github-actions github-actions bot added this to the Gutenberg 9.3 milestone Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI config to use Xcode 12
3 participants