From 5bd403122645ef0f0924ac5466f56e670a8f5b8d Mon Sep 17 00:00:00 2001 From: Timothy Yung Date: Thu, 30 May 2024 07:26:12 -0700 Subject: [PATCH] Revert Build Versions from Content Hash to Commit Hash (#29663) https://github.com/facebook/react/pull/29236 caused issues for internal syncs at Meta, because we were computing version numbers using file hashes (to eliminate "no-op" internal sync commits). The problem is that since version numbers may not be consistent across synced files (e.g. if some files have not changed in recent commits), the newly introduced version mismatch check fails. There's some more work that needs to be done here to restore the benefits of file-specific hashing, but for now this simply reverts the content hash changes from the following PRs: - https://github.com/facebook/react/pull/28633 (95319ab5afd384f5858f7c080573b9736e6b2f9c) - https://github.com/facebook/react/pull/28590 (37676aba76a9b97e1059e6dec39c3f401f44248d) - https://github.com/facebook/react/pull/28582 (cb076b593cec3a92338958f58468cce19cb8f0d9) - https://github.com/facebook/react/pull/26734 (5dd90c562354758942c833b0a46923176e92208e) - https://github.com/facebook/react/pull/26331 (3cad3a54eda7b2d1c670c2d414f33d78a4c3f6af) --- .github/workflows/commit_artifacts.yml | 23 ++------- scripts/rollup/build-all-release-channels.js | 54 ++++---------------- 2 files changed, 12 insertions(+), 65 deletions(-) diff --git a/.github/workflows/commit_artifacts.yml b/.github/workflows/commit_artifacts.yml index 3b09f99803..2f49dfbf29 100644 --- a/.github/workflows/commit_artifacts.yml +++ b/.github/workflows/commit_artifacts.yml @@ -147,7 +147,7 @@ jobs: mkdir -p ${BASE_FOLDER}/react-native-github/Libraries/Renderer/ mkdir -p ${BASE_FOLDER}/RKJSModules/vendor/react/{scheduler,react,react-is,react-test-renderer}/ - # Move React Native renderer + # Move React Native renderer mv build/react-native/implementations/ $BASE_FOLDER/react-native-github/Libraries/Renderer/ mv build/react-native/shims/ $BASE_FOLDER/react-native-github/Libraries/Renderer/ mv build/facebook-react-native/scheduler/cjs/ $BASE_FOLDER/RKJSModules/vendor/react/scheduler/ @@ -161,9 +161,10 @@ jobs: rm $RENDERER_FOLDER/ReactNativeRenderer-{dev,prod,profiling}.js ls -R ./compiled - - name: Add REVISION file + - name: Add REVISION files run: | echo ${{ github.sha }} >> ./compiled/facebook-www/REVISION + cp ./compiled/facebook-www/REVISION ./compiled/facebook-www/REVISION_TRANSFORMS echo ${{ github.sha }} >> ./compiled-rn/facebook-fbsource/xplat/js/react-native-github/Libraries/Renderer/REVISION - uses: actions/upload-artifact@v3 with: @@ -189,16 +190,7 @@ jobs: name: compiled path: compiled/ - run: git status -u - - name: Check if only the REVISION file has changed - id: check_should_commit - run: | - if git status --porcelain | grep -qv '/REVISION$'; then - echo "should_commit=true" >> "$GITHUB_OUTPUT" - else - echo "should_commit=false" >> "$GITHUB_OUTPUT" - fi - name: Commit changes to branch - if: steps.check_should_commit.outputs.should_commit == 'true' uses: stefanzweifel/git-auto-commit-action@v4 with: commit_message: | @@ -225,16 +217,7 @@ jobs: name: compiled-rn path: compiled-rn/ - run: git status -u - - name: Check if only the REVISION file has changed - id: check_should_commit - run: | - if git status --porcelain | grep -qv '/REVISION$'; then - echo "should_commit=true" >> "$GITHUB_OUTPUT" - else - echo "should_commit=false" >> "$GITHUB_OUTPUT" - fi - name: Commit changes to branch - if: steps.check_should_commit.outputs.should_commit == 'true' uses: stefanzweifel/git-auto-commit-action@v4 with: commit_message: | diff --git a/scripts/rollup/build-all-release-channels.js b/scripts/rollup/build-all-release-channels.js index 098177baf9..aef2834174 100644 --- a/scripts/rollup/build-all-release-channels.js +++ b/scripts/rollup/build-all-release-channels.js @@ -2,7 +2,6 @@ /* eslint-disable no-for-of-loops/no-for-of-loops */ -const crypto = require('node:crypto'); const fs = require('fs'); const fse = require('fs-extra'); const {spawnSync} = require('child_process'); @@ -41,7 +40,10 @@ if (dateString.startsWith("'")) { // Build the artifacts using a placeholder React version. We'll then do a string // replace to swap it with the correct version per release channel. -const PLACEHOLDER_REACT_VERSION = ReactVersion + '-PLACEHOLDER'; +// +// The placeholder version is the same format that the "next" channel uses +const PLACEHOLDER_REACT_VERSION = + ReactVersion + '-' + canaryChannelLabel + '-' + sha + '-' + dateString; // TODO: We should inject the React version using a build-time parameter // instead of overwriting the source files. @@ -158,7 +160,7 @@ function processStable(buildDir) { } if (fs.existsSync(buildDir + '/facebook-www')) { - for (const fileName of fs.readdirSync(buildDir + '/facebook-www').sort()) { + for (const fileName of fs.readdirSync(buildDir + '/facebook-www')) { const filePath = buildDir + '/facebook-www/' + fileName; const stats = fs.statSync(filePath); if (!stats.isDirectory()) { @@ -167,28 +169,10 @@ function processStable(buildDir) { } updatePlaceholderReactVersionInCompiledArtifacts( buildDir + '/facebook-www', - ReactVersion + '-www-classic-%FILEHASH%' + ReactVersion + '-www-classic-' + sha + '-' + dateString ); } - [ - buildDir + '/react-native/implementations/', - buildDir + '/facebook-react-native/', - ].forEach(reactNativeBuildDir => { - if (fs.existsSync(reactNativeBuildDir)) { - updatePlaceholderReactVersionInCompiledArtifacts( - reactNativeBuildDir, - ReactVersion + '-' + canaryChannelLabel + '-%FILEHASH%' - ); - } - }); - - // Update remaining placeholders with canary channel version - updatePlaceholderReactVersionInCompiledArtifacts( - buildDir, - ReactVersion + '-' + canaryChannelLabel + '-' + sha + '-' + dateString - ); - if (fs.existsSync(buildDir + '/sizes')) { fs.renameSync(buildDir + '/sizes', buildDir + '/sizes-stable'); } @@ -222,7 +206,7 @@ function processExperimental(buildDir, version) { } if (fs.existsSync(buildDir + '/facebook-www')) { - for (const fileName of fs.readdirSync(buildDir + '/facebook-www').sort()) { + for (const fileName of fs.readdirSync(buildDir + '/facebook-www')) { const filePath = buildDir + '/facebook-www/' + fileName; const stats = fs.statSync(filePath); if (!stats.isDirectory()) { @@ -231,28 +215,10 @@ function processExperimental(buildDir, version) { } updatePlaceholderReactVersionInCompiledArtifacts( buildDir + '/facebook-www', - ReactVersion + '-www-modern-%FILEHASH%' + ReactVersion + '-www-modern-' + sha + '-' + dateString ); } - [ - buildDir + '/react-native/implementations/', - buildDir + '/facebook-react-native/', - ].forEach(reactNativeBuildDir => { - if (fs.existsSync(reactNativeBuildDir)) { - updatePlaceholderReactVersionInCompiledArtifacts( - reactNativeBuildDir, - ReactVersion + '-' + canaryChannelLabel + '-%FILEHASH%' - ); - } - }); - - // Update remaining placeholders with canary channel version - updatePlaceholderReactVersionInCompiledArtifacts( - buildDir, - ReactVersion + '-' + canaryChannelLabel + '-' + sha + '-' + dateString - ); - if (fs.existsSync(buildDir + '/sizes')) { fs.renameSync(buildDir + '/sizes', buildDir + '/sizes-experimental'); } @@ -362,11 +328,9 @@ function updatePlaceholderReactVersionInCompiledArtifacts( for (const artifactFilename of artifactFilenames) { const originalText = fs.readFileSync(artifactFilename, 'utf8'); - const fileHash = crypto.createHash('sha1'); - fileHash.update(originalText); const replacedText = originalText.replaceAll( PLACEHOLDER_REACT_VERSION, - newVersion.replace(/%FILEHASH%/g, fileHash.digest('hex').slice(0, 8)) + newVersion ); fs.writeFileSync(artifactFilename, replacedText); }