Merge pull request #4300 from microsoft/octogonz/update-autoinstaller…

…-fix

[rush] Fix issue where "rush update-autoinstaller" doesn't perform a full upgrade

common/changes/@microsoft/rush/octogonz-update-autoinstaller-fix_2023-08-29-20-22.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Fix an issue where `rush update-autoinstaller` sometimes did not fully upgrade the lockfile",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

common/changes/@microsoft/rush/octogonz-update-autoinstaller-fix_2023-08-29-20-23.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@microsoft/rush",
+      "comment": "Fix an issue where \"undefined\" was sometimes printed instead of a blank line",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush"
+}

libraries/rush-lib/src/logic/Autoinstaller.ts

@@ -15,6 +15,7 @@ import type { RushGlobalFolder } from '../api/RushGlobalFolder';
 import { RushConstants } from './RushConstants';
 import { LastInstallFlag } from '../api/LastInstallFlag';
 import { RushCommandLineParser } from '../cli/RushCommandLineParser';
+import { PnpmPackageManager } from '../api/packageManager/PnpmPackageManager';
 
 interface IAutoinstallerOptions {
   autoinstallerName: string;
@@ -167,7 +168,7 @@ export class Autoinstaller {
 
     const autoinstallerPackageJsonPath: string = path.join(this.folderFullPath, 'package.json');
 
-    if (!FileSystem.exists(autoinstallerPackageJsonPath)) {
+    if (!(await FileSystem.existsAsync(autoinstallerPackageJsonPath))) {
       throw new Error(`The specified autoinstaller path does not exist: ` + autoinstallerPackageJsonPath);
     }
 
@@ -177,10 +178,23 @@ export class Autoinstaller {
 
     let oldFileContents: string = '';
 
-    if (FileSystem.exists(this.shrinkwrapFilePath)) {
+    if (await FileSystem.existsAsync(this.shrinkwrapFilePath)) {
       oldFileContents = FileSystem.readFile(this.shrinkwrapFilePath, { convertLineEndings: NewlineKind.Lf });
       this._logIfConsoleOutputIsNotRestricted('Deleting ' + this.shrinkwrapFilePath);
-      FileSystem.deleteFile(this.shrinkwrapFilePath);
+      await FileSystem.deleteFileAsync(this.shrinkwrapFilePath);
+      if (this._rushConfiguration.packageManager === 'pnpm') {
+        // Workaround for https://github.com/pnpm/pnpm/issues/1890
+        //
+        // When "rush update-autoinstaller" is run, Rush deletes "common/autoinstallers/my-task/pnpm-lock.yaml"
+        // so that a new lockfile will be generated. However "pnpm install" by design will try to recover
+        // "pnpm-lock.yaml" from "my-task/node_modules/.pnpm/lock.yaml", which may prevent a full upgrade.
+        // Deleting both files ensures that a new lockfile will always be generated.
+        const pnpmPackageManager: PnpmPackageManager = this._rushConfiguration
+          .packageManagerWrapper as PnpmPackageManager;
+        await FileSystem.deleteFileAsync(
+          path.join(this.folderFullPath, pnpmPackageManager.internalShrinkwrapRelativePath)
+        );
+      }
     }
 
     // Detect a common mistake where PNPM prints "Already up-to-date" without creating a shrinkwrap file
@@ -218,13 +232,13 @@ export class Autoinstaller {
       this._logIfConsoleOutputIsNotRestricted();
     }
 
-    if (!FileSystem.exists(this.shrinkwrapFilePath)) {
+    if (!(await FileSystem.existsAsync(this.shrinkwrapFilePath))) {
       throw new Error(
         'The package manager did not create the expected shrinkwrap file: ' + this.shrinkwrapFilePath
       );
     }
 
-    const newFileContents: string = FileSystem.readFile(this.shrinkwrapFilePath, {
+    const newFileContents: string = await FileSystem.readFileAsync(this.shrinkwrapFilePath, {
       convertLineEndings: NewlineKind.Lf
     });
     if (oldFileContents !== newFileContents) {
@@ -239,7 +253,7 @@ export class Autoinstaller {
 
   private _logIfConsoleOutputIsNotRestricted(message?: string): void {
     if (!this._restrictConsoleOutput) {
-      console.log(message);
+      console.log(message ?? '');
     }
   }
 }

libraries/rush-lib/src/logic/base/BaseInstallManager.ts

@@ -755,12 +755,10 @@ ${gitLfsHookHandling}
       if (this.rushConfiguration.packageManager === 'pnpm') {
         // Workaround for https://github.com/pnpm/pnpm/issues/1890
         //
-        // When "rush update --full" is run, rush deletes common/temp/pnpm-lock.yaml so that
-        // a new lockfile can be generated. But because of the above bug "pnpm install" would
-        // respect "common/temp/node_modules/.pnpm-lock.yaml" and thus would not generate a
-        // new lockfile. Deleting this file in addition to deleting common/temp/pnpm-lock.yaml
-        // ensures that a new lockfile will be generated with "rush update --full".
-
+        // When "rush update --full" is run, Rush deletes "common/temp/pnpm-lock.yaml"
+        // so that a new lockfile will be generated. However "pnpm install" by design will try to recover
+        // "pnpm-lock.yaml" from "common/temp/node_modules/.pnpm/lock.yaml", which may prevent a full upgrade.
+        // Deleting both files ensures that a new lockfile will always be generated.
         const pnpmPackageManager: PnpmPackageManager = this.rushConfiguration
           .packageManagerWrapper as PnpmPackageManager;