From 77e2b92d07664972c17a4eeff7919aa077ffb628 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 7 Jun 2024 10:46:53 -0400 Subject: [PATCH 1/4] [WIP] Move @generated signing from build to sync --- scripts/rollup/packaging.js | 20 -------------------- scripts/rollup/wrappers.js | 31 ++++++++++++------------------- 2 files changed, 12 insertions(+), 39 deletions(-) diff --git a/scripts/rollup/packaging.js b/scripts/rollup/packaging.js index 24b3d79316d4a..7c433fc2dd330 100644 --- a/scripts/rollup/packaging.js +++ b/scripts/rollup/packaging.js @@ -7,7 +7,6 @@ const { readFileSync, writeFileSync, } = require('fs'); -const path = require('path'); const Bundles = require('./bundles'); const { asyncCopyTo, @@ -15,7 +14,6 @@ const { asyncExtractTar, asyncRimRaf, } = require('./utils'); -const {getSigningToken, signFile} = require('signedsource'); const { NODE_ES2015, @@ -127,24 +125,6 @@ async function copyRNShims() { require.resolve('react-native-renderer/src/ReactNativeTypes.js'), 'build/react-native/shims/ReactNativeTypes.js' ); - processGenerated('build/react-native/shims'); -} - -function processGenerated(directory) { - const files = readdirSync(directory) - .filter(dir => dir.endsWith('.js')) - .map(file => path.join(directory, file)); - - files.forEach(file => { - const originalContents = readFileSync(file, 'utf8'); - const contents = originalContents - // Replace {@}format with {@}noformat - .replace(/(\r?\n\s*\*\s*)@format\b.*(\n)/, '$1@noformat$2') - // Add {@}nolint and {@}generated - .replace(/(\r?\n\s*\*)\//, `$1 @nolint$1 ${getSigningToken()}$1/`); - const signedContents = signFile(contents); - writeFileSync(file, signedContents, 'utf8'); - }); } async function copyAllShims() { diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js index e76931bb85b42..e01ca30a6d054 100644 --- a/scripts/rollup/wrappers.js +++ b/scripts/rollup/wrappers.js @@ -1,6 +1,5 @@ 'use strict'; -const {signFile, getSigningToken} = require('signedsource'); const {bundleTypes, moduleTypes} = require('./bundles'); const { @@ -392,86 +391,80 @@ ${source}`; /****************** RN_OSS_DEV ******************/ [RN_OSS_DEV](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, /****************** RN_OSS_PROD ******************/ [RN_OSS_PROD](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, /****************** RN_OSS_PROFILING ******************/ [RN_OSS_PROFILING](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, /****************** RN_FB_DEV ******************/ [RN_FB_DEV](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, /****************** RN_FB_PROD ******************/ [RN_FB_PROD](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, /****************** RN_FB_PROFILING ******************/ [RN_FB_PROFILING](source, globalName, filename, moduleType) { - return signFile(`/** + return `/** ${license} * * @noflow * @nolint * @preventMunge - * ${getSigningToken()} */ -${source}`); +${source}`; }, }; From 2f8134ddde8d274b4639ddb3d8b445c01a580bdc Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 7 Jun 2024 11:18:42 -0400 Subject: [PATCH 2/4] Change @format headers to @noformat --- packages/react-native-renderer/src/ReactNativeTypes.js | 3 ++- .../react-native/Libraries/ReactPrivate/deepDiffer.js | 2 -- scripts/rollup/shims/react-native/ReactFabric.js | 3 ++- scripts/rollup/shims/react-native/ReactFeatureFlags.js | 3 ++- scripts/rollup/shims/react-native/ReactNative.js | 4 ++-- .../shims/react-native/ReactNativeViewConfigRegistry.js | 3 ++- .../shims/react-native/createReactNativeComponentClass.js | 3 ++- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js index 1b38ed2ad9608..0b4f9a1045b84 100644 --- a/packages/react-native-renderer/src/ReactNativeTypes.js +++ b/packages/react-native-renderer/src/ReactNativeTypes.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow strict */ diff --git a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/deepDiffer.js b/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/deepDiffer.js index 5e8b3a309f9ca..200668afdfe12 100644 --- a/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/deepDiffer.js +++ b/packages/react-native-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/deepDiffer.js @@ -4,8 +4,6 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format - * @flow */ 'use strict'; diff --git a/scripts/rollup/shims/react-native/ReactFabric.js b/scripts/rollup/shims/react-native/ReactFabric.js index 417c156c2f607..61f6d726887b3 100644 --- a/scripts/rollup/shims/react-native/ReactFabric.js +++ b/scripts/rollup/shims/react-native/ReactFabric.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow */ diff --git a/scripts/rollup/shims/react-native/ReactFeatureFlags.js b/scripts/rollup/shims/react-native/ReactFeatureFlags.js index ef3595c1f8db7..69e6f48b68a34 100644 --- a/scripts/rollup/shims/react-native/ReactFeatureFlags.js +++ b/scripts/rollup/shims/react-native/ReactFeatureFlags.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow strict-local */ diff --git a/scripts/rollup/shims/react-native/ReactNative.js b/scripts/rollup/shims/react-native/ReactNative.js index c11b4a3a8e73c..d0297cc9a61cb 100644 --- a/scripts/rollup/shims/react-native/ReactNative.js +++ b/scripts/rollup/shims/react-native/ReactNative.js @@ -4,10 +4,10 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow */ - 'use strict'; import type {ReactNativeType} from './ReactNativeTypes'; diff --git a/scripts/rollup/shims/react-native/ReactNativeViewConfigRegistry.js b/scripts/rollup/shims/react-native/ReactNativeViewConfigRegistry.js index 84a7082ea26e2..98e03187c74a5 100644 --- a/scripts/rollup/shims/react-native/ReactNativeViewConfigRegistry.js +++ b/scripts/rollup/shims/react-native/ReactNativeViewConfigRegistry.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow strict-local */ diff --git a/scripts/rollup/shims/react-native/createReactNativeComponentClass.js b/scripts/rollup/shims/react-native/createReactNativeComponentClass.js index bbacdc472e46f..388991e45b545 100644 --- a/scripts/rollup/shims/react-native/createReactNativeComponentClass.js +++ b/scripts/rollup/shims/react-native/createReactNativeComponentClass.js @@ -4,7 +4,8 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @format + * @noformat + * @nolint * @flow strict-local */ From b683b3c88fd87525508aa2b729221260ccd40d5b Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 7 Jun 2024 17:21:06 -0400 Subject: [PATCH 3/4] Update commit_artifacts --- .github/workflows/commit_artifacts.yml | 193 ++++++++++++++++++++++++- 1 file changed, 185 insertions(+), 8 deletions(-) diff --git a/.github/workflows/commit_artifacts.yml b/.github/workflows/commit_artifacts.yml index aec96b0fe31fb..e2c170d81d521 100644 --- a/.github/workflows/commit_artifacts.yml +++ b/.github/workflows/commit_artifacts.yml @@ -52,6 +52,7 @@ jobs: CIRCLECI_TOKEN: ${{secrets.CIRCLECI_TOKEN_DIFFTRAIN}} with: script: | + // TODO: Move this to a script file. const cp = require('child_process'); function sleep(ms) { @@ -250,14 +251,18 @@ jobs: grep -rl "$CURRENT_VERSION_MODERN" ./compiled || echo "No files found with $CURRENT_VERSION_MODERN" grep -rl "$CURRENT_VERSION_MODERN" ./compiled | xargs -r sed -i -e "s/$CURRENT_VERSION_MODERN/$LAST_VERSION_MODERN/g" grep -rl "$CURRENT_VERSION_MODERN" ./compiled || echo "Modern version reverted" - - name: Check if only the REVISION file has changed + - name: Check for changes id: check_should_commit run: | echo "Full git status" + git add . git status echo "====================" if git status --porcelain | grep -qv '/REVISION'; then echo "Changes detected" + echo "===== Changes =====" + git --no-pager diff -U0 | grep '^[+-]' | head -n 50 + echo "===================" echo "should_commit=true" >> "$GITHUB_OUTPUT" else echo "No Changes detected" @@ -322,17 +327,108 @@ jobs: grep -rl "$CURRENT_VERSION" ./compiled-rn || echo "No files found with $CURRENT_VERSION" grep -rl "$CURRENT_VERSION" ./compiled-rn | xargs -r sed -i -e "s/$CURRENT_VERSION/$LAST_VERSION/g" grep -rl "$CURRENT_VERSION" ./compiled-rn || echo "Version reverted" - - name: Check if only the REVISION file has changed - id: check_should_commit + - name: Check changes before signing run: | echo "Full git status" + git add . git status echo "====================" - echo "Checking for changes" - # Check if there are changes in the files other than REVISION or @generated headers - # We also filter out the file name lines with "---" and "+++". - if git diff -- . ':(exclude)*REVISION' | grep -vE "^(@@|diff|index|\-\-\-|\+\+\+|@generated SignedSource)" | grep "^[+-]" > /dev/null; then + if git status --porcelain | grep -qv '/REVISION'; then + echo "Changes detected" + echo "===== Changes =====" + git --no-pager diff -U0 --cached | grep '^[+-]' | head -n 50 + echo "===================" + else + echo "No Changes detected" + fi + - name: Revert signatures + uses: actions/github-script@v6 + with: + script: | + // TODO: Move this to a script file. + // We currently can't call scripts from the repo because + // at this point in the workflow, we're on the compiled + // artifact branch (so the scripts don't exist). + // We can fix this with a composite action in the main repo. + // This script is duplicated below. + const fs = require('fs'); + const crypto = require('crypto'); + const {execSync} = require('child_process'); + + // TODO: when we move this to a script, we can use this from npm. + // Copy of signedsource since we can't install deps on this branch + const GENERATED = '@' + 'generated'; + const NEWTOKEN = '<>'; + const PATTERN = new RegExp(`${GENERATED} (?:SignedSource<<([a-f0-9]{32})>>)`); + + const TokenNotFoundError = new Error( + `SignedSource.signFile(...): Cannot sign file without token: ${NEWTOKEN}` + ); + + function hash(data, encoding) { + const md5sum = crypto.createHash('md5'); + md5sum.update(data, encoding); + return md5sum.digest('hex'); + } + + const SignedSource = { + getSigningToken() { + return `${GENERATED} ${NEWTOKEN}`; + }, + isSigned(data) { + return PATTERN.exec(data) != null; + }, + signFile(data) { + if (!data.includes(NEWTOKEN)) { + if (SignedSource.isSigned(data)) { + // Signing a file that was previously signed. + data = data.replace(PATTERN, SignedSource.getSigningToken()); + } else { + throw TokenNotFoundError; + } + } + return data.replace(NEWTOKEN, `SignedSource<<${hash(data, 'utf8')}>>`); + }, + }; + + const directory = './compiled-rn'; + console.log('Signing files in directory:', directory); + try { + const result = execSync(`git status --porcelain ${directory}`, {encoding: 'utf8'}); + + // Parse the git status output to get file paths + const files = result.split('\n').filter(file => file.endsWith('.js')); + + if (files.length === 0) { + throw new Error( + 'git status returned no files to sign. this job should not have run.' + ); + } else { + files.forEach(line => { + const file = line.slice(3).trim(); + if (file) { + console.log(' Signing file:', file); + const originalContents = fs.readFileSync(file, 'utf8'); + const signedContents = SignedSource.signFile(originalContents); + fs.writeFileSync(file, signedContents, 'utf8'); + } + }); + } + } catch (e) { + console.error('Error signing files:', e); + } + - name: Check for changes + id: check_should_commit + run: | + echo "Full git status" + git add . + git status --porcelain + echo "====================" + if git status --porcelain | grep -qv '/REVISION'; then echo "Changes detected" + echo "===== Changes =====" + git --no-pager diff -U0 --cached | grep '^[+-]' | head -n 50 + echo "===================" echo "should_commit=true" >> "$GITHUB_OUTPUT" else echo "No Changes detected" @@ -348,10 +444,91 @@ jobs: grep -rl "$LAST_VERSION" ./compiled-rn || echo "No files found with $LAST_VERSION" grep -rl "$LAST_VERSION" ./compiled-rn | xargs -r sed -i -e "s/$LAST_VERSION/$CURRENT_VERSION/g" grep -rl "$LAST_VERSION" ./compiled-rn || echo "Version re-applied" - - name: Will commit these changes + - name: Add files if: steps.check_should_commit.outputs.should_commit == 'true' run: | echo ":" + git add . + - name: Signing files + if: steps.check_should_commit.outputs.should_commit == 'true' + uses: actions/github-script@v6 + with: + script: | + // TODO: Move this to a script file. + // We currently can't call scripts from the repo because + // at this point in the workflow, we're on the compiled + // artifact branch (so the scripts don't exist). + // We can fix this with a composite action in the main repo. + // This script is duplicated above. + const fs = require('fs'); + const crypto = require('crypto'); + const {execSync} = require('child_process'); + + // TODO: when we move this to a script, we can use this from npm. + // Copy of signedsource since we can't install deps on this branch. + const GENERATED = '@' + 'generated'; + const NEWTOKEN = '<>'; + const PATTERN = new RegExp(`${GENERATED} (?:SignedSource<<([a-f0-9]{32})>>)`); + + const TokenNotFoundError = new Error( + `SignedSource.signFile(...): Cannot sign file without token: ${NEWTOKEN}` + ); + + function hash(data, encoding) { + const md5sum = crypto.createHash('md5'); + md5sum.update(data, encoding); + return md5sum.digest('hex'); + } + + const SignedSource = { + getSigningToken() { + return `${GENERATED} ${NEWTOKEN}`; + }, + isSigned(data) { + return PATTERN.exec(data) != null; + }, + signFile(data) { + if (!data.includes(NEWTOKEN)) { + if (SignedSource.isSigned(data)) { + // Signing a file that was previously signed. + data = data.replace(PATTERN, SignedSource.getSigningToken()); + } else { + throw TokenNotFoundError; + } + } + return data.replace(NEWTOKEN, `SignedSource<<${hash(data, 'utf8')}>>`); + }, + }; + + const directory = './compiled-rn'; + console.log('Signing files in directory:', directory); + try { + const result = execSync(`git status --porcelain ${directory}`, {encoding: 'utf8'}); + + // Parse the git status output to get file paths + const files = result.split('\n').filter(file => file.endsWith('.js')); + + if (files.length === 0) { + throw new Error( + 'git status returned no files to sign. this job should not have run.' + ); + } else { + files.forEach(line => { + const file = line.slice(3).trim(); + if (file) { + console.log(' Signing file:', file); + const originalContents = fs.readFileSync(file, 'utf8'); + const signedContents = SignedSource.signFile(originalContents); + fs.writeFileSync(file, signedContents, 'utf8'); + } + }); + } + } catch (e) { + console.error('Error signing files:', e); + } + - name: Will commit these changes + if: steps.check_should_commit.outputs.should_commit == 'true' + run: | git status -u - name: Commit changes to branch if: steps.check_should_commit.outputs.should_commit == 'true' From eb089cd7c33bbf3abb469529896c1245cdb0c85a Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 10 Jun 2024 10:06:56 -0400 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Timothy Yung --- .github/workflows/commit_artifacts.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/commit_artifacts.yml b/.github/workflows/commit_artifacts.yml index e2c170d81d521..8c016b276ff05 100644 --- a/.github/workflows/commit_artifacts.yml +++ b/.github/workflows/commit_artifacts.yml @@ -389,7 +389,7 @@ jobs: } return data.replace(NEWTOKEN, `SignedSource<<${hash(data, 'utf8')}>>`); }, - }; + }; const directory = './compiled-rn'; console.log('Signing files in directory:', directory); @@ -415,6 +415,7 @@ jobs: }); } } catch (e) { + process.exitCode = 1; console.error('Error signing files:', e); } - name: Check for changes @@ -498,7 +499,7 @@ jobs: } return data.replace(NEWTOKEN, `SignedSource<<${hash(data, 'utf8')}>>`); }, - }; + }; const directory = './compiled-rn'; console.log('Signing files in directory:', directory); @@ -524,6 +525,7 @@ jobs: }); } } catch (e) { + process.exitCode = 1; console.error('Error signing files:', e); } - name: Will commit these changes