From cfc8ded3ae97dd21088d04143f00dcf0b53ade18 Mon Sep 17 00:00:00 2001 From: Ian Clanton-Thuon Date: Mon, 13 May 2024 23:40:53 -0700 Subject: [PATCH] Bring back the pnpm filtering arguments from the selection parameters. --- common/reviews/api/rush-lib.api.md | 2 +- .../rush-lib/src/api/RushConfiguration.ts | 4 +- .../src/cli/RushPnpmCommandLineParser.ts | 3 +- .../src/cli/actions/BaseInstallAction.ts | 75 +++++++++++++------ .../rush-lib/src/cli/actions/InstallAction.ts | 9 ++- .../rush-lib/src/cli/actions/UpdateAction.ts | 9 ++- .../rush-lib/src/logic/PackageJsonUpdater.ts | 11 ++- .../src/logic/base/BaseInstallManager.ts | 11 ++- .../src/logic/base/BaseInstallManagerTypes.ts | 24 ++---- .../installManager/WorkspaceInstallManager.ts | 8 +- .../installManager/doBasicInstallAsync.ts | 3 +- 11 files changed, 94 insertions(+), 65 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 57be66dd467..d985b017d44 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -1170,7 +1170,7 @@ export class RushConfiguration { // @beta (undocumented) getSubspace(subspaceName: string): Subspace; // @beta - getSubspacesForProjects(projects: ReadonlySet): ReadonlySet; + getSubspacesForProjects(projects: Iterable): ReadonlySet; readonly gitAllowedEmailRegExps: string[]; readonly gitChangefilesCommitMessage: string | undefined; readonly gitChangeLogUpdateCommitMessage: string | undefined; diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index c007b815398..97535fda7ef 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -1295,14 +1295,16 @@ export class RushConfiguration { * Returns the set of subspaces that the given projects belong to * @beta */ - public getSubspacesForProjects(projects: ReadonlySet): ReadonlySet { + public getSubspacesForProjects(projects: Iterable): ReadonlySet { if (!this._projects) { this._initializeAndValidateLocalProjects(); } + const subspaceSet: Set = new Set(); for (const project of projects) { subspaceSet.add(project.subspace); } + return subspaceSet; } diff --git a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts index 3d113b86985..a1175ff6ac1 100644 --- a/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts +++ b/libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts @@ -510,7 +510,8 @@ export class RushPnpmCommandLineParser { offline: false, collectLogFile: false, maxInstallAttempts: RushConstants.defaultMaxInstallAttempts, - filteredProjects: undefined, + pnpmFilterArguments: [], + selectedProjects: new Set(this._rushConfiguration.projects), checkOnly: false, // TODO: Support subspaces subspace: this._rushConfiguration.defaultSubspace, diff --git a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts index cafc7f4c8f2..dc16461b3a9 100644 --- a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts @@ -24,6 +24,12 @@ import type { SelectionParameterSet } from '../parsing/SelectionParameterSet'; import type { RushConfigurationProject } from '../../api/RushConfigurationProject'; import type { Subspace } from '../../api/Subspace'; +interface ISubspaceSelectedProjectsMetadata { + selectedProjects: Set; + pnpmFilterArguments: string[]; + alwaysFullInstall: boolean; +} + /** * This is the common base class for InstallAction and UpdateAction. */ @@ -143,22 +149,37 @@ export abstract class BaseInstallAction extends BaseRushAction { // If we are doing a filtered install and subspaces is enabled, we need to find the affected subspaces and install for all of them. let selectedSubspaces: ReadonlySet | undefined; - const filteredProjectsBySubspace: Map> = new Map(); + const selectedProjectsMetadataBySubspace: Map = new Map(); if (this.rushConfiguration.subspacesFeatureEnabled) { - if (installManagerOptions.filteredProjects?.size) { - // Go through each project, add it to it's subspace's pnpm filter arguments - for (const project of installManagerOptions.filteredProjects) { - let subspaceFilteredProjects: Set | undefined = - filteredProjectsBySubspace.get(project.subspace); - if (!subspaceFilteredProjects) { - subspaceFilteredProjects = new Set(); - filteredProjectsBySubspace.set(project.subspace, subspaceFilteredProjects); + const { selectedProjects } = installManagerOptions; + if (selectedProjects.size !== this.rushConfiguration.projects.length) { + // This is a filtered install. Go through each project, add its subspace's pnpm filter arguments + for (const project of selectedProjects) { + const { subspace: projectSubspace } = project; + let subspaceSelectedProjectsMetadata: ISubspaceSelectedProjectsMetadata | undefined = + selectedProjectsMetadataBySubspace.get(projectSubspace); + if (!subspaceSelectedProjectsMetadata) { + const alwaysFullInstall: boolean = projectSubspace.getPnpmOptions()?.alwaysFullInstall ?? false; + subspaceSelectedProjectsMetadata = { + selectedProjects: new Set(alwaysFullInstall ? projectSubspace.getProjects() : undefined), + pnpmFilterArguments: [], + alwaysFullInstall + }; + selectedProjectsMetadataBySubspace.set(projectSubspace, subspaceSelectedProjectsMetadata); + } + + const { + pnpmFilterArguments, + selectedProjects: subspaceSelectedProjects, + alwaysFullInstall + } = subspaceSelectedProjectsMetadata; + if (!alwaysFullInstall) { + subspaceSelectedProjects.add(project); + pnpmFilterArguments.push('--filter', project.packageName); } - subspaceFilteredProjects.add(project); } - selectedSubspaces = this.rushConfiguration.getSubspacesForProjects( - new Set(installManagerOptions.filteredProjects) - ); + + selectedSubspaces = this.rushConfiguration.getSubspacesForProjects(selectedProjects); } else if (this._subspaceParameter.value) { // Selecting a single subspace const selectedSubspace: Subspace = this.rushConfiguration.getSubspace(this._subspaceParameter.value); @@ -241,16 +262,28 @@ export abstract class BaseInstallAction extends BaseRushAction { try { if (selectedSubspaces) { // Run the install for each affected subspace - for (const selectedSubspace of selectedSubspaces) { - installManagerOptions.subspace = selectedSubspace; - if (selectedSubspace.getPnpmOptions()?.alwaysFullInstall) { - installManagerOptions.filteredProjects = undefined; + for (const subspace of selectedSubspaces) { + const selectedProjectsMetadata: ISubspaceSelectedProjectsMetadata | undefined = + selectedProjectsMetadataBySubspace.get(subspace); + // eslint-disable-next-line no-console + console.log(Colorize.green(`Installing for subspace: ${subspace.subspaceName}`)); + let installManagerOptionsForInstall: IInstallManagerOptions; + if (selectedProjectsMetadata) { + const { selectedProjects, pnpmFilterArguments } = selectedProjectsMetadata; + installManagerOptionsForInstall = { + ...installManagerOptions, + selectedProjects, + pnpmFilterArguments, + subspace + }; } else { - installManagerOptions.filteredProjects = filteredProjectsBySubspace.get(selectedSubspace); + installManagerOptionsForInstall = { + ...installManagerOptions, + subspace + }; } - // eslint-disable-next-line no-console - console.log(Colorize.green(`Installing for subspace: ${selectedSubspace.subspaceName}`)); - await this._doInstall(installManagerFactoryModule, purgeManager, installManagerOptions); + + await this._doInstall(installManagerFactoryModule, purgeManager, installManagerOptionsForInstall); } } else { await this._doInstall(installManagerFactoryModule, purgeManager, installManagerOptions); diff --git a/libraries/rush-lib/src/cli/actions/InstallAction.ts b/libraries/rush-lib/src/cli/actions/InstallAction.ts index da6bbfe112b..f3d844f1f89 100644 --- a/libraries/rush-lib/src/cli/actions/InstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/InstallAction.ts @@ -45,8 +45,9 @@ export class InstallAction extends BaseInstallAction { } protected async buildInstallOptionsAsync(): Promise { - const selectedProjects: Set | undefined = - await this._selectionParameters?.getSelectedProjectsAsync(this._terminal); + const selectedProjects: Set = + (await this._selectionParameters?.getSelectedProjectsAsync(this._terminal)) ?? + new Set(this.rushConfiguration.projects); return { debug: this.parser.isDebug, @@ -63,7 +64,9 @@ export class InstallAction extends BaseInstallAction { // it is safe to assume that the value is not null maxInstallAttempts: this._maxInstallAttempts.value!, // These are derived independently of the selection for command line brevity - filteredProjects: selectedProjects, + selectedProjects, + pnpmFilterArguments: + (await this._selectionParameters?.getPnpmFilterArgumentsAsync(this._terminal)) ?? [], checkOnly: this._checkOnlyParameter.value, subspace: this.getTargetSubspace(), beforeInstallAsync: () => this.rushSession.hooks.beforeInstall.promise(this), diff --git a/libraries/rush-lib/src/cli/actions/UpdateAction.ts b/libraries/rush-lib/src/cli/actions/UpdateAction.ts index 719102a94ba..c635f12d557 100644 --- a/libraries/rush-lib/src/cli/actions/UpdateAction.ts +++ b/libraries/rush-lib/src/cli/actions/UpdateAction.ts @@ -76,8 +76,9 @@ export class UpdateAction extends BaseInstallAction { } protected async buildInstallOptionsAsync(): Promise { - const selectedProjects: Set | undefined = - await this._selectionParameters?.getSelectedProjectsAsync(this._terminal); + const selectedProjects: Set = + (await this._selectionParameters?.getSelectedProjectsAsync(this._terminal)) ?? + new Set(this.rushConfiguration.projects); return { debug: this.parser.isDebug, @@ -94,7 +95,9 @@ export class UpdateAction extends BaseInstallAction { // it is safe to assume that the value is not null maxInstallAttempts: this._maxInstallAttempts.value!, // These are derived independently of the selection for command line brevity - filteredProjects: selectedProjects, + selectedProjects, + pnpmFilterArguments: + (await this._selectionParameters?.getPnpmFilterArgumentsAsync(this._terminal)) ?? [], checkOnly: false, subspace: this.getTargetSubspace(), diff --git a/libraries/rush-lib/src/logic/PackageJsonUpdater.ts b/libraries/rush-lib/src/logic/PackageJsonUpdater.ts index 5048ad59809..f581a1f6be5 100644 --- a/libraries/rush-lib/src/logic/PackageJsonUpdater.ts +++ b/libraries/rush-lib/src/logic/PackageJsonUpdater.ts @@ -232,7 +232,7 @@ export class PackageJsonUpdater { if (!skipUpdate) { if (this._rushConfiguration.subspacesFeatureEnabled) { const subspaceSet: ReadonlySet = this._rushConfiguration.getSubspacesForProjects( - new Set(options.projects) + options.projects ); for (const subspace of subspaceSet) { await this._doUpdate(debugInstall, subspace); @@ -262,7 +262,7 @@ export class PackageJsonUpdater { if (!skipUpdate) { if (this._rushConfiguration.subspacesFeatureEnabled) { const subspaceSet: ReadonlySet = this._rushConfiguration.getSubspacesForProjects( - new Set(options.projects) + options.projects ); for (const subspace of subspaceSet) { await this._doUpdate(debugInstall, subspace); @@ -290,7 +290,8 @@ export class PackageJsonUpdater { offline: false, collectLogFile: false, maxInstallAttempts: RushConstants.defaultMaxInstallAttempts, - filteredProjects: undefined, + pnpmFilterArguments: [], + selectedProjects: new Set(this._rushConfiguration.projects), checkOnly: false, subspace: subspace, terminal: this._terminal @@ -326,9 +327,7 @@ export class PackageJsonUpdater { ); const allPackageUpdates: IUpdateProjectOptions[] = []; - const subspaceSet: ReadonlySet = this._rushConfiguration.getSubspacesForProjects( - new Set(projects) - ); + const subspaceSet: ReadonlySet = this._rushConfiguration.getSubspacesForProjects(projects); for (const subspace of subspaceSet) { // Projects for this subspace allPackageUpdates.push(...(await this._updateProjects(subspace, dependencyAnalyzer, options))); diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts index 1bfae24a3f9..c3d3bb61b39 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts @@ -103,8 +103,8 @@ export abstract class BaseInstallManager { } public async doInstallAsync(): Promise { - const { allowShrinkwrapUpdates, filteredProjects } = this.options; - const isFilteredInstall: boolean = (filteredProjects?.size ?? 0) > 0; + const { allowShrinkwrapUpdates, selectedProjects, pnpmFilterArguments } = this.options; + const isFilteredInstall: boolean = pnpmFilterArguments.length > 0; const useWorkspaces: boolean = this.rushConfiguration.pnpmOptions && this.rushConfiguration.pnpmOptions.useWorkspaces; // Prevent filtered installs when workspaces is disabled @@ -163,9 +163,9 @@ export abstract class BaseInstallManager { const commonTempInstallFlag: LastInstallFlag = getCommonTempFlag(this.rushConfiguration, subspace, { npmrcHash: npmrcHash || '' }); - if (isFilteredInstall && filteredProjects) { + if (isFilteredInstall && selectedProjects) { const selectedProjectNames: string[] = []; - for (const { packageName } of filteredProjects) { + for (const { packageName } of selectedProjects) { selectedProjectNames.push(packageName); } @@ -714,8 +714,7 @@ ${gitLfsHookHandling} args.push('--frozen-lockfile'); if ( - options.filteredProjects && - options.filteredProjects.size > 0 && + options.pnpmFilterArguments.length > 0 && Number.parseInt(this.rushConfiguration.packageManagerToolVersion, 10) >= 8 // PNPM Major version 8+ ) { // On pnpm@8, disable the "dedupe-peer-dependents" feature when doing a filtered CI install so that filters take effect. diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManagerTypes.ts b/libraries/rush-lib/src/logic/base/BaseInstallManagerTypes.ts index dfdd450ef3e..fe8c39ce193 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManagerTypes.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManagerTypes.ts @@ -82,28 +82,20 @@ export interface IInstallManagerOptions { maxInstallAttempts: number; /** - * If `filteredProjects` is `undefined`, then Rush will perform a regular `pnpm install`. - * - * If `filteredProjects` is specified, then Rush will perform a "filtered install" by invoking PNPM like this, - * a special mode which installs or updates only a subset of the lockfile: - * - * `pnpm install --filter project1 --filter project5 --filter project6` - * - * If `filteredProjects` is specified and the set is empty, then the install manager will skip installation entirely. + * Filters to be passed to PNPM during installation, if applicable. + * These restrict the scope of a workspace installation. * * @remarks - * If `filteredProjects` specifies all projects in the workspace or subspace, filtering will still occur; - * it is the caller's responsibility to optimize that. - * - * Compared to an unfiltered install, filtering brings some limitations: For example, Rush's install skipping - * optimization may miss some cases. For `rush update`, PNPM may not remove deadwood from the lockfile as - * effectively as an unfiltered install. - * * Note that PNPM may arbitrarily ignore `--filter` (producing an unfiltered install) in certain situations, * for example when `config.dedupe-peer-dependents=true` with PNPM 8. Rush tries to circumvent this, under the * assumption that a user who invokes a filtered install cares more about lockfile stability than duplication. */ - filteredProjects: Set | undefined; + pnpmFilterArguments: string[]; + + /** + * The set of projects for which installation should be performed. + */ + selectedProjects: Set; /** * Callback to invoke between preparing the common/temp folder and running installation. diff --git a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts index 90947a734c5..4607fa06e15 100644 --- a/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts +++ b/libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts @@ -724,12 +724,8 @@ export class WorkspaceInstallManager extends BaseInstallManager { } } - const { filteredProjects } = this.options; - if (filteredProjects && filteredProjects.size !== subspace.getProjects().length) { - for (const arg of filteredProjects) { - args.push('--filter'); - args.push(arg.packageName); - } + for (const arg of this.options.pnpmFilterArguments) { + args.push(arg); } } } diff --git a/libraries/rush-lib/src/logic/installManager/doBasicInstallAsync.ts b/libraries/rush-lib/src/logic/installManager/doBasicInstallAsync.ts index 3244847572f..3d404a370c6 100644 --- a/libraries/rush-lib/src/logic/installManager/doBasicInstallAsync.ts +++ b/libraries/rush-lib/src/logic/installManager/doBasicInstallAsync.ts @@ -40,7 +40,8 @@ export async function doBasicInstallAsync(options: IRunInstallOptions): Promise< recheckShrinkwrap: false, offline: false, collectLogFile: false, - filteredProjects: undefined, + pnpmFilterArguments: [], + selectedProjects: new Set(rushConfiguration.projects), maxInstallAttempts: 1, networkConcurrency: undefined, subspace: rushConfiguration.defaultSubspace,