Skip to content
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

[rush] Fix an issue where a "Current PNPM store path does not match the last one used." error will erroneously get thrown on Windows with an unchanged path, but with a forward slash instead of a backslash. #3660

Merged
merged 2 commits into from
Sep 28, 2022
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
10 changes: 10 additions & 0 deletions common/changes/@microsoft/rush/fix-rush-bug_2022-09-28-20-55.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Fix an error message that always says to run \"rush update --purge\" even if the user only needs to run \"rush install --purge.\"",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
10 changes: 10 additions & 0 deletions common/changes/@microsoft/rush/main_2022-09-28-20-46.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Fix an issue where a \"Current PNPM store path does not match the last one used.\" error will erroneously get thrown on Windows with an unchanged path, but with a forward slash instead of a backslash.",
"type": "none"
}
],
"packageName": "@microsoft/rush"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@rushstack/node-core-library",
"comment": "Add a Path.convertToPlatformDefault API to convert a path to use the platform-default slashes.",
"type": "minor"
}
],
"packageName": "@rushstack/node-core-library"
}
1 change: 1 addition & 0 deletions common/reviews/api/node-core-library.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ export class PackageNameParser {
// @public
export class Path {
static convertToBackslashes(inputPath: string): string;
static convertToPlatformDefault(inputPath: string): string;
static convertToSlashes(inputPath: string): string;
static formatConcisely(options: IPathFormatConciselyOptions): string;
static formatFileLocation(options: IPathFormatFileLocationOptions): string;
Expand Down
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 @@ -526,7 +526,7 @@ export interface _IYarnOptionsJson extends IPackageManagerOptionsJsonBase {
// @internal
export class _LastInstallFlag {
constructor(folderPath: string, state?: JsonObject);
checkValidAndReportStoreIssues(): boolean;
checkValidAndReportStoreIssues(rushVerb: string): boolean;
clear(): void;
create(): void;
protected get flagName(): string;
Expand Down
6 changes: 6 additions & 0 deletions libraries/node-core-library/src/Path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ export class Path {
public static convertToBackslashes(inputPath: string): string {
return inputPath.replace(/\//g, '\\');
}
/**
* Replaces slashes or backslashes with the appropriate slash for the current operating system.
*/
public static convertToPlatformDefault(inputPath: string): string {
return path.sep === '/' ? Path.convertToSlashes(inputPath) : Path.convertToBackslashes(inputPath);
}

/**
* Returns true if the specified path is a relative path and does not use `..` to walk upwards.
Expand Down
40 changes: 24 additions & 16 deletions libraries/rush-lib/src/api/LastInstallFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import * as path from 'path';

import { FileSystem, JsonFile, JsonObject, Import } from '@rushstack/node-core-library';
import { FileSystem, JsonFile, JsonObject, Import, Path } from '@rushstack/node-core-library';

import { PackageManagerName } from './packageManager/PackageManager';
import { RushConfiguration } from './RushConfiguration';
Expand Down Expand Up @@ -46,11 +46,13 @@ export class LastInstallFlag {
*
* @internal
*/
public checkValidAndReportStoreIssues(): boolean {
return this._isValid(true);
public checkValidAndReportStoreIssues(rushVerb: string): boolean {
return this._isValid(true, rushVerb);
}

private _isValid(checkValidAndReportStoreIssues: boolean): boolean {
private _isValid(checkValidAndReportStoreIssues: false, rushVerb?: string): boolean;
private _isValid(checkValidAndReportStoreIssues: true, rushVerb: string): boolean;
private _isValid(checkValidAndReportStoreIssues: boolean, rushVerb: string = 'update'): boolean {
let oldState: JsonObject;
try {
oldState = JsonFile.load(this._path);
Expand All @@ -66,19 +68,25 @@ export class LastInstallFlag {
if (pkgManager === 'pnpm') {
if (
// Only throw an error if the package manager hasn't changed from PNPM
oldState.packageManager === pkgManager &&
// Throw if the store path changed
oldState.storePath !== newState.storePath
oldState.packageManager === pkgManager
) {
const oldStorePath: string = oldState.storePath || '<global>';
const newStorePath: string = newState.storePath || '<global>';

throw new Error(
'Current PNPM store path does not match the last one used. This may cause inconsistency in your builds.\n\n' +
'If you wish to install with the new store path, please run "rush update --purge"\n\n' +
`Old Path: ${oldStorePath}\n` +
`New Path: ${newStorePath}`
);
const normalizedOldStorePath: string = oldState.storePath
? Path.convertToPlatformDefault(oldState.storePath)
: '<global>';
const normalizedNewStorePath: string = newState.storePath
? Path.convertToPlatformDefault(newState.storePath)
: '<global>';
if (
// Throw if the store path changed
normalizedOldStorePath !== normalizedNewStorePath
) {
throw new Error(
'Current PNPM store path does not match the last one used. This may cause inconsistency in your builds.\n\n' +
`If you wish to install with the new store path, please run "rush ${rushVerb} --purge"\n\n` +
`Old Path: ${normalizedOldStorePath}\n` +
`New Path: ${normalizedNewStorePath}`
);
}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions libraries/rush-lib/src/api/test/LastInstallFlag.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe(LastInstallFlag.name, () => {

flag1.create();
expect(() => {
flag2.checkValidAndReportStoreIssues();
flag2.checkValidAndReportStoreIssues('install');
}).toThrowError(/PNPM store path/);
});

Expand All @@ -97,8 +97,8 @@ describe(LastInstallFlag.name, () => {

flag1.create();
expect(() => {
flag2.checkValidAndReportStoreIssues();
flag2.checkValidAndReportStoreIssues('install');
}).not.toThrow();
expect(flag2.checkValidAndReportStoreIssues()).toEqual(false);
expect(flag2.checkValidAndReportStoreIssues('install')).toEqual(false);
});
});
5 changes: 4 additions & 1 deletion libraries/rush-lib/src/logic/base/BaseInstallManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,10 @@ export abstract class BaseInstallManager {
// "--purge" was specified, or if the last install was interrupted, then we will
// need to perform a clean install. Otherwise, we can do an incremental install.
const cleanInstall: boolean =
isFilteredInstall || !this._commonTempInstallFlag.checkValidAndReportStoreIssues();
isFilteredInstall ||
!this._commonTempInstallFlag.checkValidAndReportStoreIssues(
this.options.allowShrinkwrapUpdates ? 'update' : 'install'
);

// Allow us to defer the file read until we need it
const canSkipInstall: () => boolean = () => {
Expand Down