Skip to content

Commit

Permalink
Merge pull request #1539 from tgodzik/add-proxy
Browse files Browse the repository at this point in the history
bugfix: Make sure proxy settings are being forwarded to coursier
  • Loading branch information
tgodzik authored Oct 22, 2024
2 parents 05635ca + 65c61e8 commit 691be8e
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 5 additions & 9 deletions packages/metals-languageclient/src/fetchMetals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<string> = [];

// 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<string> = convertToCoursierProperties(
javaOptions.concat(["-Dfile.encoding=UTF-8"]).concat(fetchProperties),
false
);

return {
promise: spawn(coursier, javaArgs.concat(coursierArgs), environment),
Expand Down
21 changes: 21 additions & 0 deletions packages/metals-languageclient/src/getJavaOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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("-") &&
Expand All @@ -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"
);
Expand Down
23 changes: 20 additions & 3 deletions packages/metals-languageclient/src/setupCoursier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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");
Expand Down Expand Up @@ -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",
}
Expand Down
3 changes: 2 additions & 1 deletion packages/metals-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ async function fetchAndLaunchMetals(
metalsDirPath,
context.extensionPath,
outputChannel,
forceCoursierJar
forceCoursierJar,
serverProperties
);

const canRetryWithJar =
Expand Down

0 comments on commit 691be8e

Please sign in to comment.