Skip to content

Commit b936ab6

Browse files
authored
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
1 parent 2d02575 commit b936ab6

File tree

7 files changed

+126
-231
lines changed

7 files changed

+126
-231
lines changed

.circleci/config.yml

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -170,31 +170,57 @@ jobs:
170170
- *restore_node_modules
171171
- run: yarn build-combined
172172
- persist_to_workspace:
173-
root: build2
173+
root: .
174174
paths:
175-
- facebook-www
176-
- facebook-react-native
177-
- facebook-relay
178-
- oss-stable
179-
- oss-experimental
180-
- react-native
181-
- dist
182-
- sizes/*.json
175+
- build2
176+
177+
get_base_build:
178+
docker: *docker
179+
environment: *environment
180+
steps:
181+
- checkout
182+
- run: yarn workspaces info | head -n -1 > workspace_info.txt
183+
- *restore_node_modules
184+
- run:
185+
name: Download artifacts for base revision
186+
command: |
187+
git fetch origin master
188+
cd ./scripts/release && yarn && cd ../../
189+
scripts/release/download-experimental-build.js --commit=$(git merge-base HEAD origin/master)
190+
mv ./build2 ./base-build
191+
- persist_to_workspace:
192+
root: .
193+
paths:
194+
- base-build
183195

184196
process_artifacts_combined:
185197
docker: *docker
186198
environment: *environment
187199
steps:
188200
- checkout
189201
- attach_workspace:
190-
at: build2
202+
at: .
191203
- run: yarn workspaces info | head -n -1 > workspace_info.txt
192204
- *restore_node_modules
205+
- run: echo "<< pipeline.git.revision >>" >> build2/COMMIT_SHA
193206
# Compress build directory into a single tarball for easy download
194207
- run: tar -zcvf ./build2.tgz ./build2
195208
- store_artifacts:
196209
path: ./build2.tgz
197210

211+
sizebot:
212+
docker: *docker
213+
environment: *environment
214+
steps:
215+
- checkout
216+
- attach_workspace:
217+
at: .
218+
- run: echo "<< pipeline.git.revision >>" >> build2/COMMIT_SHA
219+
- run: yarn workspaces info | head -n -1 > workspace_info.txt
220+
- *restore_node_modules
221+
- run:
222+
command: node ./scripts/tasks/danger
223+
198224
build_devtools_and_process_artifacts:
199225
docker: *docker
200226
environment: *environment
@@ -261,38 +287,6 @@ jobs:
261287
process_artifacts: *process_artifacts
262288
process_artifacts_experimental: *process_artifacts
263289

264-
sizebot_stable:
265-
docker: *docker
266-
environment: *environment
267-
steps:
268-
- checkout
269-
- attach_workspace: *attach_workspace
270-
- run: yarn workspaces info | head -n -1 > workspace_info.txt
271-
- *restore_node_modules
272-
# This runs in the process_artifacts job, too, but it's faster to run
273-
# this step in both jobs instead of running the jobs sequentially
274-
- run: node ./scripts/rollup/consolidateBundleSizes.js
275-
- run:
276-
environment:
277-
RELEASE_CHANNEL: stable
278-
command: node ./scripts/tasks/danger
279-
280-
sizebot_experimental:
281-
docker: *docker
282-
environment: *environment
283-
steps:
284-
- checkout
285-
- attach_workspace: *attach_workspace
286-
- run: yarn workspaces info | head -n -1 > workspace_info.txt
287-
- *restore_node_modules
288-
# This runs in the process_artifacts job, too, but it's faster to run
289-
# this step in both jobs instead of running the jobs sequentially
290-
- run: node ./scripts/rollup/consolidateBundleSizes.js
291-
- run:
292-
environment:
293-
RELEASE_CHANNEL: experimental
294-
command: node ./scripts/tasks/danger
295-
296290
yarn_lint_build:
297291
docker: *docker
298292
environment: *environment
@@ -341,7 +335,7 @@ jobs:
341335
steps:
342336
- checkout
343337
- attach_workspace:
344-
at: build2
338+
at: .
345339
- run: yarn workspaces info | head -n -1 > workspace_info.txt
346340
- *restore_node_modules
347341
- run: yarn test --build <<parameters.args>> --ci
@@ -394,9 +388,6 @@ workflows:
394388
- process_artifacts:
395389
requires:
396390
- RELEASE_CHANNEL_stable_yarn_build
397-
- sizebot_stable:
398-
requires:
399-
- RELEASE_CHANNEL_stable_yarn_build
400391
- RELEASE_CHANNEL_stable_yarn_lint_build:
401392
requires:
402393
- RELEASE_CHANNEL_stable_yarn_build
@@ -413,9 +404,6 @@ workflows:
413404
- process_artifacts_experimental:
414405
requires:
415406
- yarn_build
416-
- sizebot_experimental:
417-
requires:
418-
- yarn_build
419407
- yarn_lint_build:
420408
requires:
421409
- yarn_build
@@ -495,6 +483,21 @@ workflows:
495483
# - "-r=www-modern --env=production --variant"
496484

497485
# TODO: Test more persistent configurations?
486+
- get_base_build:
487+
filters:
488+
branches:
489+
ignore:
490+
- master
491+
requires:
492+
- setup
493+
- sizebot:
494+
filters:
495+
branches:
496+
ignore:
497+
- master
498+
requires:
499+
- get_base_build
500+
- yarn_build_combined
498501
fuzz_tests:
499502
triggers:
500503
- schedule:

dangerfile.js

Lines changed: 66 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -26,34 +26,10 @@
2626
// `DANGER_GITHUB_API_TOKEN=[ENV_ABOVE] yarn danger pr https://github.com/facebook/react/pull/11865
2727

2828
const {markdown, danger, warn} = require('danger');
29-
const fetch = require('node-fetch');
3029

3130
const {generateResultsArray} = require('./scripts/rollup/stats');
32-
const {existsSync, readFileSync} = require('fs');
33-
const {exec} = require('child_process');
34-
35-
// This must match the name of the CI job that creates the build artifacts
36-
const RELEASE_CHANNEL =
37-
process.env.RELEASE_CHANNEL === 'experimental' ? 'experimental' : 'stable';
38-
const artifactsJobName =
39-
process.env.RELEASE_CHANNEL === 'experimental'
40-
? 'process_artifacts_experimental'
41-
: 'process_artifacts';
42-
43-
if (!existsSync('./build/bundle-sizes.json')) {
44-
// This indicates the build failed previously.
45-
// In that case, there's nothing for the Dangerfile to do.
46-
// Exit early to avoid leaving a redundant (and potentially confusing) PR comment.
47-
warn(
48-
'No bundle size information found. This indicates the build ' +
49-
'job failed.'
50-
);
51-
process.exit(0);
52-
}
53-
54-
const currentBuildResults = JSON.parse(
55-
readFileSync('./build/bundle-sizes.json')
56-
);
31+
const {readFileSync, readdirSync} = require('fs');
32+
const path = require('path');
5733

5834
/**
5935
* Generates a Markdown table
@@ -100,99 +76,23 @@ function setBoldness(row, isBold) {
10076
}
10177
}
10278

103-
/**
104-
* Gets the commit that represents the merge between the current branch
105-
* and master.
106-
*/
107-
function git(args) {
108-
return new Promise(res => {
109-
exec('git ' + args, (err, stdout, stderr) => {
110-
if (err) {
111-
throw err;
112-
} else {
113-
res(stdout.trim());
114-
}
115-
});
116-
});
117-
}
118-
119-
(async function() {
120-
// Use git locally to grab the commit which represents the place
121-
// where the branches differ
122-
const upstreamRepo = danger.github.pr.base.repo.full_name;
123-
if (upstreamRepo !== 'facebook/react') {
124-
// Exit unless we're running in the main repo
125-
return;
126-
}
127-
128-
markdown(`## Size changes (${RELEASE_CHANNEL})`);
129-
130-
const upstreamRef = danger.github.pr.base.ref;
131-
await git(`remote add upstream https://github.com/facebook/react.git`);
132-
await git('fetch upstream');
133-
const baseCommit = await git(`merge-base HEAD upstream/${upstreamRef}`);
134-
135-
let previousBuildResults = null;
136-
try {
137-
let baseCIBuildId = null;
138-
const statusesResponse = await fetch(
139-
`https://api.github.com/repos/facebook/react/commits/${baseCommit}/status`
140-
);
141-
const {statuses, state} = await statusesResponse.json();
142-
if (state === 'failure') {
143-
warn(`Base commit is broken: ${baseCommit}`);
144-
return;
145-
}
146-
for (let i = 0; i < statuses.length; i++) {
147-
const status = statuses[i];
148-
if (status.context === `ci/circleci: ${artifactsJobName}`) {
149-
if (status.state === 'success') {
150-
baseCIBuildId = /\/facebook\/react\/([0-9]+)/.exec(
151-
status.target_url
152-
)[1];
153-
break;
154-
}
155-
if (status.state === 'pending') {
156-
warn(`Build job for base commit is still pending: ${baseCommit}`);
157-
return;
158-
}
159-
}
79+
function getBundleSizes(pathToSizesDir) {
80+
const filenames = readdirSync(pathToSizesDir);
81+
let bundleSizes = [];
82+
for (let i = 0; i < filenames.length; i++) {
83+
const filename = filenames[i];
84+
if (filename.endsWith('.json')) {
85+
const json = readFileSync(path.join(pathToSizesDir, filename));
86+
bundleSizes.push(...JSON.parse(json).bundleSizes);
16087
}
161-
162-
if (baseCIBuildId === null) {
163-
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
164-
return;
165-
}
166-
167-
const baseArtifactsInfoResponse = await fetch(
168-
`https://circleci.com/api/v1.1/project/github/facebook/react/${baseCIBuildId}/artifacts`
169-
);
170-
const baseArtifactsInfo = await baseArtifactsInfoResponse.json();
171-
172-
for (let i = 0; i < baseArtifactsInfo.length; i++) {
173-
const info = baseArtifactsInfo[i];
174-
if (info.path.endsWith('bundle-sizes.json')) {
175-
const resultsResponse = await fetch(info.url);
176-
previousBuildResults = await resultsResponse.json();
177-
break;
178-
}
179-
}
180-
} catch (error) {
181-
warn(`Failed to fetch build artifacts for base commit: ${baseCommit}`);
182-
return;
183-
}
184-
185-
if (previousBuildResults === null) {
186-
warn(`Could not find build artifacts for base commit: ${baseCommit}`);
187-
return;
18888
}
89+
return {bundleSizes};
90+
}
18991

92+
async function printResultsForChannel(baseResults, headResults) {
19093
// Take the JSON of the build response and
19194
// make an array comparing the results for printing
192-
const results = generateResultsArray(
193-
currentBuildResults,
194-
previousBuildResults
195-
);
95+
const results = generateResultsArray(baseResults, headResults);
19696

19797
const packagesToShow = results
19898
.filter(
@@ -281,15 +181,62 @@ function git(args) {
281181
<details>
282182
<summary>Details of bundled changes.</summary>
283183
284-
<p>Comparing: ${baseCommit}...${danger.github.pr.head.sha}</p>
285-
286-
287184
${allTables.join('\n')}
288185
289186
</details>
290187
`;
291-
markdown(summary);
188+
return summary;
292189
} else {
293-
markdown('No significant bundle size changes to report.');
190+
return 'No significant bundle size changes to report.';
294191
}
192+
}
193+
194+
(async function() {
195+
// Use git locally to grab the commit which represents the place
196+
// where the branches differ
197+
198+
const upstreamRepo = danger.github.pr.base.repo.full_name;
199+
if (upstreamRepo !== 'facebook/react') {
200+
// Exit unless we're running in the main repo
201+
return;
202+
}
203+
204+
let headSha;
205+
let headSizesStable;
206+
let headSizesExperimental;
207+
208+
let baseSha;
209+
let baseSizesStable;
210+
let baseSizesExperimental;
211+
212+
try {
213+
headSha = (readFileSync('./build2/COMMIT_SHA') + '').trim();
214+
headSizesStable = getBundleSizes('./build2/sizes-stable');
215+
headSizesExperimental = getBundleSizes('./build2/sizes-experimental');
216+
217+
baseSha = (readFileSync('./base-build/COMMIT_SHA') + '').trim();
218+
baseSizesStable = getBundleSizes('./base-build/sizes-stable');
219+
baseSizesExperimental = getBundleSizes('./base-build/sizes-experimental');
220+
} catch {
221+
warn(
222+
"Failed to read build artifacts. It's possible a build configuration " +
223+
'has changed upstream. Try pulling the latest changes from the ' +
224+
'main branch.'
225+
);
226+
return;
227+
}
228+
229+
markdown(`
230+
## Size changes
231+
232+
<p>Comparing: ${baseSha}...${headSha}</p>
233+
234+
### Stable channel
235+
236+
${await printResultsForChannel(baseSizesStable, headSizesStable)}
237+
238+
### Experimental channel
239+
240+
${await printResultsForChannel(baseSizesExperimental, headSizesExperimental)}
241+
`);
295242
})();

scripts/release/download-experimental-build.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const {
99
handleError,
1010
} = require('./utils');
1111

12-
const checkEnvironmentVariables = require('./shared-commands/check-environment-variables');
1312
const downloadBuildArtifacts = require('./shared-commands/download-build-artifacts');
1413
const parseParams = require('./shared-commands/parse-params');
1514
const printSummary = require('./download-experimental-build-commands/print-summary');
@@ -26,7 +25,6 @@ const run = async () => {
2625
params.cwd = join(__dirname, '..', '..');
2726
params.packages = await getPublicPackages(true);
2827

29-
await checkEnvironmentVariables(params);
3028
await downloadBuildArtifacts(params);
3129

3230
printSummary(params);

0 commit comments

Comments
 (0)