Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Avoid acquiring the global package manager install lock when the requested package manager is already installed and no install lock file exists.",
"type": "patch"
}
],
"packageName": "@microsoft/rush",
"email": "EscapeB@users.noreply.github.com"
}
111 changes: 80 additions & 31 deletions libraries/rush-lib/src/logic/installManager/InstallHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,47 +298,81 @@ export class InstallHelpers {
node: process.versions.node
});

if (
(await packageManagerMarker.isValidAsync()) &&
!InstallHelpers._doesPackageManagerInstallLockFileExist(rushUserFolder, packageManagerAndVersion)
) {
logIfConsoleOutputIsNotRestricted(
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
);
InstallHelpers._ensureLocalPackageManagerSymlink(
rushConfiguration,
packageManager,
packageManagerToolFolder,
logIfConsoleOutputIsNotRestricted
);
return;
}

logIfConsoleOutputIsNotRestricted(`Trying to acquire lock for ${packageManagerAndVersion}`);

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

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

if (!(await packageManagerMarker.isValidAsync()) || lock.dirtyWhenAcquired) {
logIfConsoleOutputIsNotRestricted(
Colorize.bold(`Installing ${packageManager} version ${packageManagerVersion}\n`)
);
try {
if (!(await packageManagerMarker.isValidAsync()) || lock.dirtyWhenAcquired) {
logIfConsoleOutputIsNotRestricted(
Colorize.bold(`Installing ${packageManager} version ${packageManagerVersion}\n`)
);

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

logIfConsoleOutputIsNotRestricted(
`Successfully installed ${packageManager} version ${packageManagerVersion}`
);
} else {
logIfConsoleOutputIsNotRestricted(
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
);
}

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

logIfConsoleOutputIsNotRestricted(
`Successfully installed ${packageManager} version ${packageManagerVersion}`
);
} else {
logIfConsoleOutputIsNotRestricted(
`Found ${packageManager} version ${packageManagerVersion} in ${packageManagerToolFolder}`
InstallHelpers._ensureLocalPackageManagerSymlink(
rushConfiguration,
packageManager,
packageManagerToolFolder,
logIfConsoleOutputIsNotRestricted
);
} finally {
lock.release();
}
}

await packageManagerMarker.createAsync();

private static _ensureLocalPackageManagerSymlink(
rushConfiguration: RushConfiguration,
packageManager: PackageManagerName,
packageManagerToolFolder: string,
logIfConsoleOutputIsNotRestricted: (message?: string) => void
): void {
// Example: "C:\MyRepo\common\temp"
FileSystem.ensureFolder(rushConfiguration.commonTempFolder);

Expand All @@ -365,8 +399,23 @@ export class InstallHelpers {
linkTargetPath: packageManagerToolFolder,
newLinkPath: localPackageManagerToolFolder
});
}

private static _doesPackageManagerInstallLockFileExist(
rushUserFolder: string,
packageManagerAndVersion: string
): boolean {
for (const itemName of FileSystem.readFolderItemNames(rushUserFolder)) {
if (itemName === `${packageManagerAndVersion}.lock`) {
return true;
}

if (itemName.startsWith(`${packageManagerAndVersion}#`) && itemName.endsWith('.lock')) {
return true;
}
}

lock.release();
return false;
}

// Helper for getPackageManagerEnvironment
Expand Down
120 changes: 119 additions & 1 deletion libraries/rush-lib/src/logic/test/InstallHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { type IPackageJson, JsonFile } from '@rushstack/node-core-library';
import * as path from 'node:path';

import { FileSystem, type IPackageJson, JsonFile, LockFile } from '@rushstack/node-core-library';
import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal';
import { TestUtilities } from '@rushstack/heft-config-file';

import { InstallHelpers } from '../installManager/InstallHelpers';
import { RushConfiguration } from '../../api/RushConfiguration';
import { LastInstallFlag } from '../../api/LastInstallFlag';
import type { RushGlobalFolder } from '../../api/RushGlobalFolder';
import { Utilities } from '../../utilities/Utilities';

describe('InstallHelpers', () => {
describe('generateCommonPackageJson', () => {
Expand Down Expand Up @@ -73,4 +78,117 @@ describe('InstallHelpers', () => {
);
});
});

describe(InstallHelpers.ensureLocalPackageManagerAsync.name, () => {
const tempFolderPath: string = `${__dirname}/temp/${InstallHelpers.name}`;
const packageManager: 'pnpm' = 'pnpm';
const packageManagerVersion: string = '10.27.0';

function getRushGlobalFolder(): RushGlobalFolder {
return {
path: `${tempFolderPath}/rush-global`,
nodeSpecificPath: `${tempFolderPath}/rush-global/node-${process.version}`
} as RushGlobalFolder;
}

function getRushConfiguration(): RushConfiguration {
return {
commonRushConfigFolder: `${tempFolderPath}/common/config/rush`,
commonTempFolder: `${tempFolderPath}/common/temp`,
packageManager,
packageManagerToolVersion: packageManagerVersion
} as RushConfiguration;
}

function getPackageManagerToolFolder(rushGlobalFolder: RushGlobalFolder): string {
return path.join(rushGlobalFolder.nodeSpecificPath, `${packageManager}-${packageManagerVersion}`);
}

async function writeInstalledPackageManagerAsync(rushGlobalFolder: RushGlobalFolder): Promise<void> {
const packageManagerToolFolder: string = getPackageManagerToolFolder(rushGlobalFolder);

JsonFile.save(
{
dependencies: {
[packageManager]: packageManagerVersion
},
description: 'Temporary file generated by the Rush tool',
name: `${packageManager}-local-install`,
private: true,
version: '0.0.0'
},
path.join(packageManagerToolFolder, 'package.json'),
{ ensureFolderExists: true }
);

JsonFile.save(
{
name: packageManager,
version: packageManagerVersion
},
path.join(packageManagerToolFolder, 'node_modules', packageManager, 'package.json'),
{ ensureFolderExists: true }
);

const packageManagerBinFolder: string = path.join(packageManagerToolFolder, 'node_modules', '.bin');
FileSystem.ensureFolder(packageManagerBinFolder);
FileSystem.writeFile(path.join(packageManagerBinFolder, packageManager), '');

await new LastInstallFlag(packageManagerToolFolder, { node: process.versions.node }).createAsync();
}

beforeEach(() => {
FileSystem.ensureEmptyFolder(tempFolderPath);
});

afterEach(() => {
FileSystem.deleteFolder(tempFolderPath);
jest.restoreAllMocks();
});

it('does not acquire the global lock when the package manager is already installed', async () => {
const rushGlobalFolder: RushGlobalFolder = getRushGlobalFolder();
await writeInstalledPackageManagerAsync(rushGlobalFolder);

const lockAcquireSpy: jest.SpyInstance = jest
.spyOn(LockFile, 'acquireAsync')
.mockResolvedValue(false as unknown as LockFile);
const installSpy: jest.SpyInstance = jest
.spyOn(Utilities, 'installPackageInDirectoryAsync')
.mockRejectedValue(new Error('The package manager should already be installed.'));
const rushConfiguration: RushConfiguration = getRushConfiguration();

await InstallHelpers.ensureLocalPackageManagerAsync(rushConfiguration, rushGlobalFolder, 1, true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the lockAcquireSpy returns false so that the test doesn't try to actually install the package if something goes wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated


expect(lockAcquireSpy).not.toHaveBeenCalled();
expect(installSpy).not.toHaveBeenCalled();
expect(FileSystem.exists(`${rushConfiguration.commonTempFolder}/pnpm-local`)).toEqual(true);
});

it('acquires the global lock if an install lock file is present', async () => {
const rushGlobalFolder: RushGlobalFolder = getRushGlobalFolder();
await writeInstalledPackageManagerAsync(rushGlobalFolder);
FileSystem.writeFile(
path.join(rushGlobalFolder.nodeSpecificPath, `${packageManager}-${packageManagerVersion}#123.lock`),
''
);

const releaseLockMock: jest.Mock = jest.fn();
const lockAcquireSpy: jest.SpyInstance = jest.spyOn(LockFile, 'acquireAsync').mockResolvedValue({
dirtyWhenAcquired: true,
release: releaseLockMock
} as unknown as LockFile);
const installSpy: jest.SpyInstance = jest
.spyOn(Utilities, 'installPackageInDirectoryAsync')
.mockResolvedValue();
const rushConfiguration: RushConfiguration = getRushConfiguration();

await InstallHelpers.ensureLocalPackageManagerAsync(rushConfiguration, rushGlobalFolder, 1, true);

expect(lockAcquireSpy).toHaveBeenCalledTimes(1);
expect(installSpy).toHaveBeenCalledTimes(1);
expect(releaseLockMock).toHaveBeenCalledTimes(1);
expect(FileSystem.exists(`${rushConfiguration.commonTempFolder}/pnpm-local`)).toEqual(true);
});
});
});