-
Notifications
You must be signed in to change notification settings - Fork 906
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
fix: doctor for 0.72 Ruby changes #1854
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
import ruby, {output} from '../ruby'; | ||
|
||
// | ||
// Mocks | ||
// | ||
const mockExeca = jest.fn(); | ||
jest.mock('execa', () => mockExeca); | ||
|
||
const mockLogger = jest.fn(); | ||
jest.mock('@react-native-community/cli-tools', () => ({ | ||
findProjectRoot: () => '.', | ||
logger: { | ||
warn: mockLogger, | ||
}, | ||
})); | ||
|
||
jest.mock('../../versionRanges', () => ({ | ||
RUBY: '>= 1.0.0', | ||
})); | ||
|
||
// | ||
// Placeholder Values | ||
// | ||
const Languages = { | ||
Ruby: {version: '1.0.0'}, | ||
}; | ||
|
||
const runRubyGetDiagnostic = () => { | ||
// @ts-ignore | ||
return ruby.getDiagnostics({Languages}); | ||
}; | ||
|
||
const Gemfile = { | ||
noGemfile: {code: 1}, | ||
noRuby: {code: 'ENOENT'}, | ||
ok: {stdout: output.OK}, | ||
unknown: (err: Error) => err, | ||
wrongRuby: (stderr: string) => ({code: 2, stderr}), | ||
}; | ||
|
||
// | ||
// Tests | ||
// | ||
|
||
describe('ruby', () => { | ||
beforeEach(() => { | ||
mockLogger.mockClear(); | ||
mockExeca.mockClear(); | ||
}); | ||
|
||
describe('Gemfile', () => { | ||
it('validates the environment', async () => { | ||
mockExeca.mockResolvedValueOnce(Gemfile.ok); | ||
|
||
expect(await runRubyGetDiagnostic()).toMatchObject({ | ||
needsToBeFixed: false, | ||
}); | ||
}); | ||
|
||
it('fails to find ruby to run the script', async () => { | ||
mockExeca.mockRejectedValueOnce(Gemfile.noRuby); | ||
|
||
const resp = await runRubyGetDiagnostic(); | ||
expect(resp.needsToBeFixed).toEqual(true); | ||
expect(resp.description).toMatch(/Ruby/i); | ||
}); | ||
|
||
it('fails to find the Gemfile and messages the user', async () => { | ||
mockExeca.mockRejectedValueOnce(Gemfile.noGemfile); | ||
|
||
const {description} = await runRubyGetDiagnostic(); | ||
expect(description).toMatch(/could not find/i); | ||
}); | ||
|
||
it('fails because the wrong version of ruby is installed', async () => { | ||
const stderr = '>= 3.2.0, < 3.2.0'; | ||
mockExeca.mockRejectedValueOnce(Gemfile.wrongRuby(stderr)); | ||
|
||
expect(await runRubyGetDiagnostic()).toMatchObject({ | ||
needsToBeFixed: true, | ||
versionRange: stderr, | ||
}); | ||
}); | ||
|
||
it('fails for unknown reasons, so we skip it but log', async () => { | ||
const error = Error('Something bad went wrong'); | ||
mockExeca.mockRejectedValueOnce(Gemfile.unknown(error)); | ||
|
||
await runRubyGetDiagnostic(); | ||
expect(mockLogger).toBeCalledTimes(1); | ||
expect(mockLogger).toBeCalledWith(error.message); | ||
}); | ||
|
||
it('uses are static ruby versions builtin into doctor if no Gemfile', async () => { | ||
mockExeca.mockRejectedValueOnce(new Error('Meh')); | ||
expect(await runRubyGetDiagnostic()).toMatchObject({ | ||
needsToBeFixed: false, | ||
version: Languages.Ruby.version, | ||
versionRange: '>= 1.0.0', | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,19 +1,174 @@ | ||||||
import execa from 'execa'; | ||||||
import chalk from 'chalk'; | ||||||
|
||||||
import {logger, findProjectRoot} from '@react-native-community/cli-tools'; | ||||||
|
||||||
import versionRanges from '../versionRanges'; | ||||||
import {doesSoftwareNeedToBeFixed} from '../checkInstallation'; | ||||||
import {HealthCheckInterface} from '../../types'; | ||||||
import {inline} from './common'; | ||||||
|
||||||
// Exposed for testing only | ||||||
export const output = { | ||||||
OK: 'Ok', | ||||||
NO_GEMFILE: 'No Gemfile', | ||||||
NO_RUBY: 'No Ruby', | ||||||
BUNDLE_INVALID_RUBY: 'Bundle invalid Ruby', | ||||||
UNKNOWN: 'Unknown', | ||||||
} as const; | ||||||
|
||||||
// The Change: | ||||||
// ----------- | ||||||
// | ||||||
// React Native 0.72 primarily defines the compatible version of Ruby in the | ||||||
// project's Gemfile [1]. It does this because it allows for ranges instead of | ||||||
// pinning to a version of Ruby. | ||||||
// | ||||||
// In previous versions the .ruby-version file defined the compatible version, | ||||||
// and it was derived in the Gemfile [2]: | ||||||
// | ||||||
// > ruby File.read(File.join(__dir__, '.ruby-version')).strip | ||||||
// | ||||||
// Why all of the changes with Ruby? | ||||||
// --------------------------------- | ||||||
// | ||||||
// React Native has had to weigh up a couple of concerns: | ||||||
// | ||||||
// - Cocoapods: we don't control the minimum supported version, although that | ||||||
// was defined almost a decade ago [3]. Practically system Ruby on macOS works | ||||||
// for our users. | ||||||
// | ||||||
// - Apple may drop support for scripting language runtimes in future version of | ||||||
// macOS [4]. Ruby 2.7 is effectively EOL, which means many supporting tools and | ||||||
// developer environments _may_ not support it going forward, and 3.0 is becoming | ||||||
// the default in, for example, places like our CI. Some users may be unable to | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
That's not entirely true. CircleCI still supports Ruby 2.7.7... we don't know what will happens in April though. |
||||||
// install Ruby 2.7 on their devices as a matter of policy. | ||||||
// | ||||||
// - Our Codegen is extensively built in Ruby 2.7. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I won't callout Codegen specifically. I'd say that we have some build scripts for iOs that are written in ruby and we used Ruby 2.7 to develop them. |
||||||
// | ||||||
// - A common pain-point for users (old and new) setting up their environment is | ||||||
// configuring a Ruby version manager or managing multiple Ruby versions on their | ||||||
// device. This occurs so frequently that we've removed the step from our docs [6] | ||||||
// | ||||||
// After users suggested bumping Ruby to 3.1.3 [5], a discussion concluded that | ||||||
// allowing a range of version of Ruby (>= 2.6.10) was the best way forward. This | ||||||
// balanced the need to make the platform easier to start with, but unblocked more | ||||||
// sophisticated users. | ||||||
// | ||||||
// [1] https://github.com/facebook/react-native/pull/36281 | ||||||
// [2] https://github.com/facebook/react-native/blob/v0.71.3/Gemfile#L4 | ||||||
// [3] https://github.com/CocoaPods/guides.cocoapods.org/commit/30881800ac2bd431d9c5d7ee74404b13e7f43888 | ||||||
// [4] https://developer.apple.com/documentation/macos-release-notes/macos-catalina-10_15-release-notes#Scripting-Language-Runtimes | ||||||
// [5] https://github.com/facebook/react-native/pull/36074 | ||||||
// [6] https://github.com/facebook/react-native-website/commit/8db97602347a8623f21e3e516245d04bdf6f1a29 | ||||||
|
||||||
async function checkRubyGemfileRequirement( | ||||||
projectRoot: string, | ||||||
): Promise<[string, string?]> { | ||||||
const evaluateGemfile = inline` | ||||||
require "Bundler" | ||||||
gemfile = Bundler::Definition.build("Gemfile", nil, {}) | ||||||
version = gemfile.ruby_version.engine_versions.join(", ") | ||||||
begin | ||||||
gemfile.validate_runtime! | ||||||
rescue Bundler::GemfileNotFound | ||||||
puts "${output.NO_GEMFILE}" | ||||||
exit 1 | ||||||
rescue Bundler::RubyVersionMismatch | ||||||
puts "${output.BUNDLE_INVALID_RUBY}" | ||||||
STDERR.puts version | ||||||
exit 2 | ||||||
rescue => e | ||||||
STDERR e.message | ||||||
exit 3 | ||||||
else | ||||||
puts "${output.OK}" | ||||||
STDERR.puts version | ||||||
end`; | ||||||
|
||||||
try { | ||||||
await execa('ruby', ['-e', evaluateGemfile], { | ||||||
cwd: projectRoot, | ||||||
}); | ||||||
return [output.OK]; | ||||||
} catch (e) { | ||||||
switch (e.code) { | ||||||
case 'ENOENT': | ||||||
return [output.NO_RUBY]; | ||||||
case 1: | ||||||
return [output.NO_GEMFILE]; | ||||||
case 2: | ||||||
return [output.BUNDLE_INVALID_RUBY, e.stderr]; | ||||||
default: | ||||||
return [output.UNKNOWN, e.message]; | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
export default { | ||||||
label: 'Ruby', | ||||||
isRequired: false, | ||||||
description: 'Required for installing iOS dependencies', | ||||||
getDiagnostics: async ({Managers}) => ({ | ||||||
needsToBeFixed: doesSoftwareNeedToBeFixed({ | ||||||
version: Managers.RubyGems.version, | ||||||
getDiagnostics: async ({Languages}) => { | ||||||
let projectRoot; | ||||||
try { | ||||||
projectRoot = findProjectRoot(); | ||||||
} catch (e) { | ||||||
logger.debug(e.message); | ||||||
} | ||||||
|
||||||
const fallbackResult = { | ||||||
needsToBeFixed: doesSoftwareNeedToBeFixed({ | ||||||
version: Languages.Ruby.version, | ||||||
versionRange: versionRanges.RUBY, | ||||||
}), | ||||||
version: Languages.Ruby.version, | ||||||
versionRange: versionRanges.RUBY, | ||||||
}), | ||||||
version: Managers.RubyGems.version, | ||||||
versionRange: versionRanges.RUBY, | ||||||
}), | ||||||
description: '', | ||||||
}; | ||||||
|
||||||
// No guidance from the project, so we make the best guess | ||||||
if (!projectRoot) { | ||||||
return fallbackResult; | ||||||
} | ||||||
|
||||||
// Gemfile | ||||||
let [code, versionOrError] = await checkRubyGemfileRequirement(projectRoot); | ||||||
switch (code) { | ||||||
case output.OK: { | ||||||
return { | ||||||
needsToBeFixed: false, | ||||||
version: Languages.Ruby.version, | ||||||
versionRange: versionOrError, | ||||||
}; | ||||||
} | ||||||
case output.BUNDLE_INVALID_RUBY: | ||||||
return { | ||||||
needsToBeFixed: true, | ||||||
version: Languages.Ruby.version, | ||||||
versionRange: versionOrError, | ||||||
}; | ||||||
case output.NO_RUBY: | ||||||
return { | ||||||
needsToBeFixed: true, | ||||||
description: 'Cannot find a working copy of Ruby.', | ||||||
}; | ||||||
case output.NO_GEMFILE: | ||||||
fallbackResult.description = `Could not find the project ${chalk.bold( | ||||||
'Gemfile', | ||||||
)} in your project folder (${chalk.dim( | ||||||
projectRoot, | ||||||
)}), guessed using my built-in version.`; | ||||||
break; | ||||||
default: | ||||||
if (versionOrError) { | ||||||
logger.warn(versionOrError); | ||||||
} | ||||||
break; | ||||||
} | ||||||
|
||||||
return fallbackResult; | ||||||
}, | ||||||
runAutomaticFix: async ({loader, logManualInstallation}) => { | ||||||
loader.fail(); | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was also discussed in : #1825
Whether we need
RubyGems
in Managers orRuby
in LanguagesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @arushikesarwani94, that thread gives great context: #1818 (comment)
I think this tackles some of the issues raised by focusing on the Gemfile. Interestingly I'm not sure it's useful to consider the .ruby-version at all, even as a backup.
It has given me some more to think about 🧐
Tasks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can but we would not have any guarantee on the version of Cocoapod used by the users. So, if they have an old version of Cocoapods installed in the current Ruby, they will be using that which may be not compatible with React Native.
So, I highly discourage that we don't use Gemfile.