Rover 0.23.0 doesn't work correctly with pnpm

[boccob]

Description

In #1819 there was introduced a new mechanism to load binary files. It utilises rover/node_modules/.bin folder with rover file as a binary. However, with pnpm, this path is already taken for shell script that triggers execution of run.js. In the end, it leads to infinite loop of spawning new node processes.

Steps to reproduce

Install rover 0.23.0 using pnpm and try to run any CLI command.

Expected result

Command is executed.

Actual result

A lot of new node.js processes are spawned but without any result.

Environment

• Operating system and version: macOS 14.3.1
• Shell (bash/zsh/powershell): zsh
• Version of Rover (rover --help): 0.23.0
• Installation method: pnpm

[awx-doma-lei]

+1 similar issue here.

I use rover with pnpm --package @apollo/rover dlx in my GitLab CI job (image: node:20.10.0-alpine). And the latest rover release is causing infinite logs as shown in the screenshot, and eventually getting the job killed due to memory limit. By pinning rover to 0.22.0 the issue is gone.

[dancallaghan]

We're experiencing a similar issue with pnpm.

Out of interest, I diffed the original code in the binary-install package vs. the inlined version, and it looks like code related to specifying the version was removed. Maybe the version number was originally preventing this naming conflict from occurring?

diff --git 1/original.js 2/inline.js
index 85b3b52..247e37d 100644
--- 1/original.js
+++ 2/inline.js
@@ -1,5 +1,5 @@
 class Binary {
-  constructor(name, url, version, config) {
+  constructor(name, url, config) {
     let errors = [];
     if (typeof url !== "string") {
       errors.push("url must be a string");
@@ -14,26 +14,9 @@ class Binary {
       errors.push("name must be a string");
     }
 
-    if (version && typeof version !== "string") {
-      errors.push("version must be a string");
-    }
-
     if (!name) {
       errors.push("You must specify the name of your binary");
     }
-
-    if (!version) {
-      errors.push("You must specify the version of your binary");
-    }
-
-    if (
-      config &&
-      config.installDirectory &&
-      typeof config.installDirectory !== "string"
-    ) {
-      errors.push("config.installDirectory must be a string");
-    }
-
     if (errors.length > 0) {
       let errorMsg =
         "One or more of the parameters you passed to the Binary constructor are invalid:\n";
@@ -41,12 +24,11 @@ class Binary {
         errorMsg += error;
       });
       errorMsg +=
-        '\n\nCorrect usage: new Binary("my-binary", "https://example.com/binary/download.tar.gz", "v1.0.0")';
+        '\n\nCorrect usage: new Binary("my-binary", "https://example.com/binary/download.tar.gz")';
       error(errorMsg);
     }
     this.url = url;
     this.name = name;
-    this.version = version;
     this.installDirectory =
       config?.installDirectory || join(__dirname, "node_modules", ".bin");
 
@@ -54,10 +36,7 @@ class Binary {
       mkdirSync(this.installDirectory, { recursive: true });
     }
 
-    this.binaryPath = join(
-      this.installDirectory,
-      `${this.name}-${this.version}`
-    );
+    this.binaryPath = join(this.installDirectory, this.name);
   }
 
   exists() {
@@ -75,7 +54,7 @@ class Binary {
     }
 
     if (existsSync(this.installDirectory)) {
-      rimraf.sync(this.installDirectory);
+      rmSync(this.installDirectory, { recursive: true });
     }
 
     mkdirSync(this.installDirectory, { recursive: true });

[wopian]

This caused a big headache when setting up Rover on our org GraphQL schemas today. Any updates on this as it's been over a month and quite a blocker for pnpm projects 😳

[y-nk]

Same here. quite painful, i'd like to avoid installing rover from npm everytime but our monorepo leverages pnpm cache for caching...