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

DEBUGGING: Bump Ruby to 3.1.3 #36205

Closed
wants to merge 25 commits into from
Closed

Conversation

traviswimer
Copy link

@traviswimer traviswimer commented Feb 18, 2023

Summary

This was forked from another PR to bump Ruby to version 3.1.3: #36074

I'm only making this PR for debugging purposes, so that CircleCI will run the tests with my changes.

Changelog

[GENERAL] [CHANGED] - Updates CircleCI config to install and use Ruby 3.1.3

Test Plan

See if CircleCI tests pass.

@facebook-github-bot
Copy link
Contributor

Hi @traviswimer!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@analysis-bot
Copy link

analysis-bot commented Feb 18, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,451,960 -67
android hermes armeabi-v7a 7,775,340 -61
android hermes x86 8,927,563 -71
android hermes x86_64 8,785,165 -76
android jsc arm64-v8a 6,668,753 +102
android jsc armeabi-v7a 7,462,720 +103
android jsc x86 9,192,880 +111
android jsc x86_64 6,893,882 +102

Base commit: 23eb380
Branch: main

@react-native-bot react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Feb 19, 2023
@cortinico
Copy link
Contributor

I'm only making this PR for debugging purposes, so that CircleCI will run the tests with my changes.

Converting this to draft

@cortinico cortinico marked this pull request as draft February 20, 2023 12:11
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2023
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@react-native-bot react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2023
@@ -593,11 +601,7 @@ jobs:
- run:
name: "Brew: Tap wix/brew"
command: brew tap wix/brew
- run:
Copy link
Author

Choose a reason for hiding this comment

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

This was just added in a PR (#36192), but it seems the python installation is no longer causing CI tests to break. Instead, this brew unlink command was throwing an error.

@traviswimer
Copy link
Author

Basically, the problems I fixed to make Ruby 3.1.3 work are:

  • bundle command was still using the wrong version of Ruby. Reinstalling the bundler gem fixes this.
  • test_ios_rntester and prepare_package_for_release were failing because the pods were installed for the wrong version of Ruby. I added commands to install them within these jobs.

Now the tests are all passing.

test_ios_rntester and prepare_package_for_release must have previously been able to retrieve cached pods somehow, so maybe there is a better fix. Although, it looks like the CI workflows are still taking about the same amount of time to complete, so I'm not sure if it's worth worrying about.

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Changes looks good to me, thank you for taking care of the bump!

@cipolleschi cipolleschi marked this pull request as ready for review February 21, 2023 17:00
@cipolleschi
Copy link
Contributor

@traviswimer could you condense the changelog lines into a single one? I think that the changelog tool could have an hard time in handling three different [GENERAL] [CHANGED] lines. Thank you!

@traviswimer
Copy link
Author

@cipolleschi Sure, I updated it.

@cipolleschi
Copy link
Contributor

Thank you so much!

@facebook-github-bot
Copy link
Contributor

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

@cipolleschi
Copy link
Contributor

Hi @traviswimer. Thank you so much for this PR: it was extremely helpful and instrumental to kickstart an internal discussion with the React org about the DevX we want to provide related to Ruby.

Currently, we are enforcing a specific version of Ruby which push all our users to either update their current version of Ruby or install a Ruby version manager. This could be challenging, especially for new users that approaches React Native for the first time and for users that comes from a Web background and have no platform-specific knowledge.

After considering this use cases, we decided to adopt a less strict approach. From now on, React Native will try to support a wider range of Ruby versions, from the system Ruby (2.6.10) to the most recent one (although Cocoapods is not working right now with Ruby 3.2.0 due to a bug in their code... :( ). This should simplify the first experience for everyone, speeding up also the system setup.

Due to these consideration, we won't be able to merge this PR. However, thank you so much for the work you put into it and I hope you will still considering contributing to the project as we think that your contributions could help the framework improve.

Thank you so much! 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants