From 6b35ff766aadedae466ce1db0133b38678d75114 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 10 Jun 2024 10:16:40 -0400 Subject: [PATCH] Move @generated from build to sync (#29799) ## Overview Reverts https://github.com/facebook/react/pull/26616 and implements the suggested way instead. This change in #26616 broken the internal sync command, which now results in duplicated `@generated` headers. It also makes it harder to detect changes during the diff train sync. Instead, we will check for changes, and if there are changes sign the files and commit them to the sync branch. ## Strategy The new sync strategy accounts for the generated headers during the sync: - **Revert Version**: Revert the version strings - **Revert @generated**: Re-sign the files (will be the same hash as before if unchanged) - **Check**: Check if there are changes **if not, skip** - **Re-apply Version**: Now add back the new version string - **Re-sign @generated**: And re-generate the headers Then commit to branch. This ensures that if there are no changes, we'll skip. --------- Co-authored-by: Timothy Yung --- .github/workflows/commit_artifacts.yml | 195 +++++++++++++++++- .../src/ReactNativeTypes.js | 3 +- .../Libraries/ReactPrivate/deepDiffer.js | 2 - scripts/rollup/packaging.js | 20 -- .../rollup/shims/react-native/ReactFabric.js | 3 +- .../shims/react-native/ReactFeatureFlags.js | 3 +- .../rollup/shims/react-native/ReactNative.js | 4 +- .../ReactNativeViewConfigRegistry.js | 3 +- .../createReactNativeComponentClass.js | 3 +- scripts/rollup/wrappers.js | 31 ++- 10 files changed, 211 insertions(+), 56 deletions(-) diff --git a/.github/workflows/commit_artifacts.yml b/.github/workflows/commit_artifacts.yml index aec96b0fe3..8c016b276f 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,109 @@ 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 + - name: Check changes before signing + 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 --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) { + process.exitCode = 1; + console.error('Error signing files:', e); + } + - name: Check for changes id: check_should_commit run: | echo "Full git status" - git status + git add . + git status --porcelain 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 "===================" echo "should_commit=true" >> "$GITHUB_OUTPUT" else echo "No Changes detected" @@ -348,10 +445,92 @@ 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) { + process.exitCode = 1; + 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' diff --git a/packages/react-native-renderer/src/ReactNativeTypes.js b/packages/react-native-renderer/src/ReactNativeTypes.js index 1b38ed2ad9..0b4f9a1045 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 5e8b3a309f..200668afdf 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/packaging.js b/scripts/rollup/packaging.js index 24b3d79316..7c433fc2dd 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/shims/react-native/ReactFabric.js b/scripts/rollup/shims/react-native/ReactFabric.js index 417c156c2f..61f6d72688 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 ef3595c1f8..69e6f48b68 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 c11b4a3a8e..d0297cc9a6 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 84a7082ea2..98e03187c7 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 bbacdc472e..388991e45b 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 */ diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js index e76931bb85..e01ca30a6d 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}`; }, };