Skip to content

[rush] Put pnpm patches back in common/pnpm-patches when subspaces are not enabled. #4876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 11, 2024
Merged
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
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": "iclanton@users.noreply.github.com"
}
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
Loading