Skip to content

Commit

Permalink
Put pnpm patches back in common/pnpm-patches when subspaces are not e…
Browse files Browse the repository at this point in the history
…nabled. (microsoft#4876)
  • Loading branch information
iclanton authored Aug 11, 2024
1 parent 9f01b5b commit 2909352
Show file tree
Hide file tree
Showing 20 changed files with 124 additions and 79 deletions.
2 changes: 1 addition & 1 deletion apps/lockfile-explorer/src/utils/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const init = (options: {

const rushConfiguration: RushConfiguration = RushConfiguration.tryLoadFromDefaultLocation()!;
const subspace: Subspace = rushConfiguration.getSubspace(subspaceName);
const workspaceFolder: string = subspace.getSubspaceTempFolder();
const workspaceFolder: string = subspace.getSubspaceTempFolderPath();

const projectsByProjectFolder: Map<string, IRushProjectDetails> = new Map();
for (const project of rushConfiguration.projects) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Revert a breaking change in Rush 5.131.3 where pnpm patches were moved from `common/pnpm-patches` to `common/config/rush/pnpm-patches`.",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"comment": "",
"type": "none",
"packageName": "@rushstack/lockfile-explorer"
}
],
"packageName": "@rushstack/lockfile-explorer",
"email": "[email protected]"
}
6 changes: 4 additions & 2 deletions common/reviews/api/rush-lib.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1466,9 +1466,11 @@ export class Subspace {
// @beta
getRepoStateFilePath(): string;
// @beta
getSubspaceConfigFolder(): string;
getSubspaceConfigFolderPath(): string;
// @beta
getSubspaceTempFolder(): string;
getSubspacePnpmPatchesFolderPath(): string;
// @beta
getSubspaceTempFolderPath(): string;
// @beta
getTempShrinkwrapFilename(): string;
// @beta
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/api/LastInstallFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,5 +210,5 @@ export function getCommonTempFlag(
}
}

return new LastInstallFlag(subspace.getSubspaceTempFolder(), currentState);
return new LastInstallFlag(subspace.getSubspaceTempFolderPath(), currentState);
}
73 changes: 46 additions & 27 deletions libraries/rush-lib/src/api/Subspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ export interface ISubspaceOptions {
}

interface ISubspaceDetail {
subspaceConfigFolder: string;
subspaceTempFolder: string;
subspaceConfigFolderPath: string;
subspacePnpmPatchesFolderPath: string;
subspaceTempFolderPath: string;
tempShrinkwrapFilename: string;
tempShrinkwrapPreinstallFilename: string;
}
Expand Down Expand Up @@ -74,8 +75,8 @@ export class Subspace {
public getPnpmOptions(): PnpmOptionsConfiguration | undefined {
if (!this._cachedPnpmOptionsInitialized) {
// Calculate these outside the try/catch block since their error messages shouldn't be annotated:
const subspaceConfigFolder: string = this.getSubspaceConfigFolder();
const subspaceTempFolder: string = this.getSubspaceTempFolder();
const subspaceConfigFolder: string = this.getSubspaceConfigFolderPath();
const subspaceTempFolder: string = this.getSubspaceTempFolderPath();
try {
this._cachedPnpmOptions = PnpmOptionsConfiguration.loadFromJsonFileOrThrow(
`${subspaceConfigFolder}/${RushConstants.pnpmConfigFilename}`,
Expand All @@ -99,7 +100,8 @@ export class Subspace {
private _ensureDetail(): ISubspaceDetail {
if (!this._detail) {
const rushConfiguration: RushConfiguration = this._rushConfiguration;
let subspaceConfigFolder: string;
let subspaceConfigFolderPath: string;
let subspacePnpmPatchesFolderPath: string;

if (rushConfiguration.subspacesFeatureEnabled) {
if (!rushConfiguration.pnpmOptions.useWorkspaces) {
Expand All @@ -114,7 +116,7 @@ export class Subspace {
// Example: C:\MyRepo\common\config\subspaces\my-subspace
const standardSubspaceConfigFolder: string = `${rushConfiguration.commonFolder}/config/subspaces/${this.subspaceName}`;

subspaceConfigFolder = standardSubspaceConfigFolder;
subspaceConfigFolderPath = standardSubspaceConfigFolder;

if (this._splitWorkspaceCompatibility && this.subspaceName.startsWith('split_')) {
if (FileSystem.exists(standardSubspaceConfigFolder + '/pnpm-lock.yaml')) {
Expand All @@ -132,47 +134,52 @@ export class Subspace {
}
const project: RushConfigurationProject = this._projects[0];

subspaceConfigFolder = `${project.projectFolder}/subspace/${this.subspaceName}`;
subspaceConfigFolderPath = `${project.projectFolder}/subspace/${this.subspaceName}`;

// Ensure that this project does not have it's own pnpmfile.cjs or .npmrc file
if (FileSystem.exists(`${project.projectFolder}/.npmrc`)) {
throw new Error(
`The project level configuration file ${project.projectFolder}/.npmrc is no longer valid. Please use a ${subspaceConfigFolder}/.npmrc file instead.`
`The project level configuration file ${project.projectFolder}/.npmrc is no longer valid. Please use a ${subspaceConfigFolderPath}/.npmrc file instead.`
);
}
if (FileSystem.exists(`${project.projectFolder}/.pnpmfile.cjs`)) {
throw new Error(
`The project level configuration file ${project.projectFolder}/.pnpmfile.cjs is no longer valid. Please use a ${subspaceConfigFolder}/.pnpmfile.cjs file instead.`
`The project level configuration file ${project.projectFolder}/.pnpmfile.cjs is no longer valid. Please use a ${subspaceConfigFolderPath}/.pnpmfile.cjs file instead.`
);
}
}

if (!FileSystem.exists(subspaceConfigFolder)) {
if (!FileSystem.exists(subspaceConfigFolderPath)) {
throw new Error(
`The configuration folder for the "${this.subspaceName}" subspace does not exist: ` +
subspaceConfigFolder
subspaceConfigFolderPath
);
}

subspacePnpmPatchesFolderPath = `${subspaceConfigFolderPath}/${RushConstants.pnpmPatchesCommonFolderName}`;
} else {
// Example: C:\MyRepo\common\config\rush
subspaceConfigFolder = rushConfiguration.commonRushConfigFolder;
subspaceConfigFolderPath = rushConfiguration.commonRushConfigFolder;
// Example: C:\MyRepo\common\pnpm-patches
subspacePnpmPatchesFolderPath = `${rushConfiguration.commonFolder}/${RushConstants.pnpmPatchesCommonFolderName}`;
}

// Example: C:\MyRepo\common\temp
const commonTempFolder: string =
EnvironmentConfiguration.rushTempFolderOverride || rushConfiguration.commonTempFolder;

let subspaceTempFolder: string;
let subspaceTempFolderPath: string;
if (rushConfiguration.subspacesFeatureEnabled) {
// Example: C:\MyRepo\common\temp\my-subspace
subspaceTempFolder = path.join(commonTempFolder, this.subspaceName);
subspaceTempFolderPath = path.join(commonTempFolder, this.subspaceName);
} else {
// Example: C:\MyRepo\common\temp
subspaceTempFolder = commonTempFolder;
subspaceTempFolderPath = commonTempFolder;
}

// Example: C:\MyRepo\common\temp\my-subspace\pnpm-lock.yaml
const tempShrinkwrapFilename: string = subspaceTempFolder + `/${rushConfiguration.shrinkwrapFilename}`;
const tempShrinkwrapFilename: string =
subspaceTempFolderPath + `/${rushConfiguration.shrinkwrapFilename}`;

/// From "C:\MyRepo\common\temp\pnpm-lock.yaml" --> "C:\MyRepo\common\temp\pnpm-lock-preinstall.yaml"
const parsedPath: path.ParsedPath = path.parse(tempShrinkwrapFilename);
Expand All @@ -182,8 +189,9 @@ export class Subspace {
);

this._detail = {
subspaceConfigFolder,
subspaceTempFolder,
subspaceConfigFolderPath,
subspacePnpmPatchesFolderPath,
subspaceTempFolderPath,
tempShrinkwrapFilename,
tempShrinkwrapPreinstallFilename
};
Expand All @@ -197,8 +205,19 @@ export class Subspace {
* Example: `common/config/subspaces/my-subspace`
* @beta
*/
public getSubspaceConfigFolder(): string {
return this._ensureDetail().subspaceConfigFolder;
public getSubspaceConfigFolderPath(): string {
return this._ensureDetail().subspaceConfigFolderPath;
}

/**
* Returns the full path of the folder containing this subspace's configuration files such as `pnpm-lock.yaml`.
*
* Example: `common/config/subspaces/my-subspace/pnpm-patches` (subspaces feature enabled)
* Example: `common/config/pnpm-patches` (subspaces feature disabled)
* @beta
*/
public getSubspacePnpmPatchesFolderPath(): string {
return this._ensureDetail().subspacePnpmPatchesFolderPath;
}

/**
Expand All @@ -207,8 +226,8 @@ export class Subspace {
* Example: `common/temp/subspaces/my-subspace`
* @beta
*/
public getSubspaceTempFolder(): string {
return this._ensureDetail().subspaceTempFolder;
public getSubspaceTempFolderPath(): string {
return this._ensureDetail().subspaceTempFolderPath;
}

/**
Expand Down Expand Up @@ -247,7 +266,7 @@ export class Subspace {
* @beta
*/
public getCommonVersionsFilePath(): string {
return this._ensureDetail().subspaceConfigFolder + '/' + RushConstants.commonVersionsFilename;
return this._ensureDetail().subspaceConfigFolderPath + '/' + RushConstants.commonVersionsFilename;
}

/**
Expand All @@ -257,7 +276,7 @@ export class Subspace {
* @beta
*/
public getPnpmConfigFilePath(): string {
return this._ensureDetail().subspaceConfigFolder + '/' + RushConstants.pnpmConfigFilename;
return this._ensureDetail().subspaceConfigFolderPath + '/' + RushConstants.pnpmConfigFilename;
}

/**
Expand Down Expand Up @@ -299,7 +318,7 @@ export class Subspace {
* @beta
*/
public getRepoStateFilePath(): string {
return this._ensureDetail().subspaceConfigFolder + '/' + RushConstants.repoStateFilename;
return this._ensureDetail().subspaceConfigFolderPath + '/' + RushConstants.repoStateFilename;
}

/**
Expand All @@ -317,7 +336,7 @@ export class Subspace {
* @beta
*/
public getCommittedShrinkwrapFilename(): string {
const subspaceConfigFolderPath: string = this.getSubspaceConfigFolder();
const subspaceConfigFolderPath: string = this.getSubspaceConfigFolderPath();
return path.join(subspaceConfigFolderPath, this._rushConfiguration.shrinkwrapFilename);
}

Expand All @@ -329,7 +348,7 @@ export class Subspace {
* @beta
*/
public getPnpmfilePath(): string {
const subspaceConfigFolderPath: string = this.getSubspaceConfigFolder();
const subspaceConfigFolderPath: string = this.getSubspaceConfigFolderPath();

const pnpmFilename: string = (this._rushConfiguration.packageManagerWrapper as PnpmPackageManager)
.pnpmfileFilename;
Expand Down
21 changes: 12 additions & 9 deletions libraries/rush-lib/src/cli/RushPnpmCommandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import type { IInstallManagerOptions } from '../logic/base/BaseInstallManagerTyp
import { objectsAreDeepEqual } from '../utilities/objectUtilities';
import { Utilities } from '../utilities/Utilities';
import type { Subspace } from '../api/Subspace';
import type { PnpmOptionsConfiguration } from '../logic/pnpm/PnpmOptionsConfiguration';

const RUSH_SKIP_CHECKS_PARAMETER: string = '--rush-skip-checks';

Expand Down Expand Up @@ -135,7 +136,7 @@ export class RushPnpmCommandLineParser {
const subspace: Subspace = rushConfiguration.getSubspace(subspaceName);
this._subspace = subspace;

const workspaceFolder: string = subspace.getSubspaceTempFolder();
const workspaceFolder: string = subspace.getSubspaceTempFolderPath();
const workspaceFilePath: string = path.join(workspaceFolder, 'pnpm-workspace.yaml');

if (!FileSystem.exists(workspaceFilePath)) {
Expand Down Expand Up @@ -374,7 +375,7 @@ export class RushPnpmCommandLineParser {

private async _executeAsync(): Promise<void> {
const rushConfiguration: RushConfiguration = this._rushConfiguration;
const workspaceFolder: string = this._subspace.getSubspaceTempFolder();
const workspaceFolder: string = this._subspace.getSubspaceTempFolderPath();
const pnpmEnvironmentMap: EnvironmentMap = new EnvironmentMap(process.env);
pnpmEnvironmentMap.set('NPM_CONFIG_WORKSPACE_DIR', workspaceFolder);

Expand Down Expand Up @@ -442,8 +443,8 @@ export class RushPnpmCommandLineParser {
return;
}

const subspaceTempFolder: string = this._subspace.getSubspaceTempFolder();
const subspaceConfigFolder: string = this._subspace.getSubspaceConfigFolder();
const subspaceTempFolder: string = this._subspace.getSubspaceTempFolderPath();
const subspaceConfigFolder: string = this._subspace.getSubspaceConfigFolderPath();

switch (commandName) {
case 'patch-commit': {
Expand All @@ -454,7 +455,7 @@ export class RushPnpmCommandLineParser {
if (this._subspace.getPnpmOptions() === undefined) {
this._terminal.writeErrorLine(
`The "rush-pnpm patch-commit" command cannot proceed without a pnpm-config.json file.` +
` Create one in this folder: ${this._subspace.getSubspaceConfigFolder()}`
` Create one in this folder: ${subspaceConfigFolder}`
);
break;
}
Expand All @@ -464,13 +465,15 @@ export class RushPnpmCommandLineParser {
const commonPackageJson: JsonObject = JsonFile.load(commonPackageJsonFilename);
const newGlobalPatchedDependencies: Record<string, string> | undefined =
commonPackageJson?.pnpm?.patchedDependencies;
const pnpmOptions: PnpmOptionsConfiguration | undefined = this._subspace.getPnpmOptions();
const currentGlobalPatchedDependencies: Record<string, string> | undefined =
this._subspace.getPnpmOptions()?.globalPatchedDependencies;
pnpmOptions?.globalPatchedDependencies;

if (!objectsAreDeepEqual(currentGlobalPatchedDependencies, newGlobalPatchedDependencies)) {
const commonTempPnpmPatchesFolder: string = `${subspaceTempFolder}/${RushConstants.pnpmPatchesFolderName}`;
const rushPnpmPatchesFolder: string = `${subspaceConfigFolder}/${RushConstants.pnpmPatchesCommonFolderName}`;
// Copy (or delete) common\temp\subspace\patches\ --> subspace\pnpm-patches\
const rushPnpmPatchesFolder: string = this._subspace.getSubspacePnpmPatchesFolderPath();

// Copy (or delete) common\temp\subspace\patches\ --> common\config\pnpm-patches\ OR common\config\rush\pnpm-patches\
if (FileSystem.exists(commonTempPnpmPatchesFolder)) {
FileSystem.ensureEmptyFolder(rushPnpmPatchesFolder);
// eslint-disable-next-line no-console
Expand All @@ -490,7 +493,7 @@ export class RushPnpmCommandLineParser {
}

// Update patchedDependencies to pnpm configuration file
this._subspace.getPnpmOptions()?.updateGlobalPatchedDependencies(newGlobalPatchedDependencies);
pnpmOptions?.updateGlobalPatchedDependencies(newGlobalPatchedDependencies);

// Rerun installation to update
await this._doRushUpdateAsync();
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/cli/actions/DeployAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export class DeployAction extends BaseRushAction {
}

if (!scenarioConfiguration.json.omitPnpmWorkaroundLinks) {
subspace.pnpmInstallFolder = project.subspace.getSubspaceTempFolder();
subspace.pnpmInstallFolder = project.subspace.getSubspaceTempFolderPath();
}
subspaces.set(subspace.subspaceName, subspace);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ export class PhasedScriptAction extends BaseScriptAction<IPhasedCommandConfig> {
if (!this._runsBeforeInstall) {
// TODO: Replace with last-install.flag when "rush link" and "rush unlink" are removed
const lastLinkFlag: FlagFile = new FlagFile(
this.rushConfiguration.defaultSubspace.getSubspaceTempFolder(),
this.rushConfiguration.defaultSubspace.getSubspaceTempFolderPath(),
RushConstants.lastLinkFlagFilename,
{}
);
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/cli/test/TestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export async function getCommandLineParserInstanceAsync(
// Bulk tasks are hard-coded to expect install to have been completed. So, ensure the last-link.flag
// file exists and is valid
await new FlagFile(
parser.rushConfiguration.defaultSubspace.getSubspaceTempFolder(),
parser.rushConfiguration.defaultSubspace.getSubspaceTempFolderPath(),
RushConstants.lastLinkFlagFilename,
{}
).createAsync();
Expand Down
6 changes: 3 additions & 3 deletions libraries/rush-lib/src/logic/TempProjectHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class TempProjectHelper {
* Deletes the existing tarball and creates a tarball for the given rush project
*/
public createTempProjectTarball(rushProject: RushConfigurationProject): void {
FileSystem.ensureFolder(path.resolve(this._subspace.getSubspaceTempFolder(), 'projects'));
FileSystem.ensureFolder(path.resolve(this._subspace.getSubspaceTempFolderPath(), 'projects'));
const tarballFile: string = this.getTarballFilePath(rushProject);
const tempProjectFolder: string = this.getTempProjectFolder(rushProject);

Expand Down Expand Up @@ -64,7 +64,7 @@ export class TempProjectHelper {
*/
public getTarballFilePath(project: RushConfigurationProject): string {
return path.join(
this._subspace.getSubspaceTempFolder(),
this._subspace.getSubspaceTempFolderPath(),
RushConstants.rushTempProjectsFolderName,
`${project.unscopedTempProjectName}.tgz`
);
Expand All @@ -73,7 +73,7 @@ export class TempProjectHelper {
public getTempProjectFolder(rushProject: RushConfigurationProject): string {
const unscopedTempProjectName: string = rushProject.unscopedTempProjectName;
return path.join(
this._subspace.getSubspaceTempFolder(),
this._subspace.getSubspaceTempFolderPath(),
RushConstants.rushTempProjectsFolderName,
unscopedTempProjectName
);
Expand Down
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/logic/UnlinkManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class UnlinkManager {
}

await new FlagFile(
this._rushConfiguration.defaultSubspace.getSubspaceTempFolder(),
this._rushConfiguration.defaultSubspace.getSubspaceTempFolderPath(),
RushConstants.lastLinkFlagFilename,
{}
).clearAsync();
Expand Down
Loading

0 comments on commit 2909352

Please sign in to comment.