From f12b12c999fb36e66bcef242f7cf1f0751afedcd Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Mon, 21 Nov 2022 10:52:17 -0800 Subject: [PATCH] Improve version checks to avoid mistakes in the versioning (#35296) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35296 This change adds some version checks and enforces that every version matches some specific format based on the build type we are trying to run. ## Changelog [General][Changed] - Improve version checks Reviewed By: cortinico Differential Revision: D41161756 fbshipit-source-id: 4172195c5e031c1eaf7b33bb74f381c04e9adaf5 --- .circleci/config.yml | 11 +- scripts/__tests__/version-utils-test.js | 275 ++++++++++++++++++++++-- scripts/bump-oss-version.js | 2 +- scripts/prepare-package-for-release.js | 14 +- scripts/publish-npm.js | 15 +- scripts/set-rn-version.js | 46 ++-- scripts/test-e2e-local.js | 11 +- scripts/version-utils.js | 139 +++++++++++- 8 files changed, 455 insertions(+), 58 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ea68661ca20478..12264e6fd30e7f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1363,7 +1363,14 @@ jobs: - run: name: "Set new react-native version and commit changes" command: | - node ./scripts/prepare-package-for-release.js -v << parameters.version >> -l << parameters.latest >> --dry-run << parameters.dryrun >> + VERSION=<< parameters.version >> + + if [[ -z "$VERSION" ]]; then + VERSION=$(grep '"version"' package.json | cut -d '"' -f 4 | head -1) + echo "Using the version from the package.json: $VERSION" + fi + + node ./scripts/prepare-package-for-release.js -v "$VERSION" -l << parameters.latest >> --dry-run << parameters.dryrun >> build_npm_package: parameters: @@ -1643,7 +1650,7 @@ workflows: jobs: - prepare_package_for_release: name: prepare_package_for_release - version: 'v1000.0.1' + version: '' latest : false dryrun: true - prepare_hermes_workspace: diff --git a/scripts/__tests__/version-utils-test.js b/scripts/__tests__/version-utils-test.js index 19450483399d06..0dd087e20b01ba 100644 --- a/scripts/__tests__/version-utils-test.js +++ b/scripts/__tests__/version-utils-test.js @@ -7,7 +7,11 @@ * @format */ -const {parseVersion, isReleaseBranch} = require('../version-utils'); +const { + parseVersion, + isReleaseBranch, + validateBuildType, +} = require('../version-utils'); let execResult = null; jest.mock('shelljs', () => ({ @@ -38,18 +42,53 @@ describe('version-utils', () => { }); describe('parseVersion', () => { - it('should throw error if invalid match', () => { + it('should throw error if buildType is undefined', () => { function testInvalidVersion() { - parseVersion(''); + parseVersion('v0.10.5'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"Unsupported build type: undefined"`, + ); + }); + + it('should throw error if buildType is not `release`, `dry-run` or `nightly`', () => { + function testInvalidVersion() { + parseVersion('v0.10.5', 'invalid_build_type'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"Unsupported build type: invalid_build_type"`, + ); + }); + it('should throw error if invalid match with release', () => { + function testInvalidVersion() { + parseVersion('', 'release'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"You must pass a correctly formatted version; couldn't parse "`, + ); + }); + it('should throw error if invalid match with dry-run', () => { + function testInvalidVersion() { + parseVersion('', 'dry-run'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"You must pass a correctly formatted version; couldn't parse "`, + ); + }); + it('should throw error if invalid match with nightly', () => { + function testInvalidVersion() { + parseVersion('', 'nightly'); } expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( `"You must pass a correctly formatted version; couldn't parse "`, ); }); - it('should parse pre-release version with .', () => { - const {version, major, minor, patch, prerelease} = - parseVersion('0.66.0-rc.4'); + it('should parse pre-release version with release and `.`', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.66.0-rc.4', + 'release', + ); expect(version).toBe('0.66.0-rc.4'); expect(major).toBe('0'); expect(minor).toBe('66'); @@ -57,9 +96,11 @@ describe('version-utils', () => { expect(prerelease).toBe('rc.4'); }); - it('should parse pre-release version with -', () => { - const {version, major, minor, patch, prerelease} = - parseVersion('0.66.0-rc-4'); + it('should parse pre-release version with release and `-`', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.66.0-rc-4', + 'release', + ); expect(version).toBe('0.66.0-rc-4'); expect(major).toBe('0'); expect(minor).toBe('66'); @@ -67,8 +108,20 @@ describe('version-utils', () => { expect(prerelease).toBe('rc-4'); }); + it('should reject pre-release version with random prerelease pattern', () => { + function testInvalidVersion() { + parseVersion('0.66.0-something_invalid', 'release'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"Version 0.66.0-something_invalid is not valid for Release"`, + ); + }); + it('should parse stable version', () => { - const {version, major, minor, patch, prerelease} = parseVersion('0.66.0'); + const {version, major, minor, patch, prerelease} = parseVersion( + '0.66.0', + 'release', + ); expect(version).toBe('0.66.0'); expect(major).toBe('0'); expect(minor).toBe('66'); @@ -77,18 +130,40 @@ describe('version-utils', () => { }); it('should parse pre-release version from tag', () => { - const {version, major, minor, patch, prerelease} = - parseVersion('v0.66.1-rc.4'); - expect(version).toBe('0.66.1-rc.4'); + const {version, major, minor, patch, prerelease} = parseVersion( + 'v0.66.0-rc.4', + 'release', + ); + expect(version).toBe('0.66.0-rc.4'); expect(major).toBe('0'); expect(minor).toBe('66'); - expect(patch).toBe('1'); + expect(patch).toBe('0'); expect(prerelease).toBe('rc.4'); }); + it('should reject pre-release version with patch != 0', () => { + function testInvalidVersion() { + parseVersion('0.66.3-rc.4', 'release'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"Version 0.66.3-rc.4 is not valid for Release"`, + ); + }); + + it('should reject pre-release version from tag with random prerelease pattern', () => { + function testInvalidVersion() { + parseVersion('v0.66.0-something_invalid', 'release'); + } + expect(testInvalidVersion).toThrowErrorMatchingInlineSnapshot( + `"Version 0.66.0-something_invalid is not valid for Release"`, + ); + }); + it('should parse stable version from tag', () => { - const {version, major, minor, patch, prerelease} = - parseVersion('v0.66.0'); + const {version, major, minor, patch, prerelease} = parseVersion( + 'v0.66.0', + 'release', + ); expect(version).toBe('0.66.0'); expect(major).toBe('0'); expect(minor).toBe('66'); @@ -96,23 +171,179 @@ describe('version-utils', () => { expect(prerelease).toBeUndefined(); }); - it('should parse nightly fake version', () => { - const {version, major, minor, patch, prerelease} = parseVersion('0.0.0'); - expect(version).toBe('0.0.0'); + it('should reject nightly with no prerelease', () => { + // this should fail + function testInvalidFunction() { + parseVersion('0.0.0', 'nightly'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 0.0.0 is not valid for nightlies"`, + ); + }); + + it('should reject nightly with prerelease but wrong version numbers', () => { + // this should fail + function testInvalidFunction() { + parseVersion('1.2.3-pre-release', 'nightly'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 1.2.3-pre-release is not valid for nightlies"`, + ); + }); + + it('should parse nightly with 0.0.0 and a prerelease part', () => { + // this should fail + const {version, major, minor, patch, prerelease} = parseVersion( + '0.0.0-pre-release', + 'nightly', + ); + + expect(version).toBe('0.0.0-pre-release'); expect(major).toBe('0'); expect(minor).toBe('0'); expect(patch).toBe('0'); + expect(prerelease).toBe('pre-release'); + }); + it('should parse dryrun with release version', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.7.3', + 'dry-run', + ); + expect(version).toBe('0.7.3'); + expect(major).toBe('0'); + expect(minor).toBe('7'); + expect(patch).toBe('3'); expect(prerelease).toBeUndefined(); }); - it('should parse dryrun fake version', () => { - const {version, major, minor, patch, prerelease} = - parseVersion('1000.0.0'); + it('should parse dryrun with prerelease . version', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.20.0-rc.0', + 'dry-run', + ); + expect(version).toBe('0.20.0-rc.0'); + expect(major).toBe('0'); + expect(minor).toBe('20'); + expect(patch).toBe('0'); + expect(prerelease).toBe('rc.0'); + }); + + it('should reject dryrun with prerelease . version with patch different from 0', () => { + function testInvalidFunction() { + parseVersion('0.20.3-rc.0', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 0.20.3-rc.0 is not valid for dry-runs"`, + ); + }); + + it('should parse dryrun with prerelease - version', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.20.0-rc-0', + 'dry-run', + ); + expect(version).toBe('0.20.0-rc-0'); + expect(major).toBe('0'); + expect(minor).toBe('20'); + expect(patch).toBe('0'); + expect(prerelease).toBe('rc-0'); + }); + + it('should reject dryrun with prerelease - version with patch different from 0', () => { + function testInvalidFunction() { + parseVersion('0.20.3-rc-0', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 0.20.3-rc-0 is not valid for dry-runs"`, + ); + }); + + it('should parse dryrun with main version', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '1000.0.0', + 'dry-run', + ); expect(version).toBe('1000.0.0'); expect(major).toBe('1000'); expect(minor).toBe('0'); expect(patch).toBe('0'); expect(prerelease).toBeUndefined(); }); + + it('should fail for dryrun with v1000.0.1 version', () => { + function testInvalidFunction() { + parseVersion('v1000.0.1', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 1000.0.1 is not valid for dry-runs"`, + ); + }); + it('should parse dryrun with nightly version', () => { + const {version, major, minor, patch, prerelease} = parseVersion( + '0.0.0-something-else', + 'dry-run', + ); + expect(version).toBe('0.0.0-something-else'); + expect(major).toBe('0'); + expect(minor).toBe('0'); + expect(patch).toBe('0'); + expect(prerelease).toBe('something-else'); + }); + + it('should reject dryrun invalid values', () => { + function testInvalidFunction() { + parseVersion('1000.0.4', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 1000.0.4 is not valid for dry-runs"`, + ); + }); + + it('should reject dryrun for invalid prerelease', () => { + function testInvalidFunction() { + parseVersion('0.6.4-something-else', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 0.6.4-something-else is not valid for dry-runs"`, + ); + }); + + it('should reject dryrun for nightlies with invalid prerelease', () => { + function testInvalidFunction() { + parseVersion('0.0.0', 'dry-run'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Version 0.0.0 is not valid for dry-runs"`, + ); + }); + }); + + describe('Validate version', () => { + it('Throw error if the buildType is unknown', () => { + function testInvalidFunction() { + validateBuildType('wrong_build'); + } + expect(testInvalidFunction).toThrowErrorMatchingInlineSnapshot( + `"Unsupported build type: wrong_build"`, + ); + }); + it('Does not throw if the buildType is release', () => { + function testValidCall() { + validateBuildType('release'); + } + expect(testValidCall).not.toThrowError(); + }); + it('Does not throw if the buildType is nightly', () => { + function testValidCall() { + validateBuildType('nightly'); + } + expect(testValidCall).not.toThrowError(); + }); + it('Does not throw if the buildType is dry-run', () => { + function testValidCall() { + validateBuildType('dry-run'); + } + expect(testValidCall).not.toThrowError(); + }); }); }); diff --git a/scripts/bump-oss-version.js b/scripts/bump-oss-version.js index b76f161ad874cd..21bf2d71043ade 100755 --- a/scripts/bump-oss-version.js +++ b/scripts/bump-oss-version.js @@ -91,7 +91,7 @@ async function main() { } let latest = false; - const {version, prerelease} = parseVersion(releaseVersion); + const {version, prerelease} = parseVersion(releaseVersion, 'release'); if (!prerelease) { const {setLatest} = await inquirer.prompt({ type: 'confirm', diff --git a/scripts/prepare-package-for-release.js b/scripts/prepare-package-for-release.js index 2191e8eb026acb..05d72be149f5dd 100755 --- a/scripts/prepare-package-for-release.js +++ b/scripts/prepare-package-for-release.js @@ -60,13 +60,23 @@ if (branch && !isReleaseBranch(branch) && !isDryRun) { exit(1); } -const {version} = parseVersion(releaseVersion); +const buildType = isDryRun + ? 'dry-run' + : isReleaseBranch(branch) + ? 'release' + : 'nightly'; + +const {version} = parseVersion(releaseVersion, buildType); if (version == null) { console.error(`Invalid version provided: ${releaseVersion}`); exit(1); } -if (exec(`node scripts/set-rn-version.js --to-version ${version}`).code) { +if ( + exec( + `node scripts/set-rn-version.js --to-version ${version} --build-type ${buildType}`, + ).code +) { echo(`Failed to set React Native version to ${version}`); exit(1); } diff --git a/scripts/publish-npm.js b/scripts/publish-npm.js index 4f2939b47928fd..1ee22e34b3b54e 100755 --- a/scripts/publish-npm.js +++ b/scripts/publish-npm.js @@ -65,15 +65,22 @@ const argv = yargs default: false, }) .option('r', { - alias: 'release', // useless but needed for CI + alias: 'release', type: 'boolean', default: false, }) .strict().argv; const nightlyBuild = argv.nightly; const dryRunBuild = argv.dryRun; +const releaseBuild = argv.release; const isCommitly = nightlyBuild || dryRunBuild; +const buildType = releaseBuild + ? 'release' + : nightlyBuild + ? 'nightly' + : 'dry-run'; + if (!argv.help) { echo(`The temp publishing folder is ${tmpPublishingFolder}`); } @@ -97,7 +104,7 @@ let version, minor, prerelease = null; try { - ({version, major, minor, prerelease} = parseVersion(rawVersion)); + ({version, major, minor, prerelease} = parseVersion(rawVersion, buildType)); } catch (e) { echo(e.message); exit(1); @@ -122,7 +129,9 @@ if (dryRunBuild) { // For stable, pre-release releases, we rely on CircleCI job `prepare_package_for_release` to handle this if (isCommitly) { if ( - exec(`node scripts/set-rn-version.js --to-version ${releaseVersion}`).code + exec( + `node scripts/set-rn-version.js --to-version ${releaseVersion} --build-type ${buildType}`, + ).code ) { echo(`Failed to set version number to ${releaseVersion}`); exit(1); diff --git a/scripts/set-rn-version.js b/scripts/set-rn-version.js index 23705ed53fc194..44f9ae4764a47a 100755 --- a/scripts/set-rn-version.js +++ b/scripts/set-rn-version.js @@ -21,37 +21,41 @@ const os = require('os'); const path = require('path'); const {cat, echo, exec, exit, sed} = require('shelljs'); const yargs = require('yargs'); -const {parseVersion} = require('./version-utils'); +const {parseVersion, validateBuildType} = require('./version-utils'); const {saveFiles} = require('./scm-utils'); -const tmpVersioningFolder = fs.mkdtempSync( - path.join(os.tmpdir(), 'rn-set-version'), -); -echo(`The temp versioning folder is ${tmpVersioningFolder}`); - -let argv = yargs.option('v', { - alias: 'to-version', - type: 'string', -}).argv; - +let argv = yargs + .option('v', { + alias: 'to-version', + type: 'string', + required: true, + }) + .option('b', { + alias: 'build-type', + type: 'string', + required: true, + }).argv; + +const buildType = argv.buildType; const version = argv.toVersion; - -if (!version) { - echo('You must specify a version using -v'); - exit(1); -} +validateBuildType(buildType); let major, minor, patch, prerelease = -1; try { - ({major, minor, patch, prerelease} = parseVersion(version)); + ({major, minor, patch, prerelease} = parseVersion(version, buildType)); } catch (e) { echo(e.message); exit(1); } +const tmpVersioningFolder = fs.mkdtempSync( + path.join(os.tmpdir(), 'rn-set-version'), +); +echo(`The temp versioning folder is ${tmpVersioningFolder}`); + saveFiles(['package.json', 'template/package.json'], tmpVersioningFolder); fs.writeFileSync( @@ -161,13 +165,17 @@ const numberOfChangedLinesWithNewVersion = exec( ).stdout.trim(); if (+numberOfChangedLinesWithNewVersion !== filesToValidate.length) { + // TODO: the logic that checks whether all the changes have been applied + // is missing several files. For example, it is not checking Ruby version nor that + // the Objecive-C files, the codegen and other files are properly updated. + // We are going to work on this in another PR. + echo('WARNING:'); echo( `Failed to update all the files: [${filesToValidate.join( ', ', )}] must have versions in them`, ); - echo('Fix the issue and try again'); - exit(1); + echo(`These files already had version ${version} set.`); } exit(0); diff --git a/scripts/test-e2e-local.js b/scripts/test-e2e-local.js index b4c2175d4afea0..09b095d9ad59b0 100644 --- a/scripts/test-e2e-local.js +++ b/scripts/test-e2e-local.js @@ -21,6 +21,7 @@ const yargs = require('yargs'); const fs = require('fs'); const path = require('path'); const os = require('os'); +const {getBranchName} = require('./scm-utils'); const { launchAndroidEmulator, @@ -155,6 +156,12 @@ if (argv.target === 'RNTester') { // we need to add the unique timestamp to avoid npm/yarn to use some local caches const baseVersion = require('../package.json').version; + const branchName = getBranchName(); + const buildType = + branchName.endsWith('-stable') && baseVersion !== '1000.0.0' + ? 'release' + : 'dry-run'; + const dateIdentifier = new Date() .toISOString() .slice(0, -8) @@ -164,7 +171,9 @@ if (argv.target === 'RNTester') { const releaseVersion = `${baseVersion}-${dateIdentifier}`; // this is needed to generate the Android artifacts correctly - exec(`node scripts/set-rn-version.js --to-version ${releaseVersion}`).code; + exec( + `node scripts/set-rn-version.js --to-version ${releaseVersion} --build-type ${buildType}`, + ).code; // Generate native files for Android generateAndroidArtifacts(releaseVersion, tmpPublishingFolder); diff --git a/scripts/version-utils.js b/scripts/version-utils.js index daae4e7d60c923..e73ea62ed44249 100644 --- a/scripts/version-utils.js +++ b/scripts/version-utils.js @@ -9,21 +9,140 @@ const VERSION_REGEX = /^v?((\d+)\.(\d+)\.(\d+)(?:-(.+))?)$/; -function parseVersion(versionStr) { - const match = versionStr.match(VERSION_REGEX); - if (!match) { - throw new Error( - `You must pass a correctly formatted version; couldn't parse ${versionStr}`, - ); - } +/** + * Parses a version string and performs some checks to verify its validity. + * A valid version is in the format vX.Y.Z[-KKK] where X, Y, Z are numbers and KKK can be something else. + * The `builtType` is used to enforce that the major version can assume only specific + * values. + * + * Some examples of valid versions are: + * - stable: 0.68.1 + * - stable prerelease: 0.70.0-rc.0 + * - nightly: 0.0.0-20221116-2018-0bc4547fc + * - dryrun: 1000.0.0 + * + * Parameters: + * - @versionStr the string representing a version + * - @buildType the build type. It can be of values: `dry-run`, `release`, `nightly` + * + * Returns: an object with the shape: + * ``` + * { + * version: string, + * major: number, + * minor: number, + * patch: number, + * prerelease: string + * } + * ``` + * + */ +function parseVersion(versionStr, buildType) { + validateBuildType(buildType); + + const match = extractMatchIfValid(versionStr); const [, version, major, minor, patch, prerelease] = match; - return { + + const versionObject = { version, major, minor, patch, prerelease, }; + + validateVersion(versionObject, buildType); + + return versionObject; +} + +function validateBuildType(buildType) { + const validBuildTypes = new Set(['release', 'dry-run', 'nightly']); + if (!validBuildTypes.has(buildType)) { + throw new Error(`Unsupported build type: ${buildType}`); + } +} + +function extractMatchIfValid(versionStr) { + const match = versionStr.match(VERSION_REGEX); + if (!match) { + throw new Error( + `You must pass a correctly formatted version; couldn't parse ${versionStr}`, + ); + } + return match; +} + +function validateVersion(versionObject, buildType) { + const map = { + release: validateRelease, + 'dry-run': validateDryRun, + nightly: validateNightly, + }; + + const validationFunction = map[buildType]; + validationFunction(versionObject); +} + +/** + * Releases are in the form of 0.Y.Z[-RC.0] + */ +function validateRelease(version) { + const validRelease = isStableRelease(version) || isStablePrerelease(version); + if (!validRelease) { + throw new Error(`Version ${version.version} is not valid for Release`); + } +} + +function validateDryRun(version) { + const isNightly = isNightlyBuild(version) && version.prerelease != null; + + if ( + !isMain(version) && + !isNightly && + !isStableRelease(version) && + !isStablePrerelease(version) + ) { + throw new Error(`Version ${version.version} is not valid for dry-runs`); + } +} + +function validateNightly(version) { + // a valid nightly is a prerelease + const isPrerelease = version.prerelease != null; + const isValidNightly = isNightlyBuild(version) && isPrerelease; + if (!isValidNightly) { + throw new Error(`Version ${version.version} is not valid for nightlies`); + } +} + +function isStableRelease(version) { + return ( + version.major === '0' && version.minor !== '0' && version.prerelease == null + ); +} + +function isStablePrerelease(version) { + return ( + version.major === '0' && + version.minor !== '0' && + version.patch === '0' && + version.prerelease != null && + (version.prerelease.startsWith('rc.') || + version.prerelease.startsWith('rc-')) + ); +} + +function isNightlyBuild(version) { + return ( + version.major === '0' && version.minor === '0' && version.patch === '0' + ); +} + +function isMain(version) { + return ( + version.major === '1000' && version.minor === '0' && version.patch === '0' + ); } function isReleaseBranch(branch) { @@ -31,6 +150,10 @@ function isReleaseBranch(branch) { } module.exports = { + validateBuildType, parseVersion, isReleaseBranch, + isMain, + isStableRelease, + isStablePrerelease, };