From 65c61e854909f2e1f95e2f4dd6986de34fb97785 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 13 Sep 2024 18:28:21 +0200 Subject: [PATCH] bugfix: Make sure proxy settings are being forwarded to coursier --- .../src/__tests__/getJavaOptions.test.ts | 8 ++++--- .../src/__tests__/setupCoursier.test.ts | 3 ++- .../metals-languageclient/src/fetchMetals.ts | 14 ++++------- .../src/getJavaOptions.ts | 21 +++++++++++++++++ .../src/setupCoursier.ts | 23 ++++++++++++++++--- packages/metals-vscode/src/extension.ts | 3 ++- 6 files changed, 55 insertions(+), 17 deletions(-) diff --git a/packages/metals-languageclient/src/__tests__/getJavaOptions.test.ts b/packages/metals-languageclient/src/__tests__/getJavaOptions.test.ts index da987724d..1cf94f129 100644 --- a/packages/metals-languageclient/src/__tests__/getJavaOptions.test.ts +++ b/packages/metals-languageclient/src/__tests__/getJavaOptions.test.ts @@ -25,11 +25,13 @@ describe("getJavaOptions", () => { }); it("sanitizes options from .jvmopts", () => { - const malformedOptions = ["-XX:+UseNUMA ", "-XX:+UseZGC "]; - const expectedOptions = ["-XX:+UseNUMA", "-XX:+UseZGC"]; + const malformedOptions = [ + "-Dhttp.proxyHost=proxy.example.com ", + "-Dhttp.proxyPort=1234 ", + ]; const workspaceRoot = createWorskpace(malformedOptions); const options = getJavaOptions(workspaceRoot); - expect(options).toEqual(expectedOptions); + expect(options).toEqual(proxyOptions); }); it("reads from JAVA_OPTS", () => { diff --git a/packages/metals-languageclient/src/__tests__/setupCoursier.test.ts b/packages/metals-languageclient/src/__tests__/setupCoursier.test.ts index 53d7bd75f..485e5e431 100644 --- a/packages/metals-languageclient/src/__tests__/setupCoursier.test.ts +++ b/packages/metals-languageclient/src/__tests__/setupCoursier.test.ts @@ -51,7 +51,8 @@ describe("setupCoursier", () => { tmpDir, process.cwd(), new LogOutputChannel(), - false + false, + ["-Xmx1000M", "-XX-fake!"] ); expect(fs.existsSync(coursier)).toBeTruthy; expect(fs.existsSync(javaHome)).toBeTruthy; diff --git a/packages/metals-languageclient/src/fetchMetals.ts b/packages/metals-languageclient/src/fetchMetals.ts index bee1ee98d..368eb58b1 100644 --- a/packages/metals-languageclient/src/fetchMetals.ts +++ b/packages/metals-languageclient/src/fetchMetals.ts @@ -2,6 +2,7 @@ import * as semver from "semver"; import { ChildProcessPromise, spawn } from "promisify-child-process"; import { JavaConfig } from "./getJavaConfig"; import { OutputChannel } from "./interfaces/OutputChannel"; +import { convertToCoursierProperties } from "./getJavaOptions"; interface FetchMetalsOptions { serverVersion: string; @@ -70,15 +71,10 @@ export async function fetchMetals({ ].concat(coursierArgs); return { promise: spawn(javaPath, jarArgs, environment) }; } else { - // Convert Java properties to the "-J" argument form used by Coursier - var javaArgs: Array = []; - - // setting properties on windows native launcher doesn't work - if (process.platform != "win32") - javaArgs = javaOptions - .concat(["-Dfile.encoding=UTF-8"]) - .concat(fetchProperties) - .map((p) => `-J${p}`); + const javaArgs: Array = convertToCoursierProperties( + javaOptions.concat(["-Dfile.encoding=UTF-8"]).concat(fetchProperties), + false + ); return { promise: spawn(coursier, javaArgs.concat(coursierArgs), environment), diff --git a/packages/metals-languageclient/src/getJavaOptions.ts b/packages/metals-languageclient/src/getJavaOptions.ts index ab8c98ce0..fe7e21f54 100644 --- a/packages/metals-languageclient/src/getJavaOptions.ts +++ b/packages/metals-languageclient/src/getJavaOptions.ts @@ -19,6 +19,26 @@ function sanitizeOption(option: string): string { return option.trim(); } +export function convertToCoursierProperties( + serverProperties: string[], + isCoursierJar: boolean +): string[] { + // setting properties on windows native launcher doesn't work + if (!isCoursierJar && process.platform == "win32") { + return []; + } else { + return serverProperties + .filter((prop) => isValidOption(prop)) + .map((prop) => { + if (isCoursierJar) { + return prop; + } else { + return "-J" + prop; + } + }); + } +} + function isValidOption(option: string): boolean { return ( option.startsWith("-") && @@ -28,6 +48,7 @@ function isValidOption(option: string): boolean { !option.startsWith("-Xms") && !option.startsWith("-Xmx") && !option.startsWith("-Xss") && + !option.startsWith("-XX:") && // Do not alter stdout that we capture when using Coursier option !== "-XX:+PrintCommandLineFlags" ); diff --git a/packages/metals-languageclient/src/setupCoursier.ts b/packages/metals-languageclient/src/setupCoursier.ts index 7e1e5690e..138088e15 100644 --- a/packages/metals-languageclient/src/setupCoursier.ts +++ b/packages/metals-languageclient/src/setupCoursier.ts @@ -9,6 +9,7 @@ import { OutputChannel } from "./interfaces/OutputChannel"; import path from "path"; import fs from "fs"; import { findOnPath } from "./util"; +import { convertToCoursierProperties } from "./getJavaOptions"; const coursierVersion = "v2.1.8"; // https://github.com/coursier/launchers contains only launchers with the most recent version @@ -20,7 +21,8 @@ export async function setupCoursier( coursierFetchPath: string, extensionPath: string, output: OutputChannel, - forceCoursierJar: boolean + forceCoursierJar: boolean, + serverProperties: string[] ): Promise<{ coursier: string; javaHome: string }> { const handleOutput = (out: Buffer) => { const msg = "\t" + out.toString().trim().split("\n").join("\n\t"); @@ -54,15 +56,30 @@ export async function setupCoursier( }; const resolveJavaHomeWithCoursier = async (coursier: string) => { + const nonJvmServerProperties = convertToCoursierProperties( + serverProperties, + coursier.endsWith(".jar") + ); await run( coursier, - ["java", "--jvm", `temurin:${javaVersion}`, "-version"], + [ + "java", + ...nonJvmServerProperties, + "--jvm", + `temurin:${javaVersion}`, + "-version", + ], handleOutput ); const getJavaPath = spawn( coursier, - ["java-home", "--jvm", `temurin:${javaVersion}`], + [ + "java-home", + ...nonJvmServerProperties, + "--jvm", + `temurin:${javaVersion}`, + ], { encoding: "utf8", } diff --git a/packages/metals-vscode/src/extension.ts b/packages/metals-vscode/src/extension.ts index ed2444de3..c4dbffce4 100644 --- a/packages/metals-vscode/src/extension.ts +++ b/packages/metals-vscode/src/extension.ts @@ -230,7 +230,8 @@ async function fetchAndLaunchMetals( metalsDirPath, context.extensionPath, outputChannel, - forceCoursierJar + forceCoursierJar, + serverProperties ); const canRetryWithJar =