Skip to content

Commit 14530ef

Browse files
committed
fix(@angular/cli): batch Prettier invocations during migration formatting
`formatFiles` passed every changed file to Prettier as a single `execFile` argument list. On large repositories (thousands of changed files) the combined command line exceeds the OS limit and the spawn is rejected with `spawn E2BIG` (POSIX) or `spawn ENAMETOOLONG` (Windows). The error is caught and only logged, so `ng update` migrations print `WARNING: Formatting of files failed ...` and silently skip formatting on exactly the large repos the step targets. Split the file list into batches that stay under a conservative command-line length budget and invoke Prettier once per batch. Fixes #33597
1 parent a2346bf commit 14530ef

2 files changed

Lines changed: 143 additions & 7 deletions

File tree

packages/angular/cli/src/utilities/prettier.ts

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,50 @@ import { promisify } from 'node:util';
1515
const execFileAsync = promisify(execFile);
1616
let prettierCliPath: string | null | undefined;
1717

18+
/**
19+
* Conservative upper bound for the length of a single spawned command line.
20+
* Windows caps `CreateProcess` command lines at 32,767 characters; POSIX `ARG_MAX`
21+
* is larger. Staying below this budget avoids `E2BIG`/`ENAMETOOLONG` when a large
22+
* migration changes thousands of files.
23+
*/
24+
const MAX_COMMAND_LINE_LENGTH = 32_000;
25+
26+
/**
27+
* Groups files into batches whose combined argument length (including `baseLength`
28+
* for the fixed leading arguments) stays under `maxLength`. A file longer on its own
29+
* than the budget still gets its own batch, so files are never dropped.
30+
*/
31+
export function batchFilesByArgumentLength(
32+
files: Iterable<string>,
33+
baseLength: number,
34+
maxLength: number,
35+
): string[][] {
36+
const batches: string[][] = [];
37+
let batch: string[] = [];
38+
let length = baseLength;
39+
40+
for (const file of files) {
41+
// Account for the separator between arguments, plus the surrounding quotes the OS
42+
// adds to paths containing spaces when they are spawned.
43+
const fileLength = file.length + (file.includes(' ') ? 3 : 1);
44+
45+
if (batch.length > 0 && length + fileLength > maxLength) {
46+
batches.push(batch);
47+
batch = [];
48+
length = baseLength;
49+
}
50+
51+
batch.push(file);
52+
length += fileLength;
53+
}
54+
55+
if (batch.length > 0) {
56+
batches.push(batch);
57+
}
58+
59+
return batches;
60+
}
61+
1862
/**
1963
* Formats files using Prettier.
2064
* @param cwd The current working directory.
@@ -42,12 +86,37 @@ export async function formatFiles(cwd: string, files: Set<string>): Promise<void
4286
return;
4387
}
4488

45-
await execFileAsync(
46-
process.execPath,
47-
[prettierCliPath, '--write', '--no-error-on-unmatched-pattern', '--ignore-unknown', ...files],
48-
{
49-
cwd,
50-
shell: false,
51-
},
89+
const baseArgs = [
90+
prettierCliPath,
91+
'--write',
92+
'--no-error-on-unmatched-pattern',
93+
'--ignore-unknown',
94+
];
95+
// `process.execPath` is spawned as the first argument and also counts toward the
96+
// command-line limit; it and `prettierCliPath` are absolute paths that may contain
97+
// spaces and therefore be quoted.
98+
const baseLength = [process.execPath, ...baseArgs].reduce(
99+
(total, arg) => total + arg.length + (arg.includes(' ') ? 3 : 1),
100+
0,
52101
);
102+
103+
// Spawn Prettier once per batch so repositories with many changed files do not
104+
// overflow the OS command-line length limit. A failure in one batch (e.g. a file
105+
// Prettier cannot parse) must not stop the remaining batches, matching the previous
106+
// single-invocation behavior, so errors are collected and reported together.
107+
const errors: string[] = [];
108+
for (const batch of batchFilesByArgumentLength(files, baseLength, MAX_COMMAND_LINE_LENGTH)) {
109+
try {
110+
await execFileAsync(process.execPath, [...baseArgs, ...batch], {
111+
cwd,
112+
shell: false,
113+
});
114+
} catch (error) {
115+
errors.push(error instanceof Error ? error.message : String(error));
116+
}
117+
}
118+
119+
if (errors.length > 0) {
120+
throw new Error(errors.join('\n'));
121+
}
53122
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { batchFilesByArgumentLength } from './prettier';
10+
11+
describe('batchFilesByArgumentLength', () => {
12+
it('returns no batches when there are no files', () => {
13+
expect(batchFilesByArgumentLength([], 10, 100)).toEqual([]);
14+
});
15+
16+
it('keeps files that fit the budget in a single batch', () => {
17+
const files = ['a.ts', 'b.ts', 'c.ts'];
18+
19+
expect(batchFilesByArgumentLength(files, 0, 1000)).toEqual([files]);
20+
});
21+
22+
it('splits files into multiple batches when the budget is exceeded', () => {
23+
// Each file contributes `length + 1` => 5; a budget of 12 fits two files per batch.
24+
expect(batchFilesByArgumentLength(['aaaa', 'bbbb', 'cccc', 'dddd'], 0, 12)).toEqual([
25+
['aaaa', 'bbbb'],
26+
['cccc', 'dddd'],
27+
]);
28+
});
29+
30+
it('reserves the base argument length in every batch', () => {
31+
// base 8, each file => 5: 8 + 5 fits, 8 + 5 + 5 > 15 so the second file starts a new batch.
32+
expect(batchFilesByArgumentLength(['aaaa', 'bbbb'], 8, 15)).toEqual([['aaaa'], ['bbbb']]);
33+
});
34+
35+
it('reserves extra length for files that contain spaces (quoted when spawned)', () => {
36+
// 'a a' contains a space => length + 3 = 6; the others => length + 1 = 2. Budget 10:
37+
// 6 + 2 + 2 fits, adding the fourth file (12) exceeds it.
38+
expect(batchFilesByArgumentLength(['a a', 'b', 'c', 'd'], 0, 10)).toEqual([
39+
['a a', 'b', 'c'],
40+
['d'],
41+
]);
42+
});
43+
44+
it('never drops a file that is longer than the budget on its own', () => {
45+
const files = ['short', 'a-very-long-single-file-name-well-over-budget', 'tiny'];
46+
47+
const batches = batchFilesByArgumentLength(files, 0, 10);
48+
49+
expect(batches.flat()).toEqual(files);
50+
expect(batches).toContain(['a-very-long-single-file-name-well-over-budget']);
51+
});
52+
53+
it('keeps multi-file batches within the budget for a large file set', () => {
54+
const files = Array.from({ length: 5000 }, (_, index) => `path/to/file-${index}.component.ts`);
55+
const baseLength = 40;
56+
const maxLength = 2000;
57+
58+
for (const batch of batchFilesByArgumentLength(files, baseLength, maxLength)) {
59+
const length = batch.reduce((total, file) => total + file.length + 1, baseLength);
60+
61+
// A batch may exceed the budget only when it is a single unavoidable oversized file.
62+
if (batch.length > 1) {
63+
expect(length).toBeLessThanOrEqual(maxLength);
64+
}
65+
}
66+
});
67+
});

0 commit comments

Comments
 (0)