From 7eb2c0a5741589d8ea6063934b2e4a2212a6f88a Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Mon, 21 Nov 2022 04:31:35 -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: fcae8101e57dbef79203881961b61d8663285fdf --- .circleci/config.yml | 6 +- 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 | 38 ++-- scripts/test-e2e-local.js | 11 +- scripts/version-utils.js | 139 +++++++++++- 8 files changed, 444 insertions(+), 56 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ea68661ca20478..c38387e79d744f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1363,7 +1363,9 @@ 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 >> + PACKAGE_JSON_VERSION=$(npm pgk get version | tr -d '"') + VERSION=${<< parameters.version >>:-"$PACKAGE_JSON_VERSION"} + node ./scripts/prepare-package-for-release.js -v "$VERSION" -l << parameters.latest >> --dry-run << parameters.dryrun >> build_npm_package: parameters: @@ -1643,7 +1645,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 ed14b1015270a0..6a5f1a8e50539b 100755 --- a/scripts/bump-oss-version.js +++ b/scripts/bump-oss-version.js @@ -89,7 +89,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 4f89743c618827..b976b231e4a8e6 100755 --- a/scripts/prepare-package-for-release.js +++ b/scripts/prepare-package-for-release.js @@ -57,13 +57,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..37b15c04d81877 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( 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, };