From 9d28ed7be8b9fdc1376067eb1a65759b34cbb7d9 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 11 Apr 2023 18:21:08 +0200 Subject: [PATCH] feature: Run local coursier if available to avoid using bootstraped one This might help out in corporate environements since it will require installing coursier only once and Metals will be able to use it. In later steps I will try to automatically download coursier or native image via metals if not available. --- .github/workflows/ci.yml | 1 + .vscode/launch.json | 7 +- .../src/__tests__/fetchMetals.test.ts | 14 ++ .../metals-languageclient/src/fetchMetals.ts | 134 +++++++++++++----- packages/metals-vscode/src/extension.ts | 13 +- 5 files changed, 123 insertions(+), 46 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cadc55db7..2b777f514 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,7 @@ jobs: os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v2 + - uses: coursier/setup-action@v1 - uses: actions/setup-node@v2 with: node-version: "16" diff --git a/.vscode/launch.json b/.vscode/launch.json index a7caf6f6c..2ce88134f 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -7,10 +7,11 @@ "type": "extensionHost", "request": "launch", "runtimeExecutable": "${execPath}", - "args": ["--extensionDevelopmentPath=${workspaceRoot}"], - "stopOnEntry": false, + "args": [ + "--extensionDevelopmentPath=${workspaceRoot}/packages/metals-vscode" + ], "sourceMaps": true, - "outFiles": ["${workspaceRoot}/out/**/*.js"], + "outFiles": ["${workspaceRoot}/packages/metals-vscode/out/**/*.js"], "preLaunchTask": "npm: watch" } ] diff --git a/packages/metals-languageclient/src/__tests__/fetchMetals.test.ts b/packages/metals-languageclient/src/__tests__/fetchMetals.test.ts index a3f79236f..6f0b6df3a 100644 --- a/packages/metals-languageclient/src/__tests__/fetchMetals.test.ts +++ b/packages/metals-languageclient/src/__tests__/fetchMetals.test.ts @@ -1,4 +1,5 @@ import { calcServerDependency } from "../fetchMetals"; +import { validateCoursier } from "../fetchMetals"; describe("fetchMetals", () => { describe("calcServerDependency", () => { @@ -34,4 +35,17 @@ describe("fetchMetals", () => { expect(calcServerDependency(customVersion)).toBe(customVersion); }); }); + + it("should find coursier in PATH", () => { + const pathEnv = process.env["PATH"]; + if (pathEnv) { + expect(validateCoursier(pathEnv)).toBeDefined(); + } else { + fail("PATH environment variable is not defined"); + } + }); + + it("should not find coursier if not present in PATH", () => { + expect(validateCoursier("path/fake")).toBeUndefined(); + }); }); diff --git a/packages/metals-languageclient/src/fetchMetals.ts b/packages/metals-languageclient/src/fetchMetals.ts index 1092a42c2..959a71f3d 100644 --- a/packages/metals-languageclient/src/fetchMetals.ts +++ b/packages/metals-languageclient/src/fetchMetals.ts @@ -1,6 +1,10 @@ import * as semver from "semver"; +import path from "path"; +import fs from "fs"; import { ChildProcessPromise, spawn } from "promisify-child-process"; import { JavaConfig } from "./getJavaConfig"; +import { OutputChannel } from "./interfaces/OutputChannel"; +import { spawnSync } from "child_process"; interface FetchMetalsOptions { serverVersion: string; @@ -8,49 +12,103 @@ interface FetchMetalsOptions { javaConfig: JavaConfig; } -export function fetchMetals({ - serverVersion, - serverProperties, - javaConfig: { javaPath, javaOptions, extraEnv, coursierPath }, -}: FetchMetalsOptions): ChildProcessPromise { +export function fetchMetals( + { + serverVersion, + serverProperties, + javaConfig: { javaPath, javaOptions, extraEnv, coursierPath }, + }: FetchMetalsOptions, + output: OutputChannel +): ChildProcessPromise { const fetchProperties = serverProperties.filter( (p) => !p.startsWith("-agentlib") ); - const serverDependency = calcServerDependency(serverVersion); - return spawn( - javaPath, - [ - ...javaOptions, - ...fetchProperties, - "-Dfile.encoding=UTF-8", - "-jar", - coursierPath, - "fetch", - "-p", - "--ttl", - // Use infinite ttl to avoid redunant "Checking..." logs when using SNAPSHOT - // versions. Metals SNAPSHOT releases are effectively immutable since we - // never publish the same version twice. - "Inf", - serverDependency, - "-r", - "bintray:scalacenter/releases", - "-r", - "sonatype:public", - "-r", - "sonatype:snapshots", - "-p", - ], - { - env: { - COURSIER_NO_TERM: "true", - ...extraEnv, - ...process.env, - }, - stdio: ["ignore"], // Due to Issue: #219 + + const coursierArgs = [ + "fetch", + "-p", + "--ttl", + // Use infinite ttl to avoid redunant "Checking..." logs when using SNAPSHOT + // versions. Metals SNAPSHOT releases are effectively immutable since we + // never publish the same version twice. + "Inf", + serverDependency, + "-r", + "bintray:scalacenter/releases", + "-r", + "sonatype:public", + "-r", + "sonatype:snapshots", + "-p", + ]; + + const path = process.env["PATH"]; + let possibleCoursier: string | undefined; + if (path) { + possibleCoursier = validateCoursier(path); + } + + function spawnDefault(): ChildProcessPromise { + return spawn( + javaPath, + [ + ...javaOptions, + ...fetchProperties, + "-Dfile.encoding=UTF-8", + "-jar", + coursierPath, + ].concat(coursierArgs), + { + env: { + COURSIER_NO_TERM: "true", + ...extraEnv, + ...process.env, + }, + } + ); + } + + if (possibleCoursier) { + const coursier: string = possibleCoursier; + const coursierVersion = spawnSync(coursier, ["version"]); + if (coursierVersion.status !== 0) { + return spawnDefault(); + } else { + output.appendLine(`Using coursier located at ${coursier}`); + return spawn(coursier, coursierArgs); } - ); + } else { + return spawnDefault(); + } +} + +export function validateCoursier(pathEnv: string): string | undefined { + const possibleCoursier = pathEnv + .split(path.delimiter) + .flatMap((p) => { + try { + if (fs.statSync(p).isDirectory()) { + return fs.readdirSync(p).map((sub) => path.resolve(p, sub)); + } else return [p]; + } catch (e) { + return []; + } + }) + .find( + (p) => + p.endsWith(path.sep + "cs") || + p.endsWith(path.sep + "coursier") || + (process.platform === "win32" && p.endsWith(path.sep + "cs.bat")) + ); + if (possibleCoursier) { + const coursierVersion = spawnSync(possibleCoursier, ["version"]); + if (coursierVersion.status !== 0) { + return undefined; + } else { + return possibleCoursier; + } + } } export function calcServerDependency(serverVersion: string): string { diff --git a/packages/metals-vscode/src/extension.ts b/packages/metals-vscode/src/extension.ts index 788f781c5..c0134c657 100644 --- a/packages/metals-vscode/src/extension.ts +++ b/packages/metals-vscode/src/extension.ts @@ -183,11 +183,14 @@ function fetchAndLaunchMetals( extensionPath: context.extensionPath, }); - const fetchProcess = fetchMetals({ - serverVersion, - serverProperties, - javaConfig, - }); + const fetchProcess = fetchMetals( + { + serverVersion, + serverProperties, + javaConfig, + }, + outputChannel + ); const title = `Downloading Metals v${serverVersion}`; return trackDownloadProgress(title, outputChannel, fetchProcess).then(