Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improvement: Allow to override metals java home #1559

Merged
merged 3 commits into from
Jan 8, 2025
Merged

improvement: Allow to override metals java home #1559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50732c5
improvement: Allow to override mtals java home
tgodzik Dec 12, 2024
483bcf2
Apply suggestions from code review
tgodzik Jan 8, 2025
c745a6b
improvement: Return undefined if empty string
tgodzik Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/metals-languageclient/src/__tests__/setupCoursier.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe("setupCoursier", () => {
it("should setup coursier correctly", async () => {
const { coursier, javaHome } = await setupCoursier(
"17",
undefined,
tmpDir,
process.cwd(),
new LogOutputChannel(),
Expand Down
1 change: 1 addition & 0 deletions packages/metals-languageclient/src/detectLaunchConfigurationChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export function detectLaunchConfigurationChanges(
UserConfiguration.ServerVersion,
UserConfiguration.ServerProperties,
UserConfiguration.JavaVersion,
UserConfiguration.MetalsJavaHome,
UserConfiguration.CustomRepositories,
UserConfiguration.CoursierMirror,
...additionalRestartKeys,
Expand Down
4 changes: 4 additions & 0 deletions packages/metals-languageclient/src/interfaces/UserConfiguration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ export enum UserConfiguration {
* Java version. Can be one of 8, 11, 17, 21.
*/
JavaVersion = "javaVersion",
/**
* Java Home to be used by Metals instead of it inferring using javaVersion.
*/
MetalsJavaHome = "metalsJavaHome",
/**
* Additional repositories that can be used to download dependencies.
* https://get-coursier.io/docs/other-repositories
Expand Down
11 changes: 9 additions & 2 deletions packages/metals-languageclient/src/setupCoursier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const coursierCommit = "11b428f35ca84a598ca30cce1c35ae4f375e5ee3";

export async function setupCoursier(
javaVersion: JavaVersion,
javaHomeOverride: string | undefined,
coursierFetchPath: string,
extensionPath: string,
output: OutputChannel,
Expand Down Expand Up @@ -106,7 +107,13 @@ export async function setupCoursier(
}
output.appendLine(`Using coursier located at ${coursier}`);

var javaHome = await getJavaHome(javaVersion, output);
var javaHome: JavaHome | undefined;

if (javaHomeOverride) {
javaHome = await validateJavaVersion(javaHomeOverride, javaVersion, output);
} else {
javaHome = await getJavaHome(javaVersion, output);
}

if (!javaHome && coursier) {
output.appendLine(
Expand Down Expand Up @@ -136,7 +143,7 @@ export async function setupCoursier(
else
throw Error(
`Cannot resolve Java home or coursier, JAVA_HOME should exist with a version of at least ${javaVersion}.` +
`Alternatively, you can reduce the requirement using "metals.javaVersion" setting.`
`Alternatively, you can reduce the requirement using "metals.javaVersion" setting and override the path using "metals.metalsJavaHome" setting.`
);
}

Expand Down
5 changes: 5 additions & 0 deletions packages/metals-vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@
"scope": "machine-overridable",
"markdownDescription": "Optional path to the Java home directory that will be used for compiling the project.\n\nDefaults to JDK used by Metals's server (look: Java Version).\n\nThis Java version should be lower or equal to JDK version used by the Metals's server."
},
"metals.metalsJavaHome": {
"type": "string",
"scope": "machine",
"markdownDescription": "Optional path to the Java home directory that will be used for the running Metals server.\n\nBy default Metals will try to infer it using the version specified in metals.javaVersion.\n\nThis Java version should be higher or equal to 17."
},
"metals.javaVersion": {
"type": "string",
"default": "17",
Expand Down
2 changes: 2 additions & 0 deletions packages/metals-vscode/src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
import {
currentWorkspaceFolder,
getJavaVersionFromConfig,
getJavaVersionOverride,
getTextDocumentPositionParams,
getValueFromConfig,
metalsDir,
Expand Down Expand Up @@ -230,6 +231,7 @@

const { coursier, javaHome } = await metalsLanguageClient.setupCoursier(
javaVersion,
getJavaVersionOverride(),
metalsDirPath,
context.extensionPath,
outputChannel,
Expand All @@ -244,7 +246,7 @@
if (canRetryWithJar) {
outputChannel.appendLine(
"Trying again with the embedded coursier. This might take longer."
);

Check warning on line 249 in packages/metals-vscode/src/extension.ts

View workflow job for this annotation

GitHub Actions / Eslint 

Unexpected any. Specify a different type

Check warning on line 249 in packages/metals-vscode/src/extension.ts

View workflow job for this annotation

GitHub Actions / Eslint 

Unexpected any. Specify a different type
return fetchAndLaunchMetals(context, serverVersion, javaVersion, true);
} else {
return Promise.reject(error);
Expand Down Expand Up @@ -284,7 +286,7 @@
outputChannel.appendLine(
"Launching Metals failed with the following:"
);
outputChannel.appendLine(reason.message);

Check warning on line 289 in packages/metals-vscode/src/extension.ts

View workflow job for this annotation

GitHub Actions / Eslint 

Unexpected any. Specify a different type
outputChannel.appendLine(
debugInformation(serverVersion, serverProperties, javaConfig)
);
Expand Down
12 changes: 12 additions & 0 deletions packages/metals-vscode/src/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,18 @@ export function getJavaVersionFromConfig() {
return undefined;
}

export function getJavaVersionOverride(): string | undefined {
const javaVersion = workspace
.getConfiguration("metals")
.get<string>(UserConfiguration.MetalsJavaHome)
?.trim();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe if empty string you want to return undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

if (javaVersion && javaVersion.length > 0) {
return javaVersion;
} else {
return undefined;
}
}

export async function fetchFrom(
url: string,
options?: http.RequestOptions
Expand Down
Loading