diff --git a/.github/workflows/runtime_build_and_test.yml b/.github/workflows/runtime_build_and_test.yml index f2f64af45290..f08bfc450968 100644 --- a/.github/workflows/runtime_build_and_test.yml +++ b/.github/workflows/runtime_build_and_test.yml @@ -999,6 +999,8 @@ jobs: permissions: # We use github.token to download the build artifact from a previous runtime_build_and_test.yml run actions: read + # Used to resolve the merge-base commit through the compare API + contents: read runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -1020,21 +1022,40 @@ jobs: if: steps.node_modules.outputs.cache-hit != 'true' - run: yarn --cwd scripts/release install --frozen-lockfile if: steps.node_modules.outputs.cache-hit != 'true' + - name: Resolve base commit for comparison + # Compare against the commit this pull request diverged from, not the + # current tip of the base branch. Otherwise, once main moves ahead, + # unrelated commits show up as size changes on every pull request. + id: base + run: | + echo "sha=$(gh api repos/${{ github.repository }}/compare/${{ github.event.pull_request.base.sha }}...${{ github.event.pull_request.head.sha }} --jq .merge_base_commit.sha)" >> "$GITHUB_OUTPUT" + env: + GH_TOKEN: ${{ github.token }} - name: Download artifacts for base revision + id: download_base # The build could have been generated from a fork, so we must download the build without # any verification. This is safe since we only use this for sizebot calculation and the # unverified artifact is not used. Additionally this workflow runs in the pull_request # trigger so only restricted permissions are available. run: | - GH_TOKEN=${{ github.token }} scripts/release/download-experimental-build.js --commit=$(git rev-parse ${{ github.event.pull_request.base.sha }}) ${{ (github.event.pull_request.head.repo.full_name != github.repository && '--noVerify') || ''}} - mv ./build ./base-build + if GH_TOKEN=${{ github.token }} scripts/release/download-experimental-build.js --commit=${{ steps.base.outputs.sha }} ${{ (github.event.pull_request.head.repo.full_name != github.repository && '--noVerify') || ''}}; then + mv ./build ./base-build + else + # Rendered as a warning comment by render-comment.js on the + # default branch, which also fails the comment workflow's check. + jq --null-input --arg baseSha "${{ steps.base.outputs.sha }}" \ + '{version: 1, status: "base-build-not-found", baseSha: $baseSha}' \ + > sizebot-results.json + echo "missing=true" >> "$GITHUB_OUTPUT" + fi - name: Delete extraneous files # TODO: The `download-experimental-build` script copies the npm # packages into the `node_modules` directory. This is a historical # quirk of how the release script works. Let's pretend they # don't exist. run: rm -rf ./base-build/node_modules - - name: Display structure of base-build from origin/main + - name: Display structure of base-build + if: steps.download_base.outputs.missing != 'true' run: ls -R base-build - name: Ensure clean build directory run: rm -rf build @@ -1056,6 +1077,9 @@ jobs: # posted by runtime_sizebot_comment.yml, which runs on the workflow_run # trigger because this job's token is read-only for pull requests from # forks and so cannot comment. + # Skipped when the base build is missing: the download step already + # wrote the not-found results. + if: steps.download_base.outputs.missing != 'true' run: node ./scripts/sizebot/compare-sizes.js - name: Archive sizebot results uses: actions/upload-artifact@v4 @@ -1063,3 +1087,8 @@ jobs: name: sizebot-results path: sizebot-results.json if-no-files-found: error + # Note: a missing base build does not fail this job on purpose. A failed + # job would fail the whole run, and render-comment.js only reads the + # results of successful runs, so the warning would never be rendered. + # Instead the comment workflow fails on the sizebot-problem.txt marker + # after posting the warning, same as for build configuration drift. diff --git a/.github/workflows/runtime_sizebot_comment.yml b/.github/workflows/runtime_sizebot_comment.yml index 5d6909650f41..263a2a1cbbb0 100644 --- a/.github/workflows/runtime_sizebot_comment.yml +++ b/.github/workflows/runtime_sizebot_comment.yml @@ -95,10 +95,11 @@ jobs: const {post} = require(`${process.env.GITHUB_WORKSPACE}/scripts/sizebot/pull-request-comment.js`); await post({github, context, core}); - - name: Fail if the build configuration drifted + - name: Fail if sizebot reported a problem # The comment is posted first, so it explains the problem on the pull - # request itself. This step exists so the drift also shows up as a failed - # run rather than only in a comment. + # request itself. This step exists so the problem (build configuration + # drift, or no base build to compare against) also shows up as a + # failed run rather than only in a comment. if: ${{ steps.resolve.outputs.action == 'continue' && hashFiles('sizebot-problem.txt') != '' }} run: | cat sizebot-problem.txt diff --git a/packages/react-devtools-extensions/utils.js b/packages/react-devtools-extensions/utils.js index 9f02e98d1988..ce12465b35c3 100644 --- a/packages/react-devtools-extensions/utils.js +++ b/packages/react-devtools-extensions/utils.js @@ -6,39 +6,21 @@ */ const {execSync} = require('child_process'); -const {existsSync, readFileSync} = require('fs'); +const {readFileSync} = require('fs'); const {resolve} = require('path'); const GITHUB_URL = 'https://github.com/facebook/react'; +const GIT_COMMIT_HASH_LENGTH = 10; + +function shortenCommitHash(commitHash) { + return commitHash.trim().slice(0, GIT_COMMIT_HASH_LENGTH); +} function getGitCommit() { - try { - return execSync('git show -s --no-show-signature --format=%h') - .toString() - .trim(); - } catch (error) { - // Mozilla runs this command from a git archive. - // In that context, there is no Git context. - // Using the commit hash specified to download-experimental-build.js script as a fallback. - - // Try to read from build/COMMIT_SHA file - const commitShaPath = resolve(__dirname, '..', '..', 'build', 'COMMIT_SHA'); - if (!existsSync(commitShaPath)) { - throw new Error( - 'Could not find build/COMMIT_SHA file. Did you run scripts/release/download-experimental-build.js script?', - ); - } - - try { - const commitHash = readFileSync(commitShaPath, 'utf8').trim(); - // Return short hash (first 7 characters) to match abbreviated commit hash format - return commitHash.slice(0, 7); - } catch (readError) { - throw new Error( - `Failed to read build/COMMIT_SHA file: ${readError.message}`, - ); - } - } + const commitHash = execSync( + 'git show -s --no-show-signature --format=%H', + ).toString(); + return shortenCommitHash(commitHash); } function getVersionString(packageVersion = null) { diff --git a/scripts/sizebot/render-comment.js b/scripts/sizebot/render-comment.js index 32eceef8d859..913634d08d86 100644 --- a/scripts/sizebot/render-comment.js +++ b/scripts/sizebot/render-comment.js @@ -26,7 +26,11 @@ const {existsSync, readFileSync, writeFileSync} = require('fs'); // Results shapes this file knows how to read. `compare-sizes.js` on the pull // request branch may be older or newer than this list. const SUPPORTED_VERSIONS = new Set([1]); -const SUPPORTED_STATUSES = new Set(['ok', 'base-artifacts-unavailable']); +const SUPPORTED_STATUSES = new Set([ + 'ok', + 'base-artifacts-unavailable', + 'base-build-not-found', +]); const CRITICAL_THRESHOLD = 0.02; const SIGNIFICANCE_THRESHOLD = 0.002; @@ -161,6 +165,15 @@ function validateResults(raw) { if (raw.status === 'base-artifacts-unavailable') { return {ok: true, results: {status: raw.status}}; } + if (raw.status === 'base-build-not-found') { + return { + ok: true, + results: { + status: raw.status, + baseSha: isSha(raw.baseSha) ? raw.baseSha : null, + }, + }; + } if (!isSha(raw.baseSha) || !isSha(raw.headSha)) { return {ok: false, reason: 'malformed'}; } @@ -337,6 +350,23 @@ function renderCompletedReport(context) { }; } + if (validated.results.status === 'base-build-not-found') { + const {baseSha} = validated.results; + return { + markdown: + `No build was found for the base commit${ + baseSha === null ? '' : ` (${baseSha})` + } that this pull request diverged from, so there is no size report. ` + + 'The build for that commit may have failed, or its artifacts may be ' + + 'older than the retention window. Rebase the pull request onto a ' + + 'newer `main` to compare against a base commit that has a build.', + missingCriticalPaths: [], + problem: `No base build found for ${ + baseSha === null ? 'the merge-base' : baseSha + }`, + }; + } + return renderTable(validated.results); } @@ -370,6 +400,7 @@ function renderBody(context) { let reportHead; let report; let missingCriticalPaths = []; + let problem; if (context.action === 'requested') { // Only a comment that names the commit it describes holds real numbers. A @@ -392,6 +423,7 @@ function renderBody(context) { const rendered = renderCompletedReport(context); report = rendered.markdown; missingCriticalPaths = rendered.missingCriticalPaths; + problem = rendered.problem; } if (missingCriticalPaths.length > 0) { @@ -428,6 +460,7 @@ ${REPORT_END} assemble, reportHead, missingCriticalPaths, + problem, }; } @@ -454,7 +487,8 @@ function parseReportHead(body) { function main() { const context = JSON.parse(readFileSync(CONTEXT_PATH, 'utf8')); - const {body, report, assemble, missingCriticalPaths} = renderBody(context); + const {body, report, assemble, missingCriticalPaths, problem} = + renderBody(context); let comment = body; if (body.length > MAX_COMMENT_LENGTH) { @@ -473,6 +507,9 @@ function main() { `Missing expected bundles:\n${missingCriticalPaths.join('\n')}\n` ); } + if (problem !== undefined) { + writeFileSync(PROBLEM_PATH, problem + '\n'); + } process.stdout.write(comment); }