Merge pull request #1548 from tgodzik/fix-zgc

bugfix: Fix issues when zgc is not fully supported

packages/metals-languageclient/src/__tests__/getJavaHome.test.ts

@@ -61,7 +61,7 @@ describe("getJavaHome", () => {
       new MockOutput()
     );
     jest.restoreAllMocks();
-    expect(javaHome).toBe(JAVA_HOME);
+    expect(javaHome.path).toBe(JAVA_HOME);
   });
 
   // needs to run on a machine with an actual JAVA_HOME set up
@@ -90,7 +90,7 @@ describe("getJavaHome", () => {
       new MockOutput()
     );
     jest.restoreAllMocks();
-    expect(javaHome).toBe(pathJavaHome);
+    expect(javaHome.path).toBe(pathJavaHome);
   });
 });
 

packages/metals-languageclient/src/getJavaConfig.ts

@@ -1,9 +1,11 @@
+import { JavaHome } from "./getJavaHome";
 import { getJavaOptions } from "./getJavaOptions";
 import * as path from "path";
 
 export interface JavaConfig {
   javaOptions: string[];
   javaPath: string;
+  javaHome: JavaHome;
   coursier: string;
   coursierMirrorFilePath: string | undefined;
   extraEnv: {
@@ -13,7 +15,7 @@ export interface JavaConfig {
 
 interface GetJavaConfigOptions {
   workspaceRoot: string | undefined;
-  javaHome: string;
+  javaHome: JavaHome;
   coursier: string;
   coursierMirrorFilePath: string | undefined;
   customRepositories: string[] | undefined;
@@ -27,7 +29,7 @@ export function getJavaConfig({
   customRepositories = [],
 }: GetJavaConfigOptions): JavaConfig {
   const javaOptions = getJavaOptions(workspaceRoot);
-  const javaPath = path.join(javaHome, "bin", "java");
+  const javaPath = path.join(javaHome.path, "bin", "java");
 
   const coursierRepositories =
     customRepositories.length > 0
@@ -45,6 +47,7 @@ export function getJavaConfig({
   return {
     javaOptions,
     javaPath,
+    javaHome,
     coursier,
     coursierMirrorFilePath,
     extraEnv,

packages/metals-languageclient/src/getJavaHome.ts

@@ -6,6 +6,12 @@ import { realpathSync } from "fs";
 
 export type JavaVersion = "11" | "17" | "21";
 
+export interface JavaHome {
+  path: string;
+  description: string;
+  version: string;
+}
+
 /**
  * Computes the user's Java Home path, using various strategies:
  *
@@ -19,17 +25,17 @@ export type JavaVersion = "11" | "17" | "21";
 export async function getJavaHome(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   const fromEnvValue = await fromEnv(javaVersion, outputChannel);
   return fromEnvValue || (await fromPath(javaVersion, outputChannel));
 }
 
 const versionRegex = /\"\d\d/;
-async function validateJavaVersion(
+export async function validateJavaVersion(
   javaHome: string,
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<boolean> {
+): Promise<JavaHome | undefined> {
   const javaBin = path.join(javaHome, "bin", "java");
   try {
     const javaVersionOut = spawn(javaBin, ["-version"], {
@@ -43,14 +49,18 @@ async function validateJavaVersion(
 
     const javaInfoStr = (await javaVersionOut).stderr as string;
     const matches = javaInfoStr.match(versionRegex);
-    if (matches) {
-      return +matches[0].slice(1, 3) >= +javaVersion;
+    if (matches && +matches[0].slice(1, 3) >= +javaVersion) {
+      return {
+        path: javaHome,
+        description: javaInfoStr,
+        version: matches[0].slice(1, 3),
+      };
     }
   } catch (error) {
     outputChannel.appendLine(`failed while running ${javaBin} -version`);
     outputChannel.appendLine(`${error}`);
   }
-  return false;
+  return undefined;
 }
 
 function propertyValueOf(
@@ -70,7 +80,7 @@ function propertyValueOf(
 export async function fromPath(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   let javaExecutable = findOnPath(["java"]);
   if (javaExecutable) {
     const realJavaPath = realpathSync(javaExecutable);
@@ -86,13 +96,22 @@ export async function fromPath(
         cmdOutput,
         "java.specification.version"
       );
-      const isValid =
-        discoveredJavaHome &&
-        discoveredJavaVersion &&
-        parseInt(discoveredJavaVersion) >= parseInt(javaVersion);
+      function getLastThreeLines(text: string): string {
+        let lines = text.split(/\r?\n/);
+        return lines.slice(-3).join("\n");
+      }
 
-      if (isValid) return discoveredJavaHome;
-      else {
+      if (
+        discoveredJavaVersion &&
+        discoveredJavaHome &&
+        parseInt(discoveredJavaVersion) >= parseInt(javaVersion)
+      ) {
+        return {
+          path: discoveredJavaHome,
+          description: getLastThreeLines(cmdOutput),
+          version: discoveredJavaVersion,
+        };
+      } else {
         outputChannel.appendLine(
           `Java version doesn't match the required one of ${javaVersion}`
         );
@@ -109,18 +128,18 @@ export async function fromPath(
 export async function fromEnv(
   javaVersion: JavaVersion,
   outputChannel: OutputChannel
-): Promise<string | undefined> {
+): Promise<JavaHome | undefined> {
   const javaHome = process.env["JAVA_HOME"];
   if (javaHome) {
     outputChannel.appendLine(
       `Checking Java in JAVA_HOME, which points to ${javaHome}`
     );
-    const isValid = await validateJavaVersion(
+    const validatedJavaHome = await validateJavaVersion(
       javaHome,
       javaVersion,
       outputChannel
     );
-    if (isValid) return javaHome;
+    if (validatedJavaHome) return validatedJavaHome;
     else {
       outputChannel.appendLine(
         `Java version doesn't match the required one of ${javaVersion}`

packages/metals-languageclient/src/getServerOptions.ts

@@ -1,39 +1,53 @@
 import { ServerOptions } from "./interfaces/ServerOptions";
 import { JavaConfig } from "./getJavaConfig";
 
-interface GetServerOptions {
-  metalsClasspath: string;
-  serverProperties: string[] | undefined;
-  clientName?: string;
-  javaConfig: JavaConfig;
-}
-
-export function getServerOptions({
-  metalsClasspath,
-  serverProperties = [],
-  clientName,
-  javaConfig: { javaOptions, javaPath, extraEnv },
-}: GetServerOptions): ServerOptions {
+export function getServerOptions(
+  metalsClasspath: string,
+  serverProperties: string[],
+  clientName: string,
+  javaConfig: JavaConfig
+): ServerOptions {
   const baseProperties = ["-Xss4m", "-Xms100m"];
 
   if (clientName) {
     baseProperties.push(`-Dmetals.client=${clientName}`);
   }
 
+  /**
+   * GraalVM for JDK 17-20 prints additional warnings that breaks things
+   */
+  const skipZGC =
+    +javaConfig.javaHome.version < 21 &&
+    javaConfig.javaHome.description.indexOf("GraalVM") > -1;
+
+  var filteredServerProperties = serverProperties;
+  if (skipZGC) {
+    filteredServerProperties = serverProperties.filter(function (prop) {
+      return prop.indexOf("UseZGC") === -1;
+    });
+  }
   const mainArgs = ["-classpath", metalsClasspath, "scala.meta.metals.Main"];
 
   // let user properties override base properties
   const launchArgs = [
     ...baseProperties,
-    ...javaOptions,
-    ...serverProperties,
+    ...javaConfig.javaOptions,
+    ...filteredServerProperties,
     ...mainArgs,
   ];
 
-  const env = () => ({ ...process.env, ...extraEnv });
+  const env = () => ({ ...process.env, ...javaConfig.extraEnv });
 
   return {
-    run: { command: javaPath, args: launchArgs, options: { env: env() } },
-    debug: { command: javaPath, args: launchArgs, options: { env: env() } },
+    run: {
+      command: javaConfig.javaPath,
+      args: launchArgs,
+      options: { env: env() },
+    },
+    debug: {
+      command: javaConfig.javaPath,
+      args: launchArgs,
+      options: { env: env() },
+    },
   };
 }

packages/metals-languageclient/src/setupCoursier.ts

@@ -4,7 +4,12 @@ import {
   PromisifySpawnOptions,
   spawn,
 } from "promisify-child-process";
-import { JavaVersion, getJavaHome } from "./getJavaHome";
+import {
+  JavaHome,
+  JavaVersion,
+  getJavaHome,
+  validateJavaVersion,
+} from "./getJavaHome";
 import { OutputChannel } from "./interfaces/OutputChannel";
 import path from "path";
 import fs from "fs";
@@ -23,7 +28,7 @@ export async function setupCoursier(
   output: OutputChannel,
   forceCoursierJar: boolean,
   serverProperties: string[]
-): Promise<{ coursier: string; javaHome: string }> {
+): Promise<{ coursier: string; javaHome: JavaHome }> {
   const handleOutput = (out: Buffer) => {
     const msg = "\t" + out.toString().trim().split("\n").join("\n\t");
     output.appendLine(msg);
@@ -107,10 +112,18 @@ export async function setupCoursier(
     output.appendLine(
       `No installed java with version ${javaVersion} found. Will fetch one using coursier:`
     );
-    javaHome = await resolveJavaHomeWithCoursier(coursier);
+    const coursierJavaHome = await resolveJavaHomeWithCoursier(coursier);
+    const validatedJavaHome = await validateJavaVersion(
+      coursierJavaHome,
+      javaVersion,
+      output
+    );
+    if (validatedJavaHome) {
+      javaHome = validatedJavaHome;
+    }
   }
 
-  output.appendLine(`Using Java Home: ${javaHome}`);
+  output.appendLine(`Using Java Home: ${javaHome?.path}`);
 
   /* If we couldn't download coursier, but we have Java
    * we can still fall back to jar based launcher.

packages/metals-vscode/src/extension.ts

@@ -344,12 +344,12 @@ function launchMetals(
   // Make editing Scala docstrings slightly nicer.
   enableScaladocIndentation();
 
-  const serverOptions = getServerOptions({
+  const serverOptions = getServerOptions(
     metalsClasspath,
     serverProperties,
-    javaConfig,
-    clientName: "vscode",
-  });
+    "vscode",
+    javaConfig
+  );
 
   const initializationOptions: MetalsInitializationOptions = {
     compilerOptions: {