Skip to content

Commit ee66ae8

Browse files
alexander-akaitalexander-akait
authored
chore: make yarn setup work unattended, for CI and coding agents (#21989)
* chore: make yarn setup work unattended, for CI and coding agents A fresh sandbox clones the repo with no node_modules, so the first tests or linters cannot run until yarn install finishes. `yarn setup` could not be that command unattended: it installs a global yarn, writes to yarn's machine-global link registry, and rewrites the lockfile rather than verifying it. It now picks a path instead. A contributor at a terminal gets exactly what they got before, short-circuit included, so `prelint` is unchanged. Anything else installs against a frozen lockfile and links the checkout in as node_modules/webpack directly, which config cases and examples need to resolve bare "webpack". What it reads is a TTY plus an unset CI, never a list of vendor variables: such a list is wrong the day another agent ships, and wrong silently, since the one it forgot would run the interactive path. WEBPACK_SETUP=interactive|automated forces either. * fix: take the safe setup path on any defined CI, and say what is missing `!process.env.CI` is true for `CI=""`, so a terminal with an explicitly empty CI ran the contributor path — the one that installs a global yarn and writes to yarn's link registry. Any defined value now selects automation, since that is the side a wrong guess is cheap on. That path installs nothing globally, so without yarn it died on exit code 127 from the shell. It now says what is missing and which override installs it, and the yarn check it needs is shared with the contributor path rather than duplicated. The docs claimed every coding agent takes the automated path. One that allocates a PTY does not, and reads as a contributor; both files now state the predicate and name the override. Reported by CodeRabbit on #21989. * test(setup): cover which path setup picks The mode selection is the whole of this change and nothing exercised it, so a regression in it would surface as a contributor's machine getting a global yarn install rather than as a red suite. `setup.js` exports the predicate and runs itself only as the entry point, the way `check-comment-length.js` already does, and the case covers both forced modes, an unknown forced value, each stream being a terminal alone, and CI unset, empty and set. Reverting the empty-CI fix fails it. The paths that spawn a process are left to the manual check: what they do depends on what is on PATH, which a unit test would have to fake rather than observe. Reported by CodeRabbit on #21989. * test(setup): cover the paths, not just the choice between them Covering only the predicate made the patch metric worse than covering nothing: until a test required setup.js the file was absent from the report, so its lines were not coverable, and pulling it in left 19% of them hit. Codecov read that as 23.80% of the diff. The cases now drive the dispatcher with a stubbed child process, which is where the contract this change exists for can be asserted: the automated path runs yarn against a frozen lockfile and never npm install -g, the contributor path links through the registry, an existing link skips both, and a missing yarn stops before installing. That takes setup.js from 19% to 92% of lines. Note that nothing under test/ is transformed, so jest.mock is not hoisted and is registered above the requires rather than below them. * fix(setup): return the exit code instead of setting it The bun job runs the unit suites too, and there setup() assigning process.exitCode did not read back, so six cases failed on a runtime the Node ones pass. Asserting on global process state was the mistake: setup() now returns the code and only the entry point assigns it, which is both testable and honest about what the function does. The effects it reaches for are read from one object rather than from the imports, because bun's jest cannot mock a built-in module at all — jest.mock on child_process throws "Attempted to assign to readonly property" before a single case runs, and jest.spyOn on fs silently records nothing. Reproduced by applying test/patches the way the bun job does: the pushed commit fails 6 of 13 there and passes 13 of 13 with this.
1 parent 72e3e43 commit ee66ae8

4 files changed

Lines changed: 326 additions & 40 deletions

File tree

‎AGENTS.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ All commands are defined in `package.json` `scripts`.
2626
| Command | What it does |
2727
| -------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------- |
2828
| `yarn fix` | `fix:code` (ESLint) + `fix:special` (regenerate types/validators) + `fmt` (Prettier). Prefer as the final step. |
29+
| `yarn setup` | Install dependencies and link the checkout in as `webpack`; non-interactive off a TTY. |
2930
| `yarn fix:special` | Regenerate `types.d.ts`, declarations, schema validators, and generated runtime code. |
3031
| `yarn lint` | Full lint: ESLint + generated-output checks + every `tsc` project + Prettier + spellcheck (what CI runs). |
3132
| `yarn tsc` | TypeScript type check of `lib/` JSDoc (catches type errors in annotations). |
@@ -93,7 +94,7 @@ The directory listings below are the canonical map of the repository. **Whenever
9394
- `bin/` — `webpack` CLI entry point.
9495
- `tooling/` — Repo-internal scripts: build/codegen (runtime/wasm generators, hash-debug tool) invoked by `yarn fix:special`, plus standalone analysis tools such as `compare-css-minifiers.js` / `compare-html-minifiers.js` (`yarn benchmark:css-minifiers`, `yarn benchmark:html-minifiers`). Those two need no arguments and no reading of their source: each runs webpack's CSS/HTML minifier and the ecosystem's over popular framework stylesheets and real documents, printing one table per fixture — output size raw and under gzip/brotli/zstd (the `test:size` settings), best-of-3 wall and cpu ms, peak RSS (each minifier × fixture measured in its own worker process, so the numbers are attributable), and whether the output lost classes / changed the DOM ("rejects it" rows mean the tool errored on that input). They install the packages they compare against into `node_modules/.cache/` on first run rather than into webpack's dependencies; expect the first run to install for a minute and every full run to take a few. `check-comment-length.js` (`yarn lint:comments`) reports the plain comments a change grows past the two-line limit of [Code comments](#code-comments). `measure-color-agreement.js` (`yarn measure:color-agreement`) is the third: it asks a real browser for its own color conversions rather than for a pixel, and prints how far they sit from webpack's — which is where the rounding margins in `lib/css/syntax.js` and the list of spaces an engine reads through another transfer come from. Re-run it (a few seconds; `PUPPETEER_EXECUTABLE_PATH` picks the binary) rather than adjusting either by hand.
9596
- `assembly/` — WebAssembly source for the hash function.
96-
- `setup/` — One-time setup scripts.
97+
- `setup/` — One-time setup. `setup.js` (`yarn setup`) is the only entry point and picks its own path: a contributor at a terminal gets the interactive one, which installs yarn when it is missing and links through yarn's registry, while everything else gets the non-interactive one — it verifies the lockfile instead of rewriting it, installs no global yarn, and links the checkout in as `node_modules/webpack` without touching yarn's machine-global registry. What it reads is both streams being a terminal plus an unset `CI`, never a list of vendor variables, so an agent nobody has heard of yet takes the safe path — but one that allocates a PTY reads as a contributor, and should set `WEBPACK_SETUP=automated`, which with `interactive` forces either path. Safe to re-run.
9798

9899
**Schemas (the source of truth for webpack's config API)**
99100

‎_SETUP.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ If not, do: `npm run setup`, the setup will also install `yarn` for you.
88

99
That's all.
1010

11+
## Automated environments
12+
13+
`yarn setup` is all any environment needs. Off a terminal — CI, a throwaway sandbox, any coding agent — it switches itself to a non-interactive path: it verifies `yarn.lock` rather than rewriting it, installs no global `yarn`, and links the checkout in as `node_modules/webpack` without touching yarn's machine-global link registry. Safe to re-run either way.
14+
15+
The path is chosen from whether both streams are a terminal and `CI` is unset. An agent that allocates a PTY therefore reads as a contributor and should set `WEBPACK_SETUP=automated`; `WEBPACK_SETUP=interactive` forces the other way. The interactive path is also the one that installs `yarn` when it is missing, so `npm run setup` still does that from a terminal.
16+
1117
## Setup manually
1218

1319
### Setup your local webpack repository

‎setup/setup.js‎

Lines changed: 94 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,88 @@
11
"use strict";
22

3+
const childProcess = require("child_process");
34
const fs = require("fs");
45
const path = require("path");
56

7+
// A seam for the tests: Bun's jest cannot mock a built-in module, so the
8+
// effects are reached through this object rather than through the imports.
9+
const effects = {
10+
spawn: childProcess.spawn,
11+
existsSync: fs.existsSync,
12+
lstatSync: fs.lstatSync,
13+
symlinkSync: fs.symlinkSync
14+
};
15+
616
const root = process.cwd();
717
const nodeModulesFolder = path.resolve(root, "node_modules");
818
const webpackDependencyFolder = path.resolve(root, "node_modules/webpack");
919

1020
/**
11-
* @returns {Promise<void>} result
21+
* Whether a person is watching: both streams a terminal, and `CI` unset. Any
22+
* other environment, including one this predates, takes the safe path.
23+
* @returns {boolean} result
1224
*/
13-
function setup() {
14-
return checkSymlinkExistsAsync()
15-
.then(async (hasSymlink) => {
16-
if (!hasSymlink) {
17-
await ensureYarnInstalledAsync();
18-
await runSetupSymlinkAsync();
19-
if (!(await checkSymlinkExistsAsync())) {
20-
throw new Error("windows symlink was not successfully created");
21-
}
22-
}
23-
})
24-
.then(() => {
25-
process.exitCode = 0;
26-
})
27-
.catch((err) => {
28-
console.error(err);
29-
process.exitCode = 1;
30-
});
25+
function isInteractive() {
26+
const forced = process.env.WEBPACK_SETUP;
27+
if (forced === "interactive") return true;
28+
if (forced === "automated") return false;
29+
return (
30+
Boolean(process.stdin.isTTY && process.stdout.isTTY) &&
31+
process.env.CI === undefined
32+
);
33+
}
34+
35+
/**
36+
* Returns the exit code rather than setting it, so a caller can observe the
37+
* outcome without reading global state.
38+
* @returns {Promise<number>} the exit code
39+
*/
40+
async function setup() {
41+
try {
42+
await (isInteractive() ? setupForContributor() : setupForAutomation());
43+
return 0;
44+
} catch (err) {
45+
console.error(err);
46+
return 1;
47+
}
3148
}
3249

3350
/**
51+
* Installs yarn when it is missing and links through yarn's registry, so the
52+
* checkout can be linked into another project. Skipped once the link exists.
3453
* @returns {Promise<void>} result
3554
*/
36-
async function runSetupSymlinkAsync() {
55+
async function setupForContributor() {
56+
if (await checkSymlinkExistsAsync()) return;
57+
await ensureYarnInstalledAsync();
3758
await exec("yarn", ["install"], "Install dependencies");
3859
await exec("yarn", ["link"], "Create webpack symlink");
3960
await exec("yarn", ["link", "webpack"], "Link webpack into itself");
61+
if (!(await checkSymlinkExistsAsync())) {
62+
throw new Error("windows symlink was not successfully created");
63+
}
64+
}
65+
66+
/**
67+
* Verifies the lockfile rather than rewriting it, installs no global yarn, and
68+
* links without touching yarn's machine-global registry. Safe to re-run.
69+
* @returns {Promise<void>} result
70+
*/
71+
async function setupForAutomation() {
72+
if (!(await hasYarnAsync())) {
73+
throw new Error(
74+
"yarn was not found, and this path installs nothing globally. Install yarn, or re-run with WEBPACK_SETUP=interactive to have setup install it."
75+
);
76+
}
77+
await exec("yarn", ["install", "--frozen-lockfile"], "Install dependencies");
78+
if (await checkSymlinkExistsAsync()) return;
79+
console.log("Setup: Link webpack into itself");
80+
const isWindows = process.platform === "win32";
81+
effects.symlinkSync(
82+
isWindows ? root : "..",
83+
webpackDependencyFolder,
84+
isWindows ? "junction" : "dir"
85+
);
4086
}
4187

4288
/**
@@ -45,35 +91,35 @@ async function runSetupSymlinkAsync() {
4591
function checkSymlinkExistsAsync() {
4692
return new Promise((resolve) => {
4793
try {
48-
if (
49-
fs.existsSync(nodeModulesFolder) &&
50-
fs.existsSync(webpackDependencyFolder) &&
51-
fs.lstatSync(webpackDependencyFolder).isSymbolicLink()
52-
) {
53-
resolve(true);
54-
} else {
55-
resolve(false);
56-
}
94+
resolve(
95+
effects.existsSync(nodeModulesFolder) &&
96+
effects.lstatSync(webpackDependencyFolder).isSymbolicLink()
97+
);
5798
} catch {
5899
resolve(false);
59100
}
60101
});
61102
}
62103

63104
/**
64-
* @returns {Promise<void>} result
105+
* @returns {Promise<boolean>} result
65106
*/
66-
async function ensureYarnInstalledAsync() {
107+
async function hasYarnAsync() {
67108
const semverPattern =
68109
/^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-(?:0|[1-9]\d*|\d*[a-z-][0-9a-z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-z-][0-9a-z-]*))*)?(?:\+[0-9a-z-]+(?:\.[0-9a-z-]+)*)?$/i;
69-
let hasYarn = false;
70110
try {
71111
const stdout = await execGetOutput("yarn", ["-v"], "Check yarn version");
72-
hasYarn = semverPattern.test(stdout);
112+
return semverPattern.test(stdout);
73113
} catch (_err) {
74-
hasYarn = false;
114+
return false;
75115
}
76-
if (!hasYarn) await installYarnAsync();
116+
}
117+
118+
/**
119+
* @returns {Promise<void>} result
120+
*/
121+
async function ensureYarnInstalledAsync() {
122+
if (!(await hasYarnAsync())) await installYarnAsync();
77123
}
78124

79125
/**
@@ -92,7 +138,7 @@ function installYarnAsync() {
92138
function exec(command, args, description) {
93139
console.log(`Setup: ${description}`);
94140
return new Promise((resolve, reject) => {
95-
const cp = require("child_process").spawn(command, args, {
141+
const cp = effects.spawn(command, args, {
96142
cwd: root,
97143
stdio: "inherit",
98144
shell: true
@@ -120,7 +166,7 @@ function exec(command, args, description) {
120166
function execGetOutput(command, args, description) {
121167
console.log(`Setup: ${description}`);
122168
return new Promise((resolve, reject) => {
123-
const cp = require("child_process").spawn(command, args, {
169+
const cp = effects.spawn(command, args, {
124170
cwd: root,
125171
stdio: [process.stdin, "pipe", process.stderr],
126172
shell: true
@@ -138,8 +184,17 @@ function execGetOutput(command, args, description) {
138184
});
139185
/** @type {Buffer[]} */
140186
const buffers = [];
141-
cp.stdout.on("data", (data) => buffers.push(data));
187+
/** @type {import("stream").Readable} */
188+
(cp.stdout).on("data", (data) => buffers.push(data));
142189
});
143190
}
144191

145-
setup();
192+
module.exports.effects = effects;
193+
module.exports.isInteractive = isInteractive;
194+
module.exports.setup = setup;
195+
196+
if (require.main === module) {
197+
setup().then((code) => {
198+
process.exitCode = code;
199+
});
200+
}

0 commit comments

Comments
 (0)