Skip to content

Commit 7512ea5

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 7512ea5

2 files changed

Lines changed: 131 additions & 6 deletions

File tree

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

Lines changed: 64 additions & 6 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,26 @@ 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-
{
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,
101+
);
102+
103+
// Spawn Prettier once per batch so repositories with many changed files do not
104+
// overflow the OS command-line length limit.
105+
for (const batch of batchFilesByArgumentLength(files, baseLength, MAX_COMMAND_LINE_LENGTH)) {
106+
await execFileAsync(process.execPath, [...baseArgs, ...batch], {
49107
cwd,
50108
shell: false,
51-
},
52-
);
109+
});
110+
}
53111
}
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)