From 3b8efbb2eca5896abce4386d7ecbf0731f08d2df Mon Sep 17 00:00:00 2001 From: William Huang Date: Wed, 31 Jan 2024 08:39:35 +0800 Subject: [PATCH 1/8] Restore checking for subspace provided in BaseInstallManager --- .../src/cli/actions/BaseInstallAction.ts | 41 +++---------------- .../src/logic/base/BaseInstallManager.ts | 25 +++++++++++ 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts index 84d4938e0fd..97c649b9544 100644 --- a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts @@ -8,12 +8,7 @@ import type { CommandLineIntegerParameter, CommandLineStringParameter } from '@rushstack/ts-command-line'; -import { - ConsoleTerminalProvider, - type ITerminal, - Terminal, - AlreadyReportedError -} from '@rushstack/node-core-library'; +import { ConsoleTerminalProvider, type ITerminal, Terminal } from '@rushstack/node-core-library'; import { BaseRushAction, type IBaseRushActionOptions } from './BaseRushAction'; import { Event } from '../../api/EventHooks'; @@ -117,36 +112,10 @@ export abstract class BaseInstallAction extends BaseRushAction { protected getTargetSubspace(): Subspace { const parameterValue: string | undefined = this._subspaceParameter.value; - - if (this.rushConfiguration.subspacesFeatureEnabled) { - if (!parameterValue) { - // Temporarily ensure that a subspace is provided - // eslint-disable-next-line no-console - console.log(); - // eslint-disable-next-line no-console - console.log( - colors.red( - `The subspaces feature currently only supports installing for a specified set of subspace,` + - ` passed by the "--subspace" parameter or selected from targeted projects using any project selector.` - ) - ); - throw new AlreadyReportedError(); - } - return this.rushConfiguration.getSubspace(parameterValue); - } else { - if (parameterValue) { - // eslint-disable-next-line no-console - console.log(); - // eslint-disable-next-line no-console - console.log( - colors.red( - `The "--subspace" parameter can only be passed if the "enabled" option is enabled in subspaces.json.` - ) - ); - throw new AlreadyReportedError(); - } - return this.rushConfiguration.defaultSubspace; - } + const selectedSubspace: Subspace | undefined = parameterValue + ? this.rushConfiguration.getSubspace(parameterValue) + : this.rushConfiguration.defaultSubspace; + return selectedSubspace; } protected async runAsync(): Promise { diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts index bcbbc804168..962fedc0ca9 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts @@ -119,6 +119,31 @@ export abstract class BaseInstallManager { ); throw new AlreadyReportedError(); } + // Ensure that subspaces is enabled + + if (this.rushConfiguration.subspacesFeatureEnabled && !this.options.subspace) { + // Temporarily ensure that a subspace is provided + // eslint-disable-next-line no-console + console.log(); + // eslint-disable-next-line no-console + console.log( + colors.red( + `The subspaces feature currently only supports installing for a specified set of subspace,` + + ` passed by the "--subspace" parameter or selected from targeted projects using any project selector.` + ) + ); + throw new AlreadyReportedError(); + } else if (this.options.subspace && !this.rushConfiguration.subspacesFeatureEnabled) { + // eslint-disable-next-line no-console + console.log(); + // eslint-disable-next-line no-console + console.log( + colors.red( + `The "--subspace" parameter can only be passed if the "enabled" option is enabled in subspaces.json.` + ) + ); + throw new AlreadyReportedError(); + } // Prevent update when using a filter, as modifications to the shrinkwrap shouldn't be saved if (this.options.allowShrinkwrapUpdates && isFilteredInstall) { From 777dd71890532e471f31cea1bcc06a5bc5e13831 Mon Sep 17 00:00:00 2001 From: William Huang Date: Wed, 31 Jan 2024 10:59:48 +0800 Subject: [PATCH 2/8] Add support for global subspace installs --- common/reviews/api/rush-lib.api.md | 1 + .../common/config/rush/subspaces.json | 5 ++ .../src/api/SubspacesConfiguration.ts | 7 +++ .../src/cli/actions/BaseInstallAction.ts | 58 ++++++++++++++----- .../src/logic/base/BaseInstallManager.ts | 25 -------- .../src/schemas/subspaces.schema.json | 4 ++ 6 files changed, 62 insertions(+), 38 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index dd228f7cd48..8c28b2f88e8 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -1377,6 +1377,7 @@ export class SubspacesConfiguration { static _convertNameToEnvironmentVariable(subspaceName: string, splitWorkspaceCompatibility: boolean): string; readonly enabled: boolean; static explainIfInvalidSubspaceName(subspaceName: string, splitWorkspaceCompatibility?: boolean): string | undefined; + readonly preventSelectingAllSubspaces: boolean; static requireValidSubspaceName(subspaceName: string, splitWorkspaceCompatibility?: boolean): void; readonly splitWorkspaceCompatibility: boolean; readonly subspaceJsonFilePath: string; diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json index 3526906a094..1b3647dbf62 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json @@ -21,6 +21,11 @@ */ "splitWorkspaceCompatibility": false, + /** + * This allows for selecting all subspaces when no selector is provided. + */ + "preventSelectingAllSubspaces": false, + /** * The next field is an object describing the available subspaces in this workspace. Each individual subspace * must be defined in this array. diff --git a/libraries/rush-lib/src/api/SubspacesConfiguration.ts b/libraries/rush-lib/src/api/SubspacesConfiguration.ts index 73a4ba5308e..9bfc883b82b 100644 --- a/libraries/rush-lib/src/api/SubspacesConfiguration.ts +++ b/libraries/rush-lib/src/api/SubspacesConfiguration.ts @@ -23,6 +23,7 @@ export const SPLIT_WORKSPACE_SUBSPACE_NAME_REGEXP: RegExp = /^[a-z0-9][+_\-a-z0- interface ISubspacesConfigurationJson { enabled: boolean; splitWorkspaceCompatibility?: boolean; + preventSelectingAllSubspaces?: boolean; subspaceNames: string[]; } @@ -49,6 +50,11 @@ export class SubspacesConfiguration { */ public readonly splitWorkspaceCompatibility: boolean; + /** + * This determines if selectors are required when installing and building + */ + public readonly preventSelectingAllSubspaces: boolean; + /** * A set of the available subspaces */ @@ -58,6 +64,7 @@ export class SubspacesConfiguration { this.subspaceJsonFilePath = subspaceJsonFilePath; this.enabled = configuration.enabled; this.splitWorkspaceCompatibility = !!configuration.splitWorkspaceCompatibility; + this.preventSelectingAllSubspaces = !!configuration.preventSelectingAllSubspaces; const subspaceNames: Set = new Set(); for (const subspaceName of configuration.subspaceNames) { SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.enabled); diff --git a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts index 97c649b9544..a7468f44a13 100644 --- a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts @@ -8,7 +8,12 @@ import type { CommandLineIntegerParameter, CommandLineStringParameter } from '@rushstack/ts-command-line'; -import { ConsoleTerminalProvider, type ITerminal, Terminal } from '@rushstack/node-core-library'; +import { + ConsoleTerminalProvider, + type ITerminal, + Terminal, + AlreadyReportedError +} from '@rushstack/node-core-library'; import { BaseRushAction, type IBaseRushActionOptions } from './BaseRushAction'; import { Event } from '../../api/EventHooks'; @@ -112,6 +117,17 @@ export abstract class BaseInstallAction extends BaseRushAction { protected getTargetSubspace(): Subspace { const parameterValue: string | undefined = this._subspaceParameter.value; + if (parameterValue && !this.rushConfiguration.subspacesFeatureEnabled) { + // eslint-disable-next-line no-console + console.log(); + // eslint-disable-next-line no-console + console.log( + colors.red( + `The "--subspace" parameter can only be passed if the "enabled" option is enabled in subspaces.json.` + ) + ); + throw new AlreadyReportedError(); + } const selectedSubspace: Subspace | undefined = parameterValue ? this.rushConfiguration.getSubspace(parameterValue) : this.rushConfiguration.defaultSubspace; @@ -123,13 +139,35 @@ 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; - if (installManagerOptions.pnpmFilterArguments.length && this.rushConfiguration.subspacesFeatureEnabled) { - const selectedProjects: Set | undefined = - await this._selectionParameters?.getSelectedProjectsAsync(this._terminal); - if (selectedProjects) { - selectedSubspaces = this.rushConfiguration.getSubspacesForProjects(selectedProjects); + if (this.rushConfiguration.subspacesFeatureEnabled) { + if (installManagerOptions.pnpmFilterArguments.length) { + // Selecting a set of subspaces + const selectedProjects: Set | undefined = + await this._selectionParameters?.getSelectedProjectsAsync(this._terminal); + if (selectedProjects) { + selectedSubspaces = this.rushConfiguration.getSubspacesForProjects(selectedProjects); + } else { + throw new Error('The specified filter arguments resulted in no projects being selected.'); + } + } else if (this._subspaceParameter.value) { + // Selecting a single subspace + const selectedSubspace = this.rushConfiguration.getSubspace(this._subspaceParameter.value); + selectedSubspaces = new Set([selectedSubspace]); } else { - throw new Error('The specified filter arguments resulted in no projects being selected.'); + // Selecting all subspaces if preventSelectingAllSubspaces is not enabled in subspaces.json + if (!this.rushConfiguration.subspacesConfiguration?.preventSelectingAllSubspaces) { + selectedSubspaces = new Set(this.rushConfiguration.subspaces); + } else { + // eslint-disable-next-line no-console + console.log(); + // eslint-disable-next-line no-console + console.log( + colors.red( + `The subspaces preventSelectingAllSubspaces configuration is enabled, which enforces installation for a specified set of subspace,` + + ` passed by the "--subspace" parameter or selected from targeted projects using any project selector.` + ) + ); + } } } @@ -141,12 +179,6 @@ export abstract class BaseInstallAction extends BaseRushAction { subspace }); } - } else if (this._subspaceParameter.value) { - const subspace: Subspace = this.rushConfiguration.getSubspace(this._subspaceParameter.value); - VersionMismatchFinder.ensureConsistentVersions(this.rushConfiguration, this._terminal, { - variant: this._variant.value, - subspace: subspace - }); } else { VersionMismatchFinder.ensureConsistentVersions(this.rushConfiguration, this._terminal, { variant: this._variant.value diff --git a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts index 962fedc0ca9..bcbbc804168 100644 --- a/libraries/rush-lib/src/logic/base/BaseInstallManager.ts +++ b/libraries/rush-lib/src/logic/base/BaseInstallManager.ts @@ -119,31 +119,6 @@ export abstract class BaseInstallManager { ); throw new AlreadyReportedError(); } - // Ensure that subspaces is enabled - - if (this.rushConfiguration.subspacesFeatureEnabled && !this.options.subspace) { - // Temporarily ensure that a subspace is provided - // eslint-disable-next-line no-console - console.log(); - // eslint-disable-next-line no-console - console.log( - colors.red( - `The subspaces feature currently only supports installing for a specified set of subspace,` + - ` passed by the "--subspace" parameter or selected from targeted projects using any project selector.` - ) - ); - throw new AlreadyReportedError(); - } else if (this.options.subspace && !this.rushConfiguration.subspacesFeatureEnabled) { - // eslint-disable-next-line no-console - console.log(); - // eslint-disable-next-line no-console - console.log( - colors.red( - `The "--subspace" parameter can only be passed if the "enabled" option is enabled in subspaces.json.` - ) - ); - throw new AlreadyReportedError(); - } // Prevent update when using a filter, as modifications to the shrinkwrap shouldn't be saved if (this.options.allowShrinkwrapUpdates && isFilteredInstall) { diff --git a/libraries/rush-lib/src/schemas/subspaces.schema.json b/libraries/rush-lib/src/schemas/subspaces.schema.json index e6d62d9b2fe..75a3722c2bf 100644 --- a/libraries/rush-lib/src/schemas/subspaces.schema.json +++ b/libraries/rush-lib/src/schemas/subspaces.schema.json @@ -17,6 +17,10 @@ "description": "(DEPRECATED) Allows individual subspaces to be configured at the package level if that package is the only project in the subspace. Used to help migrate from a split workspace state.", "type": "boolean" }, + "preventSelectingAllSubspaces": { + "description": "If true, requires a selector for a subspace or set of subspaces when installing.", + "type": "boolean" + }, "subspaceNames": { "description": "Individual subspace configurations.", "type": "array", From b0761f08081e6aad88eba75994a554061b7de0de Mon Sep 17 00:00:00 2001 From: William Huang Date: Wed, 31 Jan 2024 11:00:59 +0800 Subject: [PATCH 3/8] Add change file --- .../rush/will-subspace-patch-1_2024-01-31-03-00.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json diff --git a/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json b/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json new file mode 100644 index 00000000000..ea2781d918f --- /dev/null +++ b/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix issues with subspace targeted installs", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file From c3cb0624a6f0768da84b8327d1b702231ef385dc Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:10:04 -0800 Subject: [PATCH 4/8] Fix up "rush init" template and enable its creation --- .../common/config/rush/subspaces.json | 40 +++++++++---------- .../src/cli/actions/BaseInstallAction.ts | 2 +- .../rush-lib/src/cli/actions/InitAction.ts | 1 + 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json index 1b3647dbf62..21ebacc05ec 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json @@ -1,35 +1,35 @@ /** - * This is the main configuration file for Rush. + * This configuration file manages the experimental "subspaces" feature for Rush, + * which allows multiple PNPM lockfiles to be used in a single Rush workspace. * For full documentation, please see https://rushjs.io */ - { - /** - * Temporarily use the default schema until the subspaces schema is uploaded - */ +{ "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/subspaces.schema.json", /** - * This specifies whether or not to use the "subspaces" feature in rush. This allows grouping of - * projects into discrete workspaces known as "subspaces". + * Set this flag to "true" to enable usage of subspaces. */ "enabled": false, /** - * (DEPRECATED) A property that allows subspace configuration files to be stored under project folders directly. - * Used for migrating from a split-workspace format where there may be a large number of individual lockfiles and - * starting subspaces. + * (DEPRECATED) This is a temporary workaround for migrating from an earlier prototype + * of this feature: https://github.com/microsoft/rushstack/pull/3481 + * It allows subspaces with only one project to store their config files in the project folder. */ - "splitWorkspaceCompatibility": false, + "splitWorkspaceCompatibility": false, - /** - * This allows for selecting all subspaces when no selector is provided. - */ - "preventSelectingAllSubspaces": false, + /** + * When a command such as "rush update" is invoked without the "--subspace" or "--to" + * parameters, Rush will install all subspaces. In a huge monorepo with numerous subspaces, + * this would be extremely slow. Set "preventSelectingAllSubspaces" to true to avoid this + * mistake by always requiring selection parameters for commands such as "rush update". + */ + "preventSelectingAllSubspaces": false, - /** - * The next field is an object describing the available subspaces in this workspace. Each individual subspace - * must be defined in this array. - */ + /** + * The list of subspace names, which should be lowercase alphanumeric words separated by + * hyphens, for example "my-subspace". The corresponding config files will have paths + * such as "common/config/subspaces/my-subspace/package-lock.yaml". + */ "subspaceNames": [] - } diff --git a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts index a7468f44a13..3ec807deaa5 100644 --- a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts @@ -151,7 +151,7 @@ export abstract class BaseInstallAction extends BaseRushAction { } } else if (this._subspaceParameter.value) { // Selecting a single subspace - const selectedSubspace = this.rushConfiguration.getSubspace(this._subspaceParameter.value); + const selectedSubspace: Subspace = this.rushConfiguration.getSubspace(this._subspaceParameter.value); selectedSubspaces = new Set([selectedSubspace]); } else { // Selecting all subspaces if preventSelectingAllSubspaces is not enabled in subspaces.json diff --git a/libraries/rush-lib/src/cli/actions/InitAction.ts b/libraries/rush-lib/src/cli/actions/InitAction.ts index d20a17313dc..557a6e60ee1 100644 --- a/libraries/rush-lib/src/cli/actions/InitAction.ts +++ b/libraries/rush-lib/src/cli/actions/InitAction.ts @@ -177,6 +177,7 @@ export class InitAction extends BaseConfiglessRushAction { 'common/config/rush/experiments.json', 'common/config/rush/pnpm-config.json', 'common/config/rush/rush-plugins.json', + 'common/config/rush/subspaces.json', 'common/config/rush/version-policies.json', 'common/git-hooks/commit-msg.sample', From 9a8dfcfcacf0af981823aec979dbbedc93e918fe Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:14:18 -0800 Subject: [PATCH 5/8] Rename "enabled" to "subspacesEnabled" for consistency with buildCacheEnabled and cobuildFeatureEnabled --- common/reviews/api/rush-lib.api.md | 3 ++- .../assets/rush-init/common/config/rush/subspaces.json | 2 +- libraries/rush-lib/src/api/RushConfiguration.ts | 4 ++-- libraries/rush-lib/src/api/SubspacesConfiguration.ts | 10 +++++----- libraries/rush-lib/src/schemas/subspaces.schema.json | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index 8c28b2f88e8..685de1635b4 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -1375,7 +1375,6 @@ export class Subspace { export class SubspacesConfiguration { // @internal static _convertNameToEnvironmentVariable(subspaceName: string, splitWorkspaceCompatibility: boolean): string; - readonly enabled: boolean; static explainIfInvalidSubspaceName(subspaceName: string, splitWorkspaceCompatibility?: boolean): string | undefined; readonly preventSelectingAllSubspaces: boolean; static requireValidSubspaceName(subspaceName: string, splitWorkspaceCompatibility?: boolean): void; @@ -1383,6 +1382,8 @@ export class SubspacesConfiguration { readonly subspaceJsonFilePath: string; readonly subspaceNames: ReadonlySet; // (undocumented) + readonly subspacesEnabled: boolean; + // (undocumented) static tryLoadFromConfigurationFile(subspaceJsonFilePath: string): SubspacesConfiguration | undefined; // (undocumented) static tryLoadFromDefaultLocation(rushConfiguration: RushConfiguration): SubspacesConfiguration | undefined; diff --git a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json index 21ebacc05ec..d3c3ae8c516 100644 --- a/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json +++ b/libraries/rush-lib/assets/rush-init/common/config/rush/subspaces.json @@ -9,7 +9,7 @@ /** * Set this flag to "true" to enable usage of subspaces. */ - "enabled": false, + "subspacesEnabled": false, /** * (DEPRECATED) This is a temporary workaround for migrating from an earlier prototype diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index 0aaaa6e19a5..a203f3d873e 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -633,7 +633,7 @@ export class RushConfiguration { // Try getting a subspace configuration this.subspacesConfiguration = SubspacesConfiguration.tryLoadFromDefaultLocation(this); - this.subspacesFeatureEnabled = !!this.subspacesConfiguration?.enabled; + this.subspacesFeatureEnabled = !!this.subspacesConfiguration?.subspacesEnabled; this._subspacesByName = new Map(); @@ -860,7 +860,7 @@ export class RushConfiguration { // Build the subspaces map const subspaceNames: string[] = []; let splitWorkspaceCompatibility: boolean = false; - if (this.subspacesConfiguration?.enabled) { + if (this.subspacesConfiguration?.subspacesEnabled) { splitWorkspaceCompatibility = this.subspacesConfiguration.splitWorkspaceCompatibility; subspaceNames.push(...this.subspacesConfiguration.subspaceNames); diff --git a/libraries/rush-lib/src/api/SubspacesConfiguration.ts b/libraries/rush-lib/src/api/SubspacesConfiguration.ts index 9bfc883b82b..a23d0655220 100644 --- a/libraries/rush-lib/src/api/SubspacesConfiguration.ts +++ b/libraries/rush-lib/src/api/SubspacesConfiguration.ts @@ -21,7 +21,7 @@ export const SPLIT_WORKSPACE_SUBSPACE_NAME_REGEXP: RegExp = /^[a-z0-9][+_\-a-z0- * See subspace.schema.json for documentation. */ interface ISubspacesConfigurationJson { - enabled: boolean; + subspacesEnabled: boolean; splitWorkspaceCompatibility?: boolean; preventSelectingAllSubspaces?: boolean; subspaceNames: string[]; @@ -40,10 +40,10 @@ export class SubspacesConfiguration { */ public readonly subspaceJsonFilePath: string; - /** + /* * Determines if the subspace feature is enabled */ - public readonly enabled: boolean; + public readonly subspacesEnabled: boolean; /** * This determines if the subspaces feature supports adding configuration files under the project folder itself @@ -62,12 +62,12 @@ export class SubspacesConfiguration { private constructor(configuration: Readonly, subspaceJsonFilePath: string) { this.subspaceJsonFilePath = subspaceJsonFilePath; - this.enabled = configuration.enabled; + this.subspacesEnabled = configuration.subspacesEnabled; this.splitWorkspaceCompatibility = !!configuration.splitWorkspaceCompatibility; this.preventSelectingAllSubspaces = !!configuration.preventSelectingAllSubspaces; const subspaceNames: Set = new Set(); for (const subspaceName of configuration.subspaceNames) { - SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.enabled); + SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.subspacesEnabled); subspaceNames.add(subspaceName); } diff --git a/libraries/rush-lib/src/schemas/subspaces.schema.json b/libraries/rush-lib/src/schemas/subspaces.schema.json index 75a3722c2bf..5322a806a7b 100644 --- a/libraries/rush-lib/src/schemas/subspaces.schema.json +++ b/libraries/rush-lib/src/schemas/subspaces.schema.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-04/schema#", "title": "Rush subspace config file.", - "description": "The configuration file for enabling the subspaces feature in rush. This is an EXPERIMENTAL feature which is not yet fully implemented. To opt into the experiemnt, simply toggle the 'enabled' property in this file.", + "description": "The configuration file for enabling the subspaces feature in rush. This is an EXPERIMENTAL feature which is not yet fully implemented. To opt into the experiment, simply toggle the 'enabled' property in this file.", "type": "object", "properties": { @@ -9,7 +9,7 @@ "description": "Part of the JSON Schema standard, this optional keyword declares the URL of the schema that the file conforms to. Editors may download the schema and use it to perform syntax highlighting.", "type": "string" }, - "enabled": { + "subspacesEnabled": { "description": "If true, rush will use the subspaces configuration.", "type": "boolean" }, From ccd9d89c04e01a03b316e7cd74699a9666b1894d Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:23:52 -0800 Subject: [PATCH 6/8] Update change file --- .../@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json b/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json index ea2781d918f..bd478971daa 100644 --- a/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json +++ b/common/changes/@microsoft/rush/will-subspace-patch-1_2024-01-31-03-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Fix issues with subspace targeted installs", + "comment": "(EXPERIMENTAL) Enable filtered installs of subspaces and add a \"preventSelectingAllSubspaces\" setting", "type": "none" } ], From fc5fff04d141909dd97a0ee2a9d7ed448e96a22d Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:34:14 -0800 Subject: [PATCH 7/8] Fix some code comments --- libraries/rush-lib/src/api/RushConfiguration.ts | 2 +- libraries/rush-lib/src/cli/actions/BaseInstallAction.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index a203f3d873e..b1243d5dcee 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -353,7 +353,7 @@ export class RushConfiguration { public readonly subspacesConfiguration: SubspacesConfiguration | undefined; /** - * Returns true if subspaces.json is present with "enabled=true". + * Returns true if subspaces.json is present with "subspacesEnabled=true". */ public readonly subspacesFeatureEnabled: boolean; diff --git a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts index 3ec807deaa5..712fd7033d2 100644 --- a/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts +++ b/libraries/rush-lib/src/cli/actions/BaseInstallAction.ts @@ -123,7 +123,7 @@ export abstract class BaseInstallAction extends BaseRushAction { // eslint-disable-next-line no-console console.log( colors.red( - `The "--subspace" parameter can only be passed if the "enabled" option is enabled in subspaces.json.` + `The "--subspace" parameter can only be passed if "subspacesEnabled" is set to true in subspaces.json.` ) ); throw new AlreadyReportedError(); From 7a4d754fb6ff8f4f287805437508cf50e252cc07 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:37:19 -0800 Subject: [PATCH 8/8] Fix a typo --- libraries/rush-lib/src/api/RushConfiguration.ts | 5 ++++- libraries/rush-lib/src/api/SubspacesConfiguration.ts | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/api/RushConfiguration.ts b/libraries/rush-lib/src/api/RushConfiguration.ts index b1243d5dcee..793c38a8c19 100644 --- a/libraries/rush-lib/src/api/RushConfiguration.ts +++ b/libraries/rush-lib/src/api/RushConfiguration.ts @@ -1310,7 +1310,10 @@ export class RushConfiguration { const subspace: Subspace | undefined = this._subspacesByName.get(subspaceName); if (!subspace) { // If the name is not even valid, that is more important information than if the subspace doesn't exist - SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.subspacesFeatureEnabled); + SubspacesConfiguration.requireValidSubspaceName( + subspaceName, + this.subspacesConfiguration?.splitWorkspaceCompatibility + ); } return subspace; } diff --git a/libraries/rush-lib/src/api/SubspacesConfiguration.ts b/libraries/rush-lib/src/api/SubspacesConfiguration.ts index a23d0655220..fd18f6fa5a4 100644 --- a/libraries/rush-lib/src/api/SubspacesConfiguration.ts +++ b/libraries/rush-lib/src/api/SubspacesConfiguration.ts @@ -67,7 +67,7 @@ export class SubspacesConfiguration { this.preventSelectingAllSubspaces = !!configuration.preventSelectingAllSubspaces; const subspaceNames: Set = new Set(); for (const subspaceName of configuration.subspaceNames) { - SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.subspacesEnabled); + SubspacesConfiguration.requireValidSubspaceName(subspaceName, this.splitWorkspaceCompatibility); subspaceNames.add(subspaceName); }