Ensure NPM Windows Installs Work Correctly (#2059)

Fixes: #2056 This PR fixes the reported bug that `rover` will not install correctly on Windows when using an `npm` installer. The problem was that the renaming function was using the word `rover` rather than `rover.exe` as it should have done when calling the rename.
apollographql · Aug 19, 2024 · 23b4aad · 23b4aad
1 parent 592cf06
commit 23b4aad
Show file tree

Hide file tree

Showing 10 changed files with 206 additions and 27 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -104,11 +104,17 @@ executors:
     environment:
       XTASK_TARGET: "x86_64-unknown-linux-gnu"
 
-  node_js:
+  node_js_nix:
     docker:
-      - image: node:lts
+      - image: node:20.16.0
     resource_class: medium
 
+  node_js_windows:
+    machine:
+      image: "windows-server-2019-vs2019:2022.08.1"
+      shell: powershell.exe -ExecutionPolicy Bypass
+    resource_class: windows.medium
+
 
 tag_matches_prerelease: &tag_matches_prerelease
   matches:
@@ -171,10 +177,11 @@ workflows:
               rust_channel: [stable]
               command: [integration-test]
       - install_js:
-          name: Test installation for Javascript Package Managers (<< matrix.package_manager >>)
+          name: Test installation for Javascript Package Managers (<< matrix.package_manager >> on << matrix.platform >>)
           matrix:
             parameters:
               package_manager: [npm, npm_global, pnpm]
+              platform: [windows, nix]
       - node/test:
           name: Test NPM Installer Scripts
           app-dir: "~/project/installers/npm"
@@ -216,10 +223,11 @@ workflows:
           <<: *run_release
 
       - install_js:
-          name: Test installation for Javascript Package Managers (<< matrix.package_manager >>)
+          name: Test installation for Javascript Package Managers (<< matrix.package_manager >> on << matrix.platform >>)
           matrix:
             parameters:
               package_manager: [ npm, npm_global, pnpm ]
+              platform: [windows, nix]
           <<: *run_release
 
       - node/test:
@@ -244,9 +252,12 @@ workflows:
             - "Run cargo tests + studio integration tests (stable rust on amd_windows)"
             - "Run studio integration tests in GitHub Actions (amd_macos)"
             - "Run supergraph-demo tests (stable rust on amd_ubuntu)"
-            - "Test installation for Javascript Package Managers (npm)"
-            - "Test installation for Javascript Package Managers (npm_global)"
-            - "Test installation for Javascript Package Managers (pnpm)"
+            - "Test installation for Javascript Package Managers (npm on nix)"
+            - "Test installation for Javascript Package Managers (npm_global on nix)"
+            - "Test installation for Javascript Package Managers (pnpm on nix)"
+            - "Test installation for Javascript Package Managers (npm on windows)"
+            - "Test installation for Javascript Package Managers (npm_global on windows)"
+            - "Test installation for Javascript Package Managers (pnpm on windows)"
             - "Test NPM Installer Scripts"
           <<: *run_release
 
@@ -348,15 +359,42 @@ jobs:
       package_manager:
         type: enum
         enum: ["npm", "npm_global", "pnpm"]
-    executor: node_js
+      platform:
+        type: enum
+        enum: ["nix", "windows"]
+    executor: node_js_<<parameters.platform>>
     steps:
       - checkout:
           path: "rover"
-      - run:
-          name: "Invoke Install Scripts"
-          command: |
-            cd rover/.circleci/scripts
-            ./install_<< parameters.package_manager >>.sh
+      - when:
+          condition:
+            equal: ["nix", <<parameters.platform>>]
+          steps:
+            - run:
+                name: "Invoke Install Scripts (Unix)"
+                command: |
+                  cd rover/.circleci/scripts/<<parameters.platform>>
+                  ./install_<< parameters.package_manager >>.sh
+      - when:
+          condition:
+            equal: ["windows", <<parameters.platform>>]
+          steps:
+            - run:
+                name: "Invoke Install Scripts (Windows)"
+                command: |
+                  Write-Output "Installing Volta"
+                  choco install volta
+                  refreshenv
+                  Write-Output "Installing Node & NPM"
+                  volta install [email protected]
+                  Write-Output "Checking Node & NPM version"
+                  node --version
+                  npm --version
+                  
+                  $script_location=Join-Path rover\.circleci\scripts << parameters.platform >>
+                  Set-Location $script_location
+                  .\install_<< parameters.package_manager >>.ps1
+        
 
 # reusable command snippets can be referred to in any `steps` object
 commands:

diff --git a/.circleci/scripts/install_npm.sh → .circleci/scripts/nix/install_npm.sh b/.circleci/scripts/install_npm.sh → .circleci/scripts/nix/install_npm.sh
@@ -2,7 +2,7 @@
 set -euo pipefail
 
 SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-INSTALLERS_DIR="$SCRIPT_DIR/../../installers/npm"
+INSTALLERS_DIR="$SCRIPT_DIR/../../../installers/npm"
 
 cd "$(mktemp -d)"
 echo "Created test directory"

diff --git a/.circleci/scripts/install_npm_global.sh → .circleci/scripts/nix/install_npm_global.sh b/.circleci/scripts/install_npm_global.sh → .circleci/scripts/nix/install_npm_global.sh
@@ -2,7 +2,7 @@
 set -euo pipefail
 
 SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-INSTALLERS_DIR="$SCRIPT_DIR/../../installers/npm"
+INSTALLERS_DIR="$SCRIPT_DIR/../../../installers/npm"
 
 cd "$(mktemp -d)"
 echo "Created test directory"

diff --git a/.circleci/scripts/install_pnpm.sh → .circleci/scripts/nix/install_pnpm.sh b/.circleci/scripts/install_pnpm.sh → .circleci/scripts/nix/install_pnpm.sh
@@ -2,7 +2,7 @@
 set -euo pipefail
 
 SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-INSTALLERS_DIR="$SCRIPT_DIR/../../installers/npm"
+INSTALLERS_DIR="$SCRIPT_DIR/../../../installers/npm"
 
 cd "$(mktemp -d)"
 echo "Created test directory"
@@ -14,7 +14,7 @@ echo "Installed pnpm"
 npm --prefix "$INSTALLERS_DIR" version --allow-same-version 0.23.0
 echo "Temporarily patched package.json to fixed stable binary"
 pnpm init
-pnpm add "file:$SCRIPT_DIR/../../installers/npm"
+pnpm add "file:$INSTALLERS_DIR"
 echo "Installed rover as pnpm package"
 cd node_modules/.bin/
 echo "Checking version"

diff --git a/.circleci/scripts/windows/install_npm.ps1 b/.circleci/scripts/windows/install_npm.ps1
@@ -0,0 +1,41 @@
+$ErrorActionPreference = "Stop"
+
+Function New-TemporaryFolder {
+    # Make a new folder based upon a TempFileName
+    $TEMP_PATH=[System.IO.Path]::GetTempPath()
+    $T= Join-Path $TEMP_PATH tmp$([convert]::tostring((get-random 65535),16).padleft(4,'0'))
+    New-Item -ItemType Directory -Path $T
+}
+
+# Find the installers directory
+$installer_dir = [System.IO.Path]::Combine($PSScriptRoot, "..", "..", "..", "installers", "npm")
+Write-Output "Found installers directory at $installer_dir"
+
+# Create a temporary folder for the test
+$test_dir = New-TemporaryFolder
+Write-Output "Created test directory at $test_dir"
+Set-Location $test_dir
+# Initialise an empty NPM package
+npm init -y
+Write-Output "Initialised new npm package"
+
+# The choice of version here is arbitrary (we just need something we know exists) so that we can test if the
+# installer works, given an existing version. This way we're not at the mercy of whether the binary that corresponds
+# to the latest commit exists.
+npm --prefix "$installer_dir" version --allow-same-version 0.23.0
+Write-Output "Temporarily patched package.json to fixed stable binary"
+
+# Install all the dependencies, including `rover`
+npm install --install-links=true "$installer_dir"
+Write-Output "Installed rover as local npm package"
+
+# Move to the installed location
+$node_modules_path=[System.IO.Path]::Combine($test_dir, "node_modules", ".bin")
+Set-Location $node_modules_path
+
+# Check the version
+Write-Output "Checking version"
+$dir_sep=[IO.Path]::DirectorySeparatorChar
+$rover_command=".${dir_sep}rover --version"
+Invoke-Expression $rover_command
+Write-Output "Checked version, all ok!"
diff --git a/.circleci/scripts/windows/install_npm_global.ps1 b/.circleci/scripts/windows/install_npm_global.ps1
@@ -0,0 +1,32 @@
+$ErrorActionPreference = "Stop"
+
+Function New-TemporaryFolder {
+    # Make a new folder based upon a TempFileName
+    $TEMP_PATH=[System.IO.Path]::GetTempPath()
+    $T= Join-Path $TEMP_PATH tmp$([convert]::tostring((get-random 65535),16).padleft(4,'0'))
+    New-Item -ItemType Directory -Path $T
+}
+
+# Find the installers directory
+$installer_dir = [System.IO.Path]::Combine($PSScriptRoot, "..", "..", "..", "installers", "npm")
+Write-Output "Found installers directory at $installer_dir"
+
+# Create a temporary folder for the test
+$test_dir = New-TemporaryFolder
+Write-Output "Created test directory at $test_dir"
+Set-Location $test_dir
+
+# The choice of version here is arbitrary (we just need something we know exists) so that we can test if the
+# installer works, given an existing version. This way we're not at the mercy of whether the binary that corresponds
+# to the latest commit exists.
+npm --prefix "$installer_dir" version --allow-same-version 0.23.0
+Write-Output "Temporarily patched package.json to fixed stable binary"
+
+# Install all the dependencies, including `rover`
+npm install --install-links=true -g "$installer_dir"
+Write-Output "Installed rover as global npm package"
+
+# Check the version
+Write-Output "Checking version"
+rover --version
+Write-Output "Checked version, all ok!"
diff --git a/.circleci/scripts/windows/install_pnpm.ps1 b/.circleci/scripts/windows/install_pnpm.ps1
@@ -0,0 +1,41 @@
+$ErrorActionPreference = "Stop"
+
+Function New-TemporaryFolder {
+    # Make a new folder based upon a TempFileName
+    $TEMP_PATH=[System.IO.Path]::GetTempPath()
+    $T= Join-Path $TEMP_PATH tmp$([convert]::tostring((get-random 65535),16).padleft(4,'0'))
+    New-Item -ItemType Directory -Path $T
+}
+
+# Find the installers directory
+$installer_dir = [System.IO.Path]::Combine($PSScriptRoot, "..", "..", "..", "installers", "npm")
+Write-Output "Found installers directory at $installer_dir"
+
+# Create a temporary folder for the test
+$test_dir = New-TemporaryFolder
+Write-Output "Created test directory at $test_dir"
+Set-Location $test_dir
+# Install pnpm
+npm install -g pnpm@v9.3.0
+
+# The choice of version here is arbitrary (we just need something we know exists) so that we can test if the
+# installer works, given an existing version. This way we're not at the mercy of whether the binary that corresponds
+# to the latest commit exists.
+npm --prefix "$installer_dir" version --allow-same-version 0.23.0
+Write-Output "Temporarily patched package.json to fixed stable binary"
+
+# Install all the dependencies, including `rover`
+pnpm init
+pnpm add "file:$installer_dir"
+Write-Output "Installed rover as local npm package"
+
+# Move to the installed location
+$node_modules_path=[System.IO.Path]::Combine($test_dir, "node_modules", ".bin")
+Set-Location $node_modules_path
+
+# Check the version
+Write-Output "Checking version"
+$dir_sep=[IO.Path]::DirectorySeparatorChar
+$rover_command=".${dir_sep}rover --version"
+Invoke-Expression $rover_command
+Write-Output "Checked version, all ok!"
diff --git a/installers/npm/binary.js b/installers/npm/binary.js
@@ -25,37 +25,39 @@ const supportedPlatforms = [
     ARCHITECTURE: "x64",
     RUST_TARGET: "x86_64-pc-windows-msvc",
     BINARY_NAME: `${name}-${version}.exe`,
+    RAW_NAME: `${name}.exe`
   },
   {
     TYPE: "Linux",
     ARCHITECTURE: "x64",
     RUST_TARGET: "x86_64-unknown-linux-gnu",
     BINARY_NAME: `${name}-${version}`,
+    RAW_NAME: `${name}`
   },
   {
     TYPE: "Linux",
     ARCHITECTURE: "arm64",
     RUST_TARGET: "aarch64-unknown-linux-gnu",
     BINARY_NAME: `${name}-${version}`,
+    RAW_NAME: `${name}`
   },
   {
     TYPE: "Darwin",
     ARCHITECTURE: "x64",
     RUST_TARGET: "x86_64-apple-darwin",
     BINARY_NAME: `${name}-${version}`,
+    RAW_NAME: `${name}`
   },
   {
     TYPE: "Darwin",
     ARCHITECTURE: "arm64",
     RUST_TARGET: "aarch64-apple-darwin",
     BINARY_NAME: `${name}-${version}`,
+    RAW_NAME: `${name}`
   },
 ];
 
-const getPlatform = () => {
-  const type = os.type();
-  const architecture = os.arch();
-
+const getPlatform = (type = os.type(), architecture = os.arch()) => {
   for (let supportedPlatform of supportedPlatforms) {
     if (
       type === supportedPlatform.TYPE &&
@@ -102,7 +104,7 @@ const getPlatform = () => {
 
 /*! Copyright (c) 2019 Avery Harnish - MIT License */
 class Binary {
-  constructor(name, url, installDirectory) {
+  constructor(name, raw_name, url, installDirectory) {
     let errors = [];
     if (typeof url !== "string") {
       errors.push("url must be a string");
@@ -132,6 +134,7 @@ class Binary {
     }
     this.url = url;
     this.name = name;
+    this.raw_name = raw_name;
     this.installDirectory = installDirectory;
 
     if (!existsSync(this.installDirectory)) {
@@ -176,7 +179,7 @@ class Binary {
         });
       })
       .then(() => {
-        fs.renameSync(join(this.installDirectory, name), this.binaryPath);
+        fs.renameSync(join(this.installDirectory, this.raw_name), this.binaryPath);
         if (!suppressLogs) {
           console.error(`${this.name} has been installed!`);
         }
@@ -212,8 +215,7 @@ class Binary {
   }
 }
 
-const getBinary = (overrideInstallDirectory) => {
-  const platform = getPlatform();
+const getBinary = (overrideInstallDirectory, platform = getPlatform()) => {
   const download_host = process.env.npm_config_apollo_rover_download_host || process.env.APOLLO_ROVER_DOWNLOAD_HOST || 'https://rover.apollo.dev'
   // the url for this binary is constructed from values in `package.json`
   // https://rover.apollo.dev/tar/rover/x86_64-unknown-linux-gnu/v0.4.8
@@ -224,7 +226,7 @@ const getBinary = (overrideInstallDirectory) => {
   if (overrideInstallDirectory != null && overrideInstallDirectory !== "") {
     installDirectory = overrideInstallDirectory
   }
-  let binary = new Binary(platform.BINARY_NAME, url, installDirectory);
+  let binary = new Binary(platform.BINARY_NAME, platform.RAW_NAME, url, installDirectory);
 
   // setting this allows us to extract supergraph plugins to the proper directory
   // the variable itself is read in Rust code