Merge pull request microsoft#1769 from iclanton/ianc/prepare-to-publish-rush

[rush] Prepare to publish Rush 5.21.0

Author: iclanton

apps/rush-lib/assets/rush-init/rush.json

@@ -36,18 +36,20 @@
    */
   "pnpmOptions": {
     /**
-    * Specifies the location of the PNPM store.  There are three possible values:
-    *
-    * - "local" - use the "pnpm-store" folder in the current configured temp folder:
-    *   "common/temp/pnpm-store" by default.
-    * - "global" - use PNPM's global store, which has the benefit of being shared
-    *    across multiple repo folders, but the disadvantage of less isolation for builds
-    *    (e.g. bugs or incompatibilities when two repos use different releases of PNPM)
-    *
-    * RUSH_PNPM_STORE_PATH will override the directory that will be used as the store
-    *
-    * The default value is "local".
-    */
+     * Specifies the location of the PNPM store.  There are two possible values:
+     *
+     * - "local" - use the "pnpm-store" folder in the current configured temp folder:
+     *   "common/temp/pnpm-store" by default.
+     * - "global" - use PNPM's global store, which has the benefit of being shared
+     *    across multiple repo folders, but the disadvantage of less isolation for builds
+     *    (e.g. bugs or incompatibilities when two repos use different releases of PNPM)
+     *
+     * RUSH_PNPM_STORE_PATH will override the directory that will be used as the store
+     *
+     * In all cases, the store path will be overridden by the environment variable RUSH_PNPM_STORE_PATH.
+     *
+     * The default value is "local".
+     */
     /*[LINE "HYPOTHETICAL"]*/ "pnpmStore": "local",
 
     /**

apps/rush-lib/src/api/EnvironmentConfiguration.ts

@@ -49,12 +49,11 @@ export const enum EnvironmentVariableNames {
   RUSH_ABSOLUTE_SYMLINKS = 'RUSH_ABSOLUTE_SYMLINKS',
 
   /**
-   * When using PNPM as the package manager this variable will be used to define the path that
+   * When using PNPM as the package manager, this variable can be used to configure the path that
    * PNPM will use as the store directory.
-   * 
+   *
    * If a relative path is used, then the store path will be resolved relative to the process's
-   * current working directory.  The best way to use this environment variable is with absolute
-   * paths.
+   * current working directory.  An absolute path is recommended.
    */
   RUSH_PNPM_STORE_PATH = 'RUSH_PNPM_STORE_PATH'
 }

apps/rush-lib/src/api/LastInstallFlag.ts

@@ -6,53 +6,9 @@ import { PackageManagerName } from './packageManager/PackageManager'
 
 export const LAST_INSTALL_FLAG_FILE_NAME: string = 'last-install.flag';
 
-interface ILastInstallFlagErrorState {
-  oldState: JsonObject;
-  newState: JsonObject;
-}
-
-/**
- * Defines an `Error` of type `LastInstallFlagError` that can be generated by `LastInstallFlag`
- * when it is needed.
- * @internal
- */
-export class LastInstallFlagError extends Error {
-  /**
-   * Creates a new LastInstallFlagError
-   * @param errorKey - The error to generate
-   * @param state - Object containing the old and new LastInstallFlag state
-   */
-  public constructor(errorKey: string, state: ILastInstallFlagErrorState) {
-    super();
-
-    const { oldState, newState } = state;
-
-    switch (errorKey) {
-      case 'storePath': {
-        const oldStorePath: string = oldState.storePath || '<global>';
-        const newStorePath: string = newState.storePath || '<global>';
-        this.message = 
-          "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}`
-        break;
-      }
-      default: {
-        // the `errorKey` didn't have an error key defined.  Throw an Error
-        // so the developer knows.
-        throw new Error(`LastInstallFlagError: an invalid 'errorKey' was defined\nThe key '${errorKey}' does not have an error message defined`);
-        break;
-      }
-    }
-  }
-
-
-}
-
 /**
  * A helper class for managing last-install flags, which are persistent and
- * indicate that something installed in the folder was sucessfully completed.
+ * indicate that something installed in the folder was successfully completed.
  * It also compares state, so that if something like the Node.js version has changed,
  * it can invalidate the last install.
  * @internal
@@ -72,57 +28,61 @@ export class LastInstallFlag {
   }
 
   /**
-   * Returns true if the file exists and the contents match the current state
-   * @param abortOnInvalid - If the current state is not equal to the previous
-   * state, and an the current state causes an error, then throw an exception
-   * with a friendly message
+   * Returns true if the file exists and the contents match the current state.
+   */
+  public isValid(): boolean {
+    return this._isValid(false);
+  }
+
+  /**
+   * Same as isValid(), but with an additional check:  If the current state is not equal to the previous
+   * state, and an the current state causes an error, then throw an exception with a friendly message.
    */
-  public isValid(abortOnInvalid: boolean = false): boolean {
+  public checkValidAndReportStoreIssues(): boolean {
+    return this._isValid(true);
+  }
+
+  private _isValid(checkValidAndReportStoreIssues: boolean): boolean {
     if (!FileSystem.exists(this._path)) {
       return false;
     }
-    let contents: JsonObject;
+
+    let oldState: JsonObject;
     try {
-      contents = JsonFile.load(this._path);
+      oldState = JsonFile.load(this._path);
     } catch (err) {
       return false;
-    } 
-    if (!_.isEqual(contents, this._state)) {
-      if (abortOnInvalid) {
-        const flagError: LastInstallFlagError | void = this._stateHasError(contents);
-        if (flagError) {
-          throw flagError;
-        }
-      }
-      return false;
     }
-    return true;
-  }
 
-  /**
-   * Determine if the new LastInstallFlag state should generate an error
-   * @param oldState - The LastInstallFlag contents read from the disk
-   */
-  private _stateHasError(oldState: JsonObject): LastInstallFlagError | void {
-    const pkgManager: PackageManagerName = this._state.packageManager;
-    switch (pkgManager) {
-      case '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 &&
-            oldState.storePath !== this._state.storePath
-          )
-        ) {
-          return new LastInstallFlagError('storePath', {
-            newState: this._state,
-            oldState,
-          });
+    const newState: JsonObject = this._state;
+
+    if (!_.isEqual(oldState, newState)) {
+      if (checkValidAndReportStoreIssues) {
+        const pkgManager: PackageManagerName = newState.packageManager;
+        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 &&
+              oldState.storePath !== newState.storePath
+            )
+          ) {
+            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}`);
+          }
         }
-        break;
       }
+      return false;
     }
+
+    return true;
   }
 
   /**

apps/rush-lib/src/api/RushConfiguration.ts

@@ -124,21 +124,6 @@ export interface IPnpmOptionsJson {
   resolutionStrategy?: ResolutionStrategy;
 }
 
-/**
- * The paths used when resolving `pnpmStorePath`
- * @internal
- */
-export interface IPnpmOptionsRootPaths {
-  /**
-   * The location of the Rush `common/temp` directory
-   */
-  commonTempFolder: string;
-  /**
-   * Stores the location of the repository root
-   */
-  repoRoot: string;
-}
-
 /**
  * Part of IRushConfigurationJson.
  */
@@ -209,7 +194,7 @@ export interface ICurrentVariantJson {
 export class PnpmOptionsConfiguration {
   /**
    * The method used to resolve the store used by PNPM.
-   * 
+   *
    * @remarks
    * Available options:
    *  - local: Use the standard Rush store path: common/temp/pnpm-store
@@ -219,7 +204,7 @@ export class PnpmOptionsConfiguration {
 
   /**
    * The path for PNPM to use as the store directory.
-   * 
+   *
    * Will be overridden by environment variable RUSH_PNPM_STORE_PATH
    */
   public readonly pnpmStorePath: string;
@@ -254,14 +239,14 @@ export class PnpmOptionsConfiguration {
   public readonly resolutionStrategy: ResolutionStrategy;
 
   /** @internal */
-  public constructor(json: IPnpmOptionsJson, rootPaths: IPnpmOptionsRootPaths) {
+  public constructor(json: IPnpmOptionsJson, commonTempFolder: string) {
     this.pnpmStore = json.pnpmStore || 'local';
     if (EnvironmentConfiguration.pnpmStorePathOverride) {
       this.pnpmStorePath = EnvironmentConfiguration.pnpmStorePathOverride;
     } else if (this.pnpmStore === 'global') {
       this.pnpmStorePath = '';
     } else {
-      this.pnpmStorePath = path.resolve(path.join(rootPaths.commonTempFolder, 'pnpm-store'));
+      this.pnpmStorePath = path.resolve(path.join(commonTempFolder, 'pnpm-store'));
     }
     this.strictPeerDependencies = !!json.strictPeerDependencies;
     this.resolutionStrategy = json.resolutionStrategy || 'fewer-dependencies';
@@ -435,10 +420,8 @@ export class RushConfiguration {
     );
     this._experimentsConfiguration = new ExperimentsConfiguration(experimentsConfigFile);
 
-    this._pnpmOptions = new PnpmOptionsConfiguration(rushConfigurationJson.pnpmOptions || {}, {
-      commonTempFolder: this._commonTempFolder,
-      repoRoot: this._rushJsonFolder
-    });
+    this._pnpmOptions = new PnpmOptionsConfiguration(rushConfigurationJson.pnpmOptions || {},
+      this._commonTempFolder);
     this._yarnOptions = new YarnOptionsConfiguration(rushConfigurationJson.yarnOptions || {});
 
     // TODO: Add an actual "packageManager" field in rush.json

apps/rush-lib/src/api/test/LastInstallFlag.test.ts

@@ -1,6 +1,6 @@
 import * as path from 'path';
 
-import { LastInstallFlag, LastInstallFlagError } from '../LastInstallFlag';
+import { LastInstallFlag } from '../LastInstallFlag';
 import { FileSystem } from '@microsoft/node-core-library';
 
 const TEMP_DIR: string = path.join(__dirname, 'temp');
@@ -79,11 +79,7 @@ describe('LastInstallFlag', () => {
     });
 
     flag1.create();
-    expect(() => { flag2.isValid(true) }).toThrowError(
-      // @ts-ignore
-      LastInstallFlagError // Constructor is private
-    );
-    expect(() => { flag2.isValid(true) }).toThrowError(/PNPM store path/);
+    expect(() => { flag2.checkValidAndReportStoreIssues() }).toThrowError(/PNPM store path/);
   });
 
   it('doesn\'t throw an error if conditions for error aren\'t met', () => {
@@ -96,16 +92,7 @@ describe('LastInstallFlag', () => {
     });
 
     flag1.create();
-    expect(() => { flag2.isValid(true) }).not.toThrow();
-    expect(flag2.isValid(true)).toEqual(false);
+    expect(() => { flag2.checkValidAndReportStoreIssues() }).not.toThrow();
+    expect(flag2.checkValidAndReportStoreIssues()).toEqual(false);
   })
 });
-
-describe('LastInstallFlagError', () => {
-  it('throws an Error if an invalid error key is defined', () => {
-    expect(() => {
-      // @ts-ignore
-      const flagError: LastInstallFlagError = new LastInstallFlagError('non-existant key'); // eslint-disable-line @typescript-eslint/no-unused-vars
-    }).toThrowError();
-  })
-});

apps/rush-lib/src/index.ts

@@ -17,7 +17,6 @@ export {
   PnpmOptionsConfiguration,
   IPnpmOptionsJson as _IPnpmOptionsJson,
   PnpmStoreOptions,
-  IPnpmOptionsRootPaths as _IPnpmOptionsRootPaths,
   YarnOptionsConfiguration
 } from './api/RushConfiguration';
 

apps/rush-lib/src/logic/InstallManager.ts

@@ -820,7 +820,7 @@ export class InstallManager {
         'node_modules');
 
       // This marker file indicates that the last "rush install" completed successfully
-      const markerFileExistedAndWasValidAtStart: boolean = this._commonNodeModulesMarker.isValid(true);
+      const markerFileExistedAndWasValidAtStart: boolean = this._commonNodeModulesMarker.checkValidAndReportStoreIssues();
 
       // If "--clean" or "--full-clean" was specified, or if the last install was interrupted,
       // then we will need to delete the node_modules folder.  Otherwise, we can do an incremental

apps/rush-lib/src/schemas/rush.schema.json

@@ -58,7 +58,7 @@
       "type": "object",
       "properties": {
         "pnpmStore": {
-          "description": "Specifies the location of the PNPM store.  There are two possible values:\n\n\"local\" - uuse the \"pnpm-store\" folder in the current configured temp folder: \"common/temp/pnpm-store\" by default.\n\"global\" - use PNPM's global store, which has the benefit of being shared across multiple repo folders, but the disadvantage of less isolation for builds (e.g. bugs or incompatibilities when two repos use different releases of PNPM)\n\nThe default value is \"local\".\n\nIn all cases, the store path will be overridden by the environment variable RUSH_PNPM_STORE_PATH",
+          "description": "Specifies the location of the PNPM store.  There are two possible values:\n\n\"local\" - use the \"pnpm-store\" folder in the current configured temp folder: \"common/temp/pnpm-store\" by default.\n\"global\" - use PNPM's global store, which has the benefit of being shared across multiple repo folders, but the disadvantage of less isolation for builds (e.g. bugs or incompatibilities when two repos use different releases of PNPM)\n\nIn all cases, the store path will be overridden by the environment variable RUSH_PNPM_STORE_PATH.\n\nThe default value is \"local\".",
           "type": "string",
           "enum": ["local", "global"]
         },

common/changes/@microsoft/rush/ianc-fix-mistake-in-rush-init_2020-03-07-01-35.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "[email protected]"
+}

common/reviews/api/rush-lib.api.md

@@ -152,12 +152,6 @@ export interface _IPnpmOptionsJson {
     strictPeerDependencies?: boolean;
 }
 
-// @internal
-export interface _IPnpmOptionsRootPaths {
-    commonTempFolder: string;
-    repoRoot: string;
-}
-
 // @public
 export interface ITryFindRushJsonLocationOptions {
     showVerbose?: boolean;
@@ -167,9 +161,10 @@ export interface ITryFindRushJsonLocationOptions {
 // @internal
 export class _LastInstallFlag {
     constructor(folderPath: string, state?: JsonObject);
+    checkValidAndReportStoreIssues(): boolean;
     clear(): void;
     create(): void;
-    isValid(abortOnInvalid?: boolean): boolean;
+    isValid(): boolean;
     readonly path: string;
     }
 
@@ -244,7 +239,7 @@ export type PackageManagerName = 'pnpm' | 'npm' | 'yarn';
 // @public
 export class PnpmOptionsConfiguration {
     // @internal
-    constructor(json: _IPnpmOptionsJson, rootPaths: _IPnpmOptionsRootPaths);
+    constructor(json: _IPnpmOptionsJson, commonTempFolder: string);
     readonly pnpmStore: PnpmStoreOptions;
     readonly pnpmStorePath: string;
     readonly resolutionStrategy: ResolutionStrategy;