Skip to content

Commit

Permalink
Update sizebot to new workflow (#20719)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@acdlite
acdlite authored Feb 3, 2021
1 parent 2d02575 commit b936ab6
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 231 deletions.
101 changes: 52 additions & 49 deletions .circleci/config.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -413,9 +404,6 @@ workflows:
- process_artifacts_experimental:
requires:
- yarn_build
- sizebot_experimental:
requires:
- yarn_build
- yarn_lint_build:
requires:
- yarn_build
Expand Down Expand Up @@ -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:
Expand Down
185 changes: 66 additions & 119 deletions dangerfile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)}
`);
})();
2 changes: 0 additions & 2 deletions scripts/release/download-experimental-build.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -26,7 +25,6 @@ const run = async () => {
params.cwd = join(__dirname, '..', '..');
params.packages = await getPublicPackages(true);

await checkEnvironmentVariables(params);
await downloadBuildArtifacts(params);

printSummary(params);
Expand Down
Loading

0 comments on commit b936ab6

Please sign in to comment.