Skip to content

Commit 8587eab

Browse files
[rush] Use AsyncRecycler for autoinstaller cleanup (#5756)
* Use AsyncRecycler for autoinstaller cleanup * Restore AsyncRecycler type annotation * Apply suggestions from code review - Use string interpolation instead of path.join - Avoid race condition from checking if file exists before operating on it Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com> * fix(Autoinstaller): fix conflict --------- Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
1 parent 1181b8e commit 8587eab

3 files changed

Lines changed: 121 additions & 1 deletion

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Move stale autoinstaller `node_modules` folders into Rush's recycler before asynchronously deleting them, instead of synchronously deleting them in place.",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@microsoft/rush"
10+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import './mockRushCommandLineParser';
5+
6+
import { FileSystem } from '@rushstack/node-core-library';
7+
8+
import { Autoinstaller } from '../../logic/Autoinstaller';
9+
import { InstallHelpers } from '../../logic/installManager/InstallHelpers';
10+
import { RushConstants } from '../../logic/RushConstants';
11+
import { Utilities } from '../../utilities/Utilities';
12+
import {
13+
getCommandLineParserInstanceAsync,
14+
isolateEnvironmentConfigurationForTests,
15+
type IEnvironmentConfigIsolation
16+
} from './TestUtils';
17+
18+
describe(Autoinstaller.name, () => {
19+
let _envIsolation: IEnvironmentConfigIsolation;
20+
21+
beforeEach(() => {
22+
_envIsolation = isolateEnvironmentConfigurationForTests();
23+
});
24+
25+
afterEach(() => {
26+
_envIsolation.restore();
27+
jest.restoreAllMocks();
28+
});
29+
30+
it('moves an existing node_modules folder into the Rush recycler before reinstalling', async () => {
31+
const { parser, repoPath, spawnMock } = await getCommandLineParserInstanceAsync(
32+
'pluginWithBuildCommandRepo',
33+
'update'
34+
);
35+
const autoinstallerPath: string = `${repoPath}/common/autoinstallers/plugins`;
36+
const nodeModulesFolder: string = `${autoinstallerPath}/${RushConstants.nodeModulesFolderName}`;
37+
const staleFilePath: string = `${nodeModulesFolder}/stale-package/index.js`;
38+
const recyclerFolder: string = `${parser.rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}`;
39+
40+
await FileSystem.writeFileAsync(staleFilePath, 'stale', {
41+
ensureFolderExists: true
42+
});
43+
44+
let recyclerEntriesBefore: Set<string>;
45+
try {
46+
recyclerEntriesBefore = new Set(await FileSystem.readFolderItemNamesAsync(recyclerFolder));
47+
} catch (error) {
48+
if (FileSystem.isNotExistError(error)) {
49+
recyclerEntriesBefore = new Set();
50+
} else {
51+
throw error;
52+
}
53+
}
54+
55+
jest.spyOn(InstallHelpers, 'ensureLocalPackageManagerAsync').mockResolvedValue(undefined);
56+
jest.spyOn(Utilities, 'syncNpmrc').mockImplementation(() => undefined);
57+
jest
58+
.spyOn(Utilities, 'executeCommandAsync')
59+
.mockImplementation(async (options: Parameters<typeof Utilities.executeCommandAsync>[0]) => {
60+
await FileSystem.ensureFolderAsync(
61+
`${options.workingDirectory}/${RushConstants.nodeModulesFolderName}`
62+
);
63+
});
64+
65+
const autoinstaller: Autoinstaller = new Autoinstaller({
66+
autoinstallerName: 'plugins',
67+
rushConfiguration: parser.rushConfiguration,
68+
rushGlobalFolder: parser.rushGlobalFolder
69+
});
70+
71+
await autoinstaller.prepareAsync();
72+
73+
const recyclerEntriesAfter: string[] = (await FileSystem.readFolderItemNamesAsync(recyclerFolder)).filter(
74+
(entry: string) => !recyclerEntriesBefore.has(entry)
75+
);
76+
77+
expect(recyclerEntriesAfter).toHaveLength(1);
78+
await expect(
79+
FileSystem.existsAsync(`${recyclerFolder}/${recyclerEntriesAfter[0]}/stale-package/index.js`)
80+
).resolves.toBe(true);
81+
await expect(FileSystem.existsAsync(staleFilePath)).resolves.toBe(false);
82+
await expect(FileSystem.existsAsync(`${nodeModulesFolder}/rush-autoinstaller.flag`)).resolves.toBe(true);
83+
84+
if (process.platform === 'win32') {
85+
expect(spawnMock).toHaveBeenCalledWith(
86+
'cmd.exe',
87+
expect.arrayContaining(['/c']),
88+
expect.objectContaining({
89+
detached: true,
90+
stdio: 'ignore',
91+
windowsVerbatimArguments: true
92+
})
93+
);
94+
} else {
95+
expect(spawnMock).toHaveBeenCalledWith(
96+
'rm',
97+
expect.arrayContaining(['-rf']),
98+
expect.objectContaining({
99+
detached: true,
100+
stdio: 'ignore'
101+
})
102+
);
103+
}
104+
});
105+
});

libraries/rush-lib/src/logic/Autoinstaller.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '@rushstack/node-core-library';
1515
import { Colorize } from '@rushstack/terminal';
1616

17+
import { AsyncRecycler } from '../utilities/AsyncRecycler';
1718
import { Utilities } from '../utilities/Utilities';
1819
import type { RushConfiguration } from '../api/RushConfiguration';
1920
import { PackageJsonEditor } from '../api/PackageJsonEditor';
@@ -131,7 +132,11 @@ export class Autoinstaller {
131132
if (isLastInstallFlagDirty || lock.dirtyWhenAcquired) {
132133
if (FileSystem.exists(nodeModulesFolder)) {
133134
this._logIfConsoleOutputIsNotRestricted('Deleting old files from ' + nodeModulesFolder);
134-
FileSystem.ensureEmptyFolder(nodeModulesFolder);
135+
const recycler: AsyncRecycler = new AsyncRecycler(
136+
`${this._rushConfiguration.commonTempFolder}/${RushConstants.rushRecyclerFolderName}`
137+
);
138+
recycler.moveFolder(nodeModulesFolder);
139+
await recycler.startDeleteAllAsync();
135140
}
136141

137142
// Copy: .../common/autoinstallers/my-task/.npmrc

0 commit comments

Comments
 (0)