Update sizebot to new workflow (facebook#20719)

* build-combined: Fix bundle sizes path * Output COMMIT_SHA in build directory Alternative to parsing an arbitrary package's version number, or its `build-info.json`. * Remove CircleCI environment variable requirement I think this only reason we needed this was to support passing any job id to `--build`, instead of requiring the `process_artifacts` job. And to do that you needed to use the workflows API endpoint, which requires an API token. But now that the `--commit` argument exists and automatically finds the correct job id, we can just use that. * Add CI job that gets base artifacts Uses download-experimental script and places the base artifacts into a top-level folder. * Migrate sizebot to combined workflow Replaces the two separate sizebot jobs (one for each channel, stable and experimental) with a single combined job that outputs size information for all bundles in a single GitHub comment. I didn't attempt to simplify the output at all, but we should. I think what I would do is remove our custom Rollup sizes plugin, and read the sizes from the filesystem instead. We would lose some information about the build configuration used to generate each artifact, but that can be inferred from the filepath. For example, the filepath `fb-www/ReactDOM-dev.classic.js` already tells us everything we need to know about the artifact. Leaving this for a follow up. * Move GitHub status check inside retry loop The download script will poll the CircleCI endpoint until the build job is complete; it should also poll the GitHub status endpoint if the build job hasn't been spawned yet. * Only run get_base_build on main branch
koto · Jun 15, 2021 · f5fbc13 · f5fbc13
1 parent a3b1afd
commit f5fbc13
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 231 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -170,31 +170,57 @@ jobs:
       - *restore_node_modules
       - run: yarn build-combined
       - persist_to_workspace:
-          root: build2
+          root: .
           paths:
-            - facebook-www
-            - facebook-react-native
-            - facebook-relay
-            - oss-stable
-            - oss-experimental
-            - react-native
-            - dist
-            - sizes/*.json
+            - build2
+
+  get_base_build:
+    docker: *docker
+    environment: *environment
+    steps:
+      - checkout
+      - run: yarn workspaces info | head -n -1 > workspace_info.txt
+      - *restore_node_modules
+      - run:
+          name: Download artifacts for base revision
+          command: |
+              git fetch origin master
+              cd ./scripts/release && yarn && cd ../../
+              scripts/release/download-experimental-build.js --commit=$(git merge-base HEAD origin/master)
+              mv ./build2 ./base-build
+      - persist_to_workspace:
+          root: .
+          paths:
+            - base-build
 
   process_artifacts_combined:
     docker: *docker
     environment: *environment
     steps:
       - checkout
       - attach_workspace:
-          at: build2
+          at: .
       - run: yarn workspaces info | head -n -1 > workspace_info.txt
       - *restore_node_modules
+      - run: echo "<< pipeline.git.revision	>>" >> build2/COMMIT_SHA
         # Compress build directory into a single tarball for easy download
       - run: tar -zcvf ./build2.tgz ./build2
       - store_artifacts:
           path: ./build2.tgz
 
+  sizebot:
+    docker: *docker
+    environment: *environment
+    steps:
+      - checkout
+      - attach_workspace:
+          at: .
+      - run: echo "<< pipeline.git.revision	>>" >> build2/COMMIT_SHA
+      - run: yarn workspaces info | head -n -1 > workspace_info.txt
+      - *restore_node_modules
+      - run:
+          command: node ./scripts/tasks/danger
+
   build_devtools_and_process_artifacts:
     docker: *docker
     environment: *environment
@@ -261,38 +287,6 @@ jobs:
   process_artifacts: *process_artifacts
   process_artifacts_experimental: *process_artifacts
 
-  sizebot_stable:
-    docker: *docker
-    environment: *environment
-    steps:
-      - checkout
-      - attach_workspace: *attach_workspace
-      - run: yarn workspaces info | head -n -1 > workspace_info.txt
-      - *restore_node_modules
-      # This runs in the process_artifacts job, too, but it's faster to run
-      # this step in both jobs instead of running the jobs sequentially
-      - run: node ./scripts/rollup/consolidateBundleSizes.js
-      - run:
-          environment:
-            RELEASE_CHANNEL: stable
-          command: node ./scripts/tasks/danger
-
-  sizebot_experimental:
-    docker: *docker
-    environment: *environment
-    steps:
-      - checkout
-      - attach_workspace: *attach_workspace
-      - run: yarn workspaces info | head -n -1 > workspace_info.txt
-      - *restore_node_modules
-      # This runs in the process_artifacts job, too, but it's faster to run
-      # this step in both jobs instead of running the jobs sequentially
-      - run: node ./scripts/rollup/consolidateBundleSizes.js
-      - run:
-          environment:
-            RELEASE_CHANNEL: experimental
-          command: node ./scripts/tasks/danger
-
   yarn_lint_build:
     docker: *docker
     environment: *environment
@@ -341,7 +335,7 @@ jobs:
     steps:
       - checkout
       - attach_workspace:
-          at: build2
+          at: .
       - run: yarn workspaces info | head -n -1 > workspace_info.txt
       - *restore_node_modules
       - run: yarn test --build <<parameters.args>> --ci
@@ -394,9 +388,6 @@ workflows:
       - process_artifacts:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
-      - sizebot_stable:
-          requires:
-            - RELEASE_CHANNEL_stable_yarn_build
       - RELEASE_CHANNEL_stable_yarn_lint_build:
           requires:
             - RELEASE_CHANNEL_stable_yarn_build
@@ -413,9 +404,6 @@ workflows:
       - process_artifacts_experimental:
           requires:
             - yarn_build
-      - sizebot_experimental:
-          requires:
-            - yarn_build
       - yarn_lint_build:
           requires:
             - yarn_build
@@ -495,6 +483,21 @@ workflows:
                 # - "-r=www-modern --env=production --variant"
 
                 # TODO: Test more persistent configurations?
+      - get_base_build:
+          filters:
+            branches:
+              ignore:
+                - master
+          requires:
+            - setup
+      - sizebot:
+          filters:
+            branches:
+              ignore:
+                - master
+          requires:
+            - get_base_build
+            - yarn_build_combined
   fuzz_tests:
     triggers:
       - schedule:

diff --git a/dangerfile.js b/dangerfile.js
@@ -26,34 +26,10 @@
 // `DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/react/pull/11865
 
 const {markdown, danger, warn} = require('danger');
-const fetch = require('node-fetch');
 
 const {generateResultsArray} = require('./scripts/rollup/stats');
-const {existsSync, readFileSync} = require('fs');
-const {exec} = require('child_process');
-
-// This must match the name of the CI job that creates the build artifacts
-const RELEASE_CHANNEL =
-  process.env.RELEASE_CHANNEL === 'experimental' ? 'experimental' : 'stable';
-const artifactsJobName =
-  process.env.RELEASE_CHANNEL === 'experimental'
-    ? 'process_artifacts_experimental'
-    : 'process_artifacts';
-
-if (!existsSync('./build/bundle-sizes.json')) {
-  // This indicates the build failed previously.
-  // In that case, there's nothing for the Dangerfile to do.
-  // Exit early to avoid leaving a redundant (and potentially confusing) PR comment.
-  warn(
-    'No bundle size information found. This indicates the build ' +
-      'job failed.'
-  );
-  process.exit(0);
-}
-
-const currentBuildResults = JSON.parse(
-  readFileSync('./build/bundle-sizes.json')
-);
+const {readFileSync, readdirSync} = require('fs');
+const path = require('path');
 
 /**
  * Generates a Markdown table
@@ -100,99 +76,23 @@ function setBoldness(row, isBold) {
   }
 }
 
-/**
- * Gets the commit that represents the merge between the current branch
- * and master.
- */
-function git(args) {
-  return new Promise(res => {
-    exec('git ' + args, (err, stdout, stderr) => {
-      if (err) {
-        throw err;
-      } else {
-        res(stdout.trim());
-      }
-    });
-  });
-}
-
-(async function() {
-  // Use git locally to grab the commit which represents the place
-  // where the branches differ
-  const upstreamRepo = danger.github.pr.base.repo.full_name;
-  if (upstreamRepo !== 'facebook/react') {
-    // Exit unless we're running in the main repo
-    return;
-  }
-
-  markdown(`## Size changes (${RELEASE_CHANNEL})`);
-
-  const upstreamRef = danger.github.pr.base.ref;
-  await git(`remote add upstream https://github.com/facebook/react.git`);
-  await git('fetch upstream');
-  const baseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);
-
-  let previousBuildResults = null;
-  try {
-    let baseCIBuildId = null;
-    const statusesResponse = await fetch(
-      `https://api.github.com/repos/facebook/react/commits/${baseCommit}/status`
-    );
-    const {statuses, state} = await statusesResponse.json();
-    if (state === 'failure') {
-      warn(`Base commit is broken: ${baseCommit}`);
-      return;
-    }
-    for (let i = 0; i < statuses.length; i++) {
-      const status = statuses[i];
-      if (status.context === `ci/circleci: ${artifactsJobName}`) {
-        if (status.state === 'success') {
-          baseCIBuildId = /\/facebook\/react\/([0-9]+)/.exec(
-            status.target_url
-          )[1];
-          break;
-        }
-        if (status.state === 'pending') {
-          warn(`Build job for base commit is still pending: ${baseCommit}`);
-          return;
-        }
-      }
+function getBundleSizes(pathToSizesDir) {
+  const filenames = readdirSync(pathToSizesDir);
+  let bundleSizes = [];
+  for (let i = 0; i < filenames.length; i++) {
+    const filename = filenames[i];
+    if (filename.endsWith('.json')) {
+      const json = readFileSync(path.join(pathToSizesDir, filename));
+      bundleSizes.push(...JSON.parse(json).bundleSizes);
     }
-
-    if (baseCIBuildId === null) {
-      warn(`Could not find build artifacts for base commit: ${baseCommit}`);
-      return;
-    }
-
-    const baseArtifactsInfoResponse = await fetch(
-      `https://circleci.com/api/v1.1/project/github/facebook/react/${baseCIBuildId}/artifacts`
-    );
-    const baseArtifactsInfo = await baseArtifactsInfoResponse.json();
-
-    for (let i = 0; i < baseArtifactsInfo.length; i++) {
-      const info = baseArtifactsInfo[i];
-      if (info.path.endsWith('bundle-sizes.json')) {
-        const resultsResponse = await fetch(info.url);
-        previousBuildResults = await resultsResponse.json();
-        break;
-      }
-    }
-  } catch (error) {
-    warn(`Failed to fetch build artifacts for base commit: ${baseCommit}`);
-    return;
-  }
-
-  if (previousBuildResults === null) {
-    warn(`Could not find build artifacts for base commit: ${baseCommit}`);
-    return;
   }
+  return {bundleSizes};
+}
 
+async function printResultsForChannel(baseResults, headResults) {
   // Take the JSON of the build response and
   // make an array comparing the results for printing
-  const results = generateResultsArray(
-    currentBuildResults,
-    previousBuildResults
-  );
+  const results = generateResultsArray(baseResults, headResults);
 
   const packagesToShow = results
     .filter(
@@ -281,15 +181,62 @@ function git(args) {
   <details>
   <summary>Details of bundled changes.</summary>
 
-  <p>Comparing: ${baseCommit}...${danger.github.pr.head.sha}</p>
-
-
   ${allTables.join('\n')}
 
   </details>
   `;
-    markdown(summary);
+    return summary;
   } else {
-    markdown('No significant bundle size changes to report.');
+    return 'No significant bundle size changes to report.';
   }
+}
+
+(async function() {
+  // Use git locally to grab the commit which represents the place
+  // where the branches differ
+
+  const upstreamRepo = danger.github.pr.base.repo.full_name;
+  if (upstreamRepo !== 'facebook/react') {
+    // Exit unless we're running in the main repo
+    return;
+  }
+
+  let headSha;
+  let headSizesStable;
+  let headSizesExperimental;
+
+  let baseSha;
+  let baseSizesStable;
+  let baseSizesExperimental;
+
+  try {
+    headSha = (readFileSync('./build2/COMMIT_SHA') + '').trim();
+    headSizesStable = getBundleSizes('./build2/sizes-stable');
+    headSizesExperimental = getBundleSizes('./build2/sizes-experimental');
+
+    baseSha = (readFileSync('./base-build/COMMIT_SHA') + '').trim();
+    baseSizesStable = getBundleSizes('./base-build/sizes-stable');
+    baseSizesExperimental = getBundleSizes('./base-build/sizes-experimental');
+  } catch {
+    warn(
+      "Failed to read build artifacts. It's possible a build configuration " +
+        'has changed upstream. Try pulling the latest changes from the ' +
+        'main branch.'
+    );
+    return;
+  }
+
+  markdown(`
+## Size changes
+
+<p>Comparing: ${baseSha}...${headSha}</p>
+
+### Stable channel
+
+${await printResultsForChannel(baseSizesStable, headSizesStable)}
+
+### Experimental channel
+
+${await printResultsForChannel(baseSizesExperimental, headSizesExperimental)}
+`);
 })();
diff --git a/scripts/release/download-experimental-build.js b/scripts/release/download-experimental-build.js
@@ -9,7 +9,6 @@ const {
   handleError,
 } = require('./utils');
 
-const checkEnvironmentVariables = require('./shared-commands/check-environment-variables');
 const downloadBuildArtifacts = require('./shared-commands/download-build-artifacts');
 const parseParams = require('./shared-commands/parse-params');
 const printSummary = require('./download-experimental-build-commands/print-summary');
@@ -26,7 +25,6 @@ const run = async () => {
     params.cwd = join(__dirname, '..', '..');
     params.packages = await getPublicPackages(true);
 
-    await checkEnvironmentVariables(params);
     await downloadBuildArtifacts(params);
 
     printSummary(params);