-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Introduce Gemfile, ruby-version #32303
Conversation
Base commit: 34b7d22 |
Looks like the build failures may be related?
|
I'll take a look tomorrow, maybe change to
and both files were added in the commit, but I need to see if stuff was copied, maybe I forgot to add those files. |
79c383a
to
bafdc8e
Compare
@fkgozali do you have more knowledge on this?
is the |
also, is it linux or macOS? Because |
Hmm it should be the checkout. I'm actually not sure here, you might need to add a bit of more logs (echo?) to debug further. The iOS tests are always running on MacOS, e.g. https://app.circleci.com/pipelines/github/facebook/react-native/10546/workflows/27b085f4-3142-4453-9125-cbbaea93d3e0/jobs/219844 |
bafdc8e
to
b93de20
Compare
added some more safety, just updates from macOS... I think that's the Since the Ruby part is only interesting for macOS, I'm ignoring updates from other OS. |
@fkgozali seems that one is fixed, but still some errors. Could you check what are they about? test_ios_unit_jsc - yarn test-ios and test_ios_unit_hermes both fail in a similar
Others looks unrelated:
|
These are unrelated, it was due to Xcode 13 upgrade, but we're not sure why yet. |
some google says it's related to build order, I'll take a look if I can fix that in another PR. As for the current approach regarding ruby-version + Gemfile, let me know what you guys think about that. |
Re: build order, sounds good. FYI it started failing in this commit: https://app.circleci.com/pipelines/github/facebook/react-native/10525/workflows/b68b10a3-325a-4892-8252-baed9017c613 For the rest of the solution, we'll take a look tomorrow. |
also cc @mikehardy for inputs |
The other error should be fixed by #32309 |
Might I suggest also adding a bundler config to tell bundler to install the dependencies locally in the project? Again, to keep things independent of global installed deps:
And then add |
I'm not good enough with ruby to offer much, but I do worry if it doesn't work on Linux for android only template init build tests (I do this sometimes) or if there's no way to override (I use ruby 3.0.2 personally). Those are feature specification hopes not code comments though. I worry a little about future issue traffic from the ruby ignorant (like myself) about all things related to this, I fear it opens a support surface area for ruby distribution, upgrades, gem files etc), and I'm curious if it will be balanced by a decrease in issue count around incorrect ruby versions, or incorrect pod versions. |
It will work on Android, but in android there is no need to As for use another version of Ruby, you definitely can, in particularly if you create a new repo ( We're sticking to the "latest stable", and it's the same as CircleCI (and likely other CI) uses. |
Those are certainly reasonable responses. My comments were all non-quantitative, they were qualitative, so any reasonable defense can be thought of as sufficient I think. Works for me and we'll see I think :-) |
b93de20
to
0b171d5
Compare
@mikehardy the new version uses the @liamjones it also uses However, I noticed something: the |
0b171d5
to
b344fb5
Compare
exactly! :-D |
@barbieri @mikehardy thanks for the inputs. To be honest, I have little context on Ruby/Gemfile setup, but I did find myself struggling to make sure my env was correct, usually around random Ruby failures. So from that angle alone, IMO "any signals to indicate that something is misconfigured in your system" is very valuable. I understand the potential risks of us getting asked non-React Native questions like Ruby stuffs, but perhaps it's gonna be just in the short term (we'll see). So I'll import this PR and get some more inputs from our team at FB, and if there's no concern, we'll merge it. I see this as an incremental improvement though, so we can always tweak things as needed and make things more robust. |
b344fb5
to
3e0522f
Compare
Nice work @barbieri 👍 I do have one question though (Please correct me if I'm wrong): These changes are primarily for working with the react-native repo itself (e.g. running e2e tests, CI, etc), correct? There already is a clearly written section about the requirement to install CocoaPods in the official React Native environment setup guide (cc @thymikee). I'm quoting one section:
To me it sounds like everything should still work for someone who set up their environment following the guide. I don't have an M1 mac available to test right now (in react-native-community/discussions-and-proposals#411 you mentioned "ffi on arm64/m1") - Are there any issues with it and the previous template? |
I'll take a look what's the exact problem, but I know I need to modify the |
Yes, they work (at least with the CI and @fkgozali internal tests as well. The idea of
That doesn't state the recommended version, more than that, it will install global in the system, which may cause issues with people working in multiple projects (common if you have to do maintenance in one project while creating a new one).
It seems it will still work, the
To make sure I cleaned everything and did a new setup: there is no real problem, seems ruby 2.6.3 DOES WORK with What happened (at least to me and some colleagues) is that many tutorials to work around M1 limitations did recommend to execute the processes in Rosetta environment. If you did that, the files may be of an incorrect architecture, ex:
If any of the EDIT-1: listed both arm64 and x86_64 output to avoid confusion |
The problem for FB internal was the command used to move |
Any reason |
hum... I forgot, nice catch :-) |
@kelset what branch should the PR be pointed at? |
main, and then we cherry pick to 0.67 branch |
Now that this is out and live, it immediately causes me a problem:
I'm running current stable version of ruby, and I try to force everything to use that, so I do not have, and do not want ruby-2.7.4 on my system. I understand the desire for react-native to have a known version to work with, but having it force the issue at a step where I cannot intercept it (during For now I have to install the old ruby just to run scripted compile tests that start with react-native init I guess But is there a way to make this optional? Or make it accept any version above the minimum, like >=2.7.4 ? |
I can use Having the |
maybe doesn't help due the lockfile, but since you're running it, have a try if that changes ( |
Summary: Ref #32303 (comment) Recently, Gemfile & .ruby-version have been introduced but local vendor artifacts have not been correctly added into gitignore, see #32303 ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [General] [Added] - Add `vendor/bundle` into .gitignore template Pull Request resolved: #32930 Test Plan: N / A Reviewed By: cortinico Differential Revision: D33688116 Pulled By: yungsters fbshipit-source-id: 787b2b90da1e9f32f0a9e754741affbda443b3e8
…as the official RN 0.67 template reference: facebook/react-native#32303
…as the official RN 0.67 template reference: facebook/react-native#32303
I am facing trouble due to this change, below are my doubts.
|
@ravirajn22 if you install rbenv or rvm that can use the .ruby-version file to ensure the required version of ruby is available for the project without affecting your system version |
Summary: For the same reason we don't keep a yarn.lock or Podfile.lock, we shouldn't be keeping a Gemfile.lock in the template. The user will generate this on his own pulling in the current dependencies with the constraints in Gemfile. No need to lock to a specific version. cc barbieri (author of #32303) cc ravirajn22 (for raising the issue) ## Changelog [iOS] [Fixed] - Remove Gemfile.lock from template Pull Request resolved: #33469 Test Plan: no test plan Reviewed By: javache Differential Revision: D35074105 Pulled By: cortinico fbshipit-source-id: 47d1b92329f1d55d4a0adbacbc7e5e45f9d957e0
I want to try the New Architecture
But
Why? How to use ruby 2.7.5 here? |
@Bardiamist I don't know if it helps, but what happens if you try running |
Ok, thanks, understand more now. Yes But idea was to run |
I have to admit I still dislike the whole bundle / Gemfile / .rubyversion thing. I do this every time now: npm_config_yes=true npx @react-native-community/cli init rnfbdemo --skip-install --version=0.69.4
cd rnfbdemo
# New versions of react-native include annoying Ruby stuff that forces use of old rubies. Obliterate.
if [ -f Gemfile ]; then
rm -f Gemfile* .ruby*
fi
# Now run our initial dependency install
yarn |
Summary: For the same reason we don't keep a yarn.lock or Podfile.lock, we shouldn't be keeping a Gemfile.lock in the template. The user will generate this on his own pulling in the current dependencies with the constraints in Gemfile. No need to lock to a specific version. cc barbieri (author of facebook#32303) cc ravirajn22 (for raising the issue) [iOS] [Fixed] - Remove Gemfile.lock from template Pull Request resolved: facebook#33469 Test Plan: no test plan Reviewed By: javache Differential Revision: D35074105 Pulled By: cortinico fbshipit-source-id: 47d1b92329f1d55d4a0adbacbc7e5e45f9d957e0
Summary
Implement par of the discussion react-native-community/discussions-and-proposals#411, except the
.nvmrc
part, this includes:.ruby-version
in the main project and alsotemplate/
Gemfile
and alsotemplate/Gemfile
pod
executions frombundle exec pod
, using the determined versionChangelog
[iOS] [Added] - Gemfile with CocoaPods 1.11 and ruby-version (2.7.4)
Test Plan
Build for iOS and run all CircleCI tests to see if nothing changed