Skip to content

Commit f010a30

Browse files
EscapeBEscapeB
committed
fix: avoid unnecessary package manager install lock
1 parent 58f2846 commit f010a30

3 files changed

Lines changed: 164 additions & 32 deletions

File tree

‎common/changes/@microsoft/rush/package-manager-install-lock_2026-06-23-13-27.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/rush",
5+
"comment": "Avoid acquiring the global package manager install lock when the requested package manager is already installed and no install lock file exists.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "EscapeB@users.noreply.github.com"
11+
}

‎libraries/rush-lib/src/logic/installManager/InstallHelpers.ts‎

Lines changed: 80 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -298,47 +298,81 @@ export class InstallHelpers {
298298
node: process.versions.node
299299
});
300300

301+
if (
302+
(await packageManagerMarker.isValidAsync()) &&
303+
!InstallHelpers._doesPackageManagerInstallLockFileExist(rushUserFolder, packageManagerAndVersion)
304+
) {
305+
logIfConsoleOutputIsNotRestricted(
306+
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
307+
);
308+
InstallHelpers._ensureLocalPackageManagerSymlink(
309+
rushConfiguration,
310+
packageManager,
311+
packageManagerToolFolder,
312+
logIfConsoleOutputIsNotRestricted
313+
);
314+
return;
315+
}
316+
301317
logIfConsoleOutputIsNotRestricted(`Trying to acquire lock for ${packageManagerAndVersion}`);
302318

303319
const lock: LockFile = await LockFile.acquireAsync(rushUserFolder, packageManagerAndVersion);
304320

305321
logIfConsoleOutputIsNotRestricted(`Acquired lock for ${packageManagerAndVersion}`);
306322

307-
if (!(await packageManagerMarker.isValidAsync()) || lock.dirtyWhenAcquired) {
308-
logIfConsoleOutputIsNotRestricted(
309-
Colorize.bold(`Installing ${packageManager} version ${packageManagerVersion}\n`)
310-
);
323+
try {
324+
if (!(await packageManagerMarker.isValidAsync()) || lock.dirtyWhenAcquired) {
325+
logIfConsoleOutputIsNotRestricted(
326+
Colorize.bold(`Installing ${packageManager} version ${packageManagerVersion}\n`)
327+
);
328+
329+
// note that this will remove the last-install flag from the directory
330+
await Utilities.installPackageInDirectoryAsync({
331+
directory: packageManagerToolFolder,
332+
packageName: packageManager,
333+
version: rushConfiguration.packageManagerToolVersion,
334+
tempPackageTitle: `${packageManager}-local-install`,
335+
maxInstallAttempts: maxInstallAttempts,
336+
// This is using a local configuration to install a package in a shared global location.
337+
// Generally that's a bad practice, but in this case if we can successfully install
338+
// the package at all, we can reasonably assume it's good for all the repositories.
339+
// In particular, we'll assume that two different NPM registries cannot have two
340+
// different implementations of the same version of the same package.
341+
// This was needed for: https://github.com/microsoft/rushstack/issues/691
342+
commonRushConfigFolder: rushConfiguration.commonRushConfigFolder,
343+
// Only filter npm-incompatible properties when the repo uses pnpm or yarn.
344+
// If the repo uses npm, the .npmrc is already configured for npm, so don't filter.
345+
filterNpmIncompatibleProperties: rushConfiguration.packageManager !== 'npm'
346+
});
347+
348+
logIfConsoleOutputIsNotRestricted(
349+
`Successfully installed ${packageManager} version ${packageManagerVersion}`
350+
);
351+
} else {
352+
logIfConsoleOutputIsNotRestricted(
353+
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
354+
);
355+
}
311356

312-
// note that this will remove the last-install flag from the directory
313-
await Utilities.installPackageInDirectoryAsync({
314-
directory: packageManagerToolFolder,
315-
packageName: packageManager,
316-
version: rushConfiguration.packageManagerToolVersion,
317-
tempPackageTitle: `${packageManager}-local-install`,
318-
maxInstallAttempts: maxInstallAttempts,
319-
// This is using a local configuration to install a package in a shared global location.
320-
// Generally that's a bad practice, but in this case if we can successfully install
321-
// the package at all, we can reasonably assume it's good for all the repositories.
322-
// In particular, we'll assume that two different NPM registries cannot have two
323-
// different implementations of the same version of the same package.
324-
// This was needed for: https://github.com/microsoft/rushstack/issues/691
325-
commonRushConfigFolder: rushConfiguration.commonRushConfigFolder,
326-
// Only filter npm-incompatible properties when the repo uses pnpm or yarn.
327-
// If the repo uses npm, the .npmrc is already configured for npm, so don't filter.
328-
filterNpmIncompatibleProperties: rushConfiguration.packageManager !== 'npm'
329-
});
357+
await packageManagerMarker.createAsync();
330358

331-
logIfConsoleOutputIsNotRestricted(
332-
`Successfully installed ${packageManager} version ${packageManagerVersion}`
333-
);
334-
} else {
335-
logIfConsoleOutputIsNotRestricted(
336-
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
359+
InstallHelpers._ensureLocalPackageManagerSymlink(
360+
rushConfiguration,
361+
packageManager,
362+
packageManagerToolFolder,
363+
logIfConsoleOutputIsNotRestricted
337364
);
365+
} finally {
366+
lock.release();
338367
}
368+
}
339369

340-
await packageManagerMarker.createAsync();
341-
370+
private static _ensureLocalPackageManagerSymlink(
371+
rushConfiguration: RushConfiguration,
372+
packageManager: PackageManagerName,
373+
packageManagerToolFolder: string,
374+
logIfConsoleOutputIsNotRestricted: (message?: string) => void
375+
): void {
342376
// Example: "C:\MyRepo\common\temp"
343377
FileSystem.ensureFolder(rushConfiguration.commonTempFolder);
344378

@@ -365,8 +399,23 @@ export class InstallHelpers {
365399
linkTargetPath: packageManagerToolFolder,
366400
newLinkPath: localPackageManagerToolFolder
367401
});
402+
}
403+
404+
private static _doesPackageManagerInstallLockFileExist(
405+
rushUserFolder: string,
406+
packageManagerAndVersion: string
407+
): boolean {
408+
for (const itemName of FileSystem.readFolderItemNames(rushUserFolder)) {
409+
if (itemName === `${packageManagerAndVersion}.lock`) {
410+
return true;
411+
}
412+
413+
if (itemName.startsWith(`${packageManagerAndVersion}#`) && itemName.endsWith('.lock')) {
414+
return true;
415+
}
416+
}
368417

369-
lock.release();
418+
return false;
370419
}
371420

372421
// Helper for getPackageManagerEnvironment

‎libraries/rush-lib/src/logic/test/InstallHelpers.test.ts‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4-
import { type IPackageJson, JsonFile } from '@rushstack/node-core-library';
4+
import * as path from 'node:path';
5+
6+
import { FileSystem, type IPackageJson, JsonFile, LockFile } from '@rushstack/node-core-library';
57
import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal';
68
import { TestUtilities } from '@rushstack/heft-config-file';
79

810
import { InstallHelpers } from '../installManager/InstallHelpers';
911
import { RushConfiguration } from '../../api/RushConfiguration';
12+
import { LastInstallFlag } from '../../api/LastInstallFlag';
13+
import type { RushGlobalFolder } from '../../api/RushGlobalFolder';
14+
import { Utilities } from '../../utilities/Utilities';
1015

1116
describe('InstallHelpers', () => {
1217
describe('generateCommonPackageJson', () => {
@@ -73,4 +78,71 @@ describe('InstallHelpers', () => {
7378
);
7479
});
7580
});
81+
82+
describe(InstallHelpers.ensureLocalPackageManagerAsync.name, () => {
83+
const tempFolderPath: string = `${__dirname}/temp/${InstallHelpers.name}`;
84+
85+
beforeEach(() => {
86+
FileSystem.ensureEmptyFolder(tempFolderPath);
87+
});
88+
89+
afterEach(() => {
90+
FileSystem.deleteFolder(tempFolderPath);
91+
jest.restoreAllMocks();
92+
});
93+
94+
it('does not acquire the global lock when the package manager is already installed', async () => {
95+
const rushGlobalFolder: RushGlobalFolder = {
96+
path: `${tempFolderPath}/rush-global`,
97+
nodeSpecificPath: `${tempFolderPath}/rush-global/node-${process.version}`
98+
} as RushGlobalFolder;
99+
const packageManagerToolFolder: string = path.join(rushGlobalFolder.nodeSpecificPath, 'pnpm-10.27.0');
100+
await new LastInstallFlag(packageManagerToolFolder, { node: process.versions.node }).createAsync();
101+
102+
const lockAcquireSpy: jest.SpyInstance = jest.spyOn(LockFile, 'acquireAsync');
103+
const rushConfiguration: RushConfiguration = {
104+
commonRushConfigFolder: `${tempFolderPath}/common/config/rush`,
105+
commonTempFolder: `${tempFolderPath}/common/temp`,
106+
packageManager: 'pnpm',
107+
packageManagerToolVersion: '10.27.0'
108+
} as RushConfiguration;
109+
110+
await InstallHelpers.ensureLocalPackageManagerAsync(rushConfiguration, rushGlobalFolder, 1, true);
111+
112+
expect(lockAcquireSpy).not.toHaveBeenCalled();
113+
expect(FileSystem.exists(`${rushConfiguration.commonTempFolder}/pnpm-local`)).toEqual(true);
114+
});
115+
116+
it('acquires the global lock if an install lock file is present', async () => {
117+
const rushGlobalFolder: RushGlobalFolder = {
118+
path: `${tempFolderPath}/rush-global`,
119+
nodeSpecificPath: `${tempFolderPath}/rush-global/node-${process.version}`
120+
} as RushGlobalFolder;
121+
const packageManagerToolFolder: string = path.join(rushGlobalFolder.nodeSpecificPath, 'pnpm-10.27.0');
122+
await new LastInstallFlag(packageManagerToolFolder, { node: process.versions.node }).createAsync();
123+
FileSystem.writeFile(`${rushGlobalFolder.nodeSpecificPath}/pnpm-10.27.0#123.lock`, '');
124+
125+
const releaseAsync: jest.Mock = jest.fn();
126+
const lockAcquireSpy: jest.SpyInstance = jest.spyOn(LockFile, 'acquireAsync').mockResolvedValue({
127+
dirtyWhenAcquired: true,
128+
release: releaseAsync
129+
} as unknown as LockFile);
130+
const installSpy: jest.SpyInstance = jest
131+
.spyOn(Utilities, 'installPackageInDirectoryAsync')
132+
.mockResolvedValue();
133+
const rushConfiguration: RushConfiguration = {
134+
commonRushConfigFolder: `${tempFolderPath}/common/config/rush`,
135+
commonTempFolder: `${tempFolderPath}/common/temp`,
136+
packageManager: 'pnpm',
137+
packageManagerToolVersion: '10.27.0'
138+
} as RushConfiguration;
139+
140+
await InstallHelpers.ensureLocalPackageManagerAsync(rushConfiguration, rushGlobalFolder, 1, true);
141+
142+
expect(lockAcquireSpy).toHaveBeenCalledTimes(1);
143+
expect(installSpy).toHaveBeenCalledTimes(1);
144+
expect(releaseAsync).toHaveBeenCalledTimes(1);
145+
expect(FileSystem.exists(`${rushConfiguration.commonTempFolder}/pnpm-local`)).toEqual(true);
146+
});
147+
});
76148
});

0 commit comments

Comments
 (0)