From 560b84f812aaff4ff4dcda9e93d574482aee3944 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Mon, 28 Oct 2024 14:31:24 +0100 Subject: [PATCH] bugfix: Fix issues when zgc is not fully supported Turns out setting ZGC on graalvm for JDK 17 will break communication between metals server and vs code as it prints an unexpected warning to STDOUT --- .../src/getJavaConfig.ts | 7 ++- .../metals-languageclient/src/getJavaHome.ts | 51 +++++++++++++------ .../src/getServerOptions.ts | 50 +++++++++++------- .../src/setupCoursier.ts | 21 ++++++-- packages/metals-vscode/src/extension.ts | 8 +-- 5 files changed, 93 insertions(+), 44 deletions(-) diff --git a/packages/metals-languageclient/src/getJavaConfig.ts b/packages/metals-languageclient/src/getJavaConfig.ts index 4c4c46ff1..83c6b9686 100644 --- a/packages/metals-languageclient/src/getJavaConfig.ts +++ b/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, diff --git a/packages/metals-languageclient/src/getJavaHome.ts b/packages/metals-languageclient/src/getJavaHome.ts index 5cf275f9e..0c16871c7 100644 --- a/packages/metals-languageclient/src/getJavaHome.ts +++ b/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 { +): Promise { 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 { +): Promise { 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 { +): Promise { 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 { +): Promise { 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}` diff --git a/packages/metals-languageclient/src/getServerOptions.ts b/packages/metals-languageclient/src/getServerOptions.ts index 941593834..89474f618 100644 --- a/packages/metals-languageclient/src/getServerOptions.ts +++ b/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() }, + }, }; } diff --git a/packages/metals-languageclient/src/setupCoursier.ts b/packages/metals-languageclient/src/setupCoursier.ts index 138088e15..54cc3195d 100644 --- a/packages/metals-languageclient/src/setupCoursier.ts +++ b/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. diff --git a/packages/metals-vscode/src/extension.ts b/packages/metals-vscode/src/extension.ts index 0b954e2ab..c17d2547a 100644 --- a/packages/metals-vscode/src/extension.ts +++ b/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: {