Fix chruby release candidate ordering (#2212)

* Fix chruby release candidate ordering Co-authored-by: Emily Samp <egiurleo@users.noreply.github.com> * Fix handling of release candidates --------- Co-authored-by: Emily Samp <egiurleo@users.noreply.github.com> Co-authored-by: Andy Waite <13400+andyw8@users.noreply.github.com>
Shopify · Jun 20, 2024 · 0b4a312 · 0b4a312
1 parent e1df207
commit 0b4a312
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 54 deletions.
diff --git a/vscode/src/ruby/chruby.ts b/vscode/src/ruby/chruby.ts
@@ -47,7 +47,7 @@ export class Chruby extends VersionManager {
     );
 
     const { defaultGems, gemHome, yjit, version } =
-      await this.runActivationScript(rubyUri);
+      await this.runActivationScript(rubyUri, versionInfo);
 
     this.outputChannel.info(
       `Activated Ruby environment: defaultGems=${defaultGems} gemHome=${gemHome} yjit=${yjit}`,
@@ -75,16 +75,6 @@ export class Chruby extends VersionManager {
 
   // Returns the full URI to the Ruby executable
   protected async findRubyUri(rubyVersion: RubyVersion): Promise<vscode.Uri> {
-    if (/\d+\.\d+\.\d+/.exec(rubyVersion.version)) {
-      return this.findRubyUriForCompleteVersion(rubyVersion);
-    }
-
-    return this.findRubyUriWithOmittedPatch(rubyVersion);
-  }
-
-  private async findRubyUriWithOmittedPatch(
-    rubyVersion: RubyVersion,
-  ): Promise<vscode.Uri> {
     const possibleVersionNames = rubyVersion.engine
       ? [`${rubyVersion.engine}-${rubyVersion.version}`, rubyVersion.version]
       : [rubyVersion.version, `ruby-${rubyVersion.version}`];
@@ -121,39 +111,6 @@ export class Chruby extends VersionManager {
     );
   }
 
-  private async findRubyUriForCompleteVersion(
-    rubyVersion: RubyVersion,
-  ): Promise<vscode.Uri> {
-    // If an engine was specified in the .ruby-version file, we favor looking for that first and also try just the
-    // version number. If no engine was specified, we first try just the version number and then we try using `ruby` as
-    // the default engine
-    const possibleVersionNames = rubyVersion.engine
-      ? [`${rubyVersion.engine}-${rubyVersion.version}`, rubyVersion.version]
-      : [rubyVersion.version, `ruby-${rubyVersion.version}`];
-
-    for (const uri of this.rubyInstallationUris) {
-      let installationUri: vscode.Uri;
-
-      for (const versionName of possibleVersionNames) {
-        try {
-          installationUri = vscode.Uri.joinPath(uri, versionName);
-          await vscode.workspace.fs.stat(installationUri);
-          return vscode.Uri.joinPath(installationUri, "bin", "ruby");
-        } catch (_error: any) {
-          // Continue to the next version name
-          this.outputChannel.debug(
-            `Tried searching for Ruby installation in ${uri.fsPath} but it doesn't exist`,
-          );
-        }
-      }
-    }
-
-    throw new Error(
-      `Cannot find installation directory for Ruby version ${possibleVersionNames.join(" or ")}.
-       Searched in ${this.rubyInstallationUris.map((uri) => uri.fsPath).join(", ")}`,
-    );
-  }
-
   // Returns the Ruby version information including version and engine. E.g.: ruby-3.3.0, truffleruby-21.3.0
   private async discoverRubyVersion(): Promise<RubyVersion> {
     let uri = this.bundleUri;
@@ -197,7 +154,10 @@ export class Chruby extends VersionManager {
   }
 
   // Run the activation script using the Ruby installation we found so that we can discover gem paths
-  private async runActivationScript(rubyExecutableUri: vscode.Uri): Promise<{
+  private async runActivationScript(
+    rubyExecutableUri: vscode.Uri,
+    rubyVersion: RubyVersion,
+  ): Promise<{
     defaultGems: string;
     gemHome: string;
     yjit: boolean;
@@ -217,7 +177,7 @@ export class Chruby extends VersionManager {
       "    user_dir = paths[0] if Dir.exist?(paths[0])",
       "  end",
       "end",
-      "newer_gem_home = File.join(File.dirname(user_dir), RUBY_VERSION)",
+      `newer_gem_home = File.join(File.dirname(user_dir), "${rubyVersion.version}")`,
       "gems = (Dir.exist?(newer_gem_home) ? newer_gem_home : user_dir)",
       "data = { defaultGems: Gem.default_dir, gemHome: gems, yjit: !!defined?(RubyVM::YJIT), version: RUBY_VERSION }",
       "STDERR.print(JSON.dump(data))",

diff --git a/vscode/src/test/suite/ruby/chruby.test.ts b/vscode/src/test/suite/ruby/chruby.test.ts
@@ -103,9 +103,11 @@ suite("Chruby", () => {
     assert.notStrictEqual(yjit, undefined);
   });
 
-  test("Prefers standard rubies over custom built ones", async () => {
+  test("Considers any version with a suffix to be the latest", async () => {
+    // chruby always considers anything with a suffix to be the latest version, even if that's not accurate. For
+    // example, 3.3.0-rc1 is older than the stable 3.3.0, but running `chruby 3.3.0` will prefer the release candidate
     fs.mkdirSync(
-      path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-custom`, "bin"),
+      path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-rc1`, "bin"),
       {
         recursive: true,
       },
@@ -116,7 +118,7 @@ suite("Chruby", () => {
         rootPath,
         "opt",
         "rubies",
-        `${RUBY_VERSION}-custom`,
+        `${RUBY_VERSION}-rc1`,
         "bin",
         "ruby",
       ),
@@ -129,13 +131,59 @@ suite("Chruby", () => {
       vscode.Uri.file(path.join(rootPath, "opt", "rubies")),
     ];
 
-    const { env, version, yjit } = await chruby.activate();
+    const { env, yjit } = await chruby.activate();
 
-    assert.match(env.GEM_PATH!, new RegExp(`ruby/${VERSION_REGEX}`));
-    assert.match(env.GEM_PATH!, new RegExp(`lib/ruby/gems/${VERSION_REGEX}`));
-    assert.strictEqual(version, RUBY_VERSION);
+    // Since we symlink the stable Ruby as if it were a release candidate, we cannot assert the version of gem paths
+    // because those will match the stable version that is running the activation script. It is enough to verify that we
+    // inserted the correct Ruby path into the PATH
+    assert.match(
+      env.PATH!,
+      new RegExp(`\\/opt\\/rubies\\/${VERSION_REGEX}-rc1`),
+    );
+    assert.notStrictEqual(yjit, undefined);
+    fs.rmSync(path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-rc1`), {
+      recursive: true,
+      force: true,
+    });
+  });
+
+  test("Finds right Ruby with explicit release candidate but omitted engine", async () => {
+    fs.mkdirSync(
+      path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-rc1`, "bin"),
+      {
+        recursive: true,
+      },
+    );
+
+    createRubySymlinks(
+      path.join(
+        rootPath,
+        "opt",
+        "rubies",
+        `${RUBY_VERSION}-rc1`,
+        "bin",
+        "ruby",
+      ),
+    );
+
+    fs.writeFileSync(
+      path.join(rootPath, ".ruby-version"),
+      `${RUBY_VERSION}-rc1`,
+    );
+
+    const chruby = new Chruby(workspaceFolder, outputChannel);
+    chruby.rubyInstallationUris = [
+      vscode.Uri.file(path.join(rootPath, "opt", "rubies")),
+    ];
+
+    const { env, yjit } = await chruby.activate();
+
+    assert.match(
+      env.PATH!,
+      new RegExp(`\\/opt\\/rubies\\/${VERSION_REGEX}-rc1`),
+    );
     assert.notStrictEqual(yjit, undefined);
-    fs.rmSync(path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-custom`), {
+    fs.rmSync(path.join(rootPath, "opt", "rubies", `${RUBY_VERSION}-rc1`), {
       recursive: true,
       force: true,
     });