Skip to content

Commit

Permalink
Bring back the pnpm filtering arguments from the selection parameters.
Browse files Browse the repository at this point in the history
  • Loading branch information
iclanton committed May 14, 2024
1 parent 2028774 commit cfc8ded
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 65 deletions.
2 changes: 1 addition & 1 deletion common/reviews/api/rush-lib.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@ export class RushConfiguration {
// @beta (undocumented)
getSubspace(subspaceName: string): Subspace;
// @beta
getSubspacesForProjects(projects: ReadonlySet<RushConfigurationProject>): ReadonlySet<Subspace>;
getSubspacesForProjects(projects: Iterable<RushConfigurationProject>): ReadonlySet<Subspace>;
readonly gitAllowedEmailRegExps: string[];
readonly gitChangefilesCommitMessage: string | undefined;
readonly gitChangeLogUpdateCommitMessage: string | undefined;
Expand Down
4 changes: 3 additions & 1 deletion libraries/rush-lib/src/api/RushConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1295,14 +1295,16 @@ export class RushConfiguration {
* Returns the set of subspaces that the given projects belong to
* @beta
*/
public getSubspacesForProjects(projects: ReadonlySet<RushConfigurationProject>): ReadonlySet<Subspace> {
public getSubspacesForProjects(projects: Iterable<RushConfigurationProject>): ReadonlySet<Subspace> {
if (!this._projects) {
this._initializeAndValidateLocalProjects();
}

const subspaceSet: Set<Subspace> = new Set();
for (const project of projects) {
subspaceSet.add(project.subspace);
}

return subspaceSet;
}

Expand Down
3 changes: 2 additions & 1 deletion libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
75 changes: 54 additions & 21 deletions libraries/rush-lib/src/cli/actions/BaseInstallAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RushConfigurationProject>;
pnpmFilterArguments: string[];
alwaysFullInstall: boolean;
}

/**
* This is the common base class for InstallAction and UpdateAction.
*/
Expand Down Expand Up @@ -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<Subspace> | undefined;
const filteredProjectsBySubspace: Map<Subspace, Set<RushConfigurationProject>> = new Map();
const selectedProjectsMetadataBySubspace: Map<Subspace, ISubspaceSelectedProjectsMetadata> = 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<RushConfigurationProject> | 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);
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions libraries/rush-lib/src/cli/actions/InstallAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ export class InstallAction extends BaseInstallAction {
}

protected async buildInstallOptionsAsync(): Promise<IInstallManagerOptions> {
const selectedProjects: Set<RushConfigurationProject> | undefined =
await this._selectionParameters?.getSelectedProjectsAsync(this._terminal);
const selectedProjects: Set<RushConfigurationProject> =
(await this._selectionParameters?.getSelectedProjectsAsync(this._terminal)) ??
new Set(this.rushConfiguration.projects);

return {
debug: this.parser.isDebug,
Expand All @@ -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),
Expand Down
9 changes: 6 additions & 3 deletions libraries/rush-lib/src/cli/actions/UpdateAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ export class UpdateAction extends BaseInstallAction {
}

protected async buildInstallOptionsAsync(): Promise<IInstallManagerOptions> {
const selectedProjects: Set<RushConfigurationProject> | undefined =
await this._selectionParameters?.getSelectedProjectsAsync(this._terminal);
const selectedProjects: Set<RushConfigurationProject> =
(await this._selectionParameters?.getSelectedProjectsAsync(this._terminal)) ??
new Set(this.rushConfiguration.projects);

return {
debug: this.parser.isDebug,
Expand All @@ -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(),

Expand Down
11 changes: 5 additions & 6 deletions libraries/rush-lib/src/logic/PackageJsonUpdater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export class PackageJsonUpdater {
if (!skipUpdate) {
if (this._rushConfiguration.subspacesFeatureEnabled) {
const subspaceSet: ReadonlySet<Subspace> = this._rushConfiguration.getSubspacesForProjects(
new Set(options.projects)
options.projects
);
for (const subspace of subspaceSet) {
await this._doUpdate(debugInstall, subspace);
Expand Down Expand Up @@ -262,7 +262,7 @@ export class PackageJsonUpdater {
if (!skipUpdate) {
if (this._rushConfiguration.subspacesFeatureEnabled) {
const subspaceSet: ReadonlySet<Subspace> = this._rushConfiguration.getSubspacesForProjects(
new Set(options.projects)
options.projects
);
for (const subspace of subspaceSet) {
await this._doUpdate(debugInstall, subspace);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -326,9 +327,7 @@ export class PackageJsonUpdater {
);

const allPackageUpdates: IUpdateProjectOptions[] = [];
const subspaceSet: ReadonlySet<Subspace> = this._rushConfiguration.getSubspacesForProjects(
new Set(projects)
);
const subspaceSet: ReadonlySet<Subspace> = this._rushConfiguration.getSubspacesForProjects(projects);
for (const subspace of subspaceSet) {
// Projects for this subspace
allPackageUpdates.push(...(await this._updateProjects(subspace, dependencyAnalyzer, options)));
Expand Down
11 changes: 5 additions & 6 deletions libraries/rush-lib/src/logic/base/BaseInstallManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export abstract class BaseInstallManager {
}

public async doInstallAsync(): Promise<void> {
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
Expand Down Expand Up @@ -163,9 +163,9 @@ export abstract class BaseInstallManager {
const commonTempInstallFlag: LastInstallFlag = getCommonTempFlag(this.rushConfiguration, subspace, {
npmrcHash: npmrcHash || '<NO NPMRC>'
});
if (isFilteredInstall && filteredProjects) {
if (isFilteredInstall && selectedProjects) {
const selectedProjectNames: string[] = [];
for (const { packageName } of filteredProjects) {
for (const { packageName } of selectedProjects) {
selectedProjectNames.push(packageName);
}

Expand Down Expand Up @@ -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.
Expand Down
24 changes: 8 additions & 16 deletions libraries/rush-lib/src/logic/base/BaseInstallManagerTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RushConfigurationProject> | undefined;
pnpmFilterArguments: string[];

/**
* The set of projects for which installation should be performed.
*/
selectedProjects: Set<RushConfigurationProject>;

/**
* Callback to invoke between preparing the common/temp folder and running installation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit cfc8ded

Please sign in to comment.