From e34ffbae87b5f53fe49dcdac1bb790ff3df98c3c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 4 Jul 2024 08:35:33 +0530 Subject: [PATCH] Try all `path`, consider `useBundled` import strategy (#509) ## Summary This PR does two things: 1. As `ruff.path` is an array, we need to check each entry and only proceed with the first valid path ([`ruff-lsp` reference](https://github.com/astral-sh/ruff-lsp/blob/c51c77c269c3f13be2534d118b81e1c3b9bbfaff/ruff_lsp/server.py#L1773-L1784)) 2. Consider `useBundled` import strategy fixes: #507 ## Test Plan ### 1 One valid path: ```json { "ruff.nativeServer": true, "ruff.importStrategy": "useBundled", "ruff.path": ["/Users/dhruv/work/astral/ruff/target/debug/ruff"] } ``` Logs: ``` 2024-07-02 15:43:01.550 [info] Using 'path' setting: /Users/dhruv/work/astral/ruff/target/debug/ruff 2024-07-02 15:43:01.550 [info] Server run command: /Users/dhruv/work/astral/ruff/target/debug/ruff server --preview 2024-07-02 15:43:01.550 [info] Server: Start requested. ``` ### 2 Multiple valid paths: ```json { "ruff.nativeServer": true, "ruff.importStrategy": "useBundled", "ruff.path": [ "/Users/dhruv/.local/bin/ruff", "/Users/dhruv/work/astral/ruff/target/debug/ruff" ] } ``` Logs: ``` 2024-07-02 15:44:12.698 [info] Using 'path' setting: /Users/dhruv/.local/bin/ruff 2024-07-02 15:44:12.698 [info] Server run command: /Users/dhruv/.local/bin/ruff server --preview 2024-07-02 15:44:12.698 [info] Server: Start requested. ``` ### 3 First is an invalid path, second is a valid path: ```json { "ruff.nativeServer": true, "ruff.importStrategy": "useBundled", "ruff.path": [ "/Users/dhruv/random/ruff", "/Users/dhruv/work/astral/ruff/target/debug/ruff" ] } ``` Logs: ``` 2024-07-02 15:44:47.816 [info] Using 'path' setting: /Users/dhruv/work/astral/ruff/target/debug/ruff 2024-07-02 15:44:47.816 [info] Server run command: /Users/dhruv/work/astral/ruff/target/debug/ruff server --preview 2024-07-02 15:44:47.816 [info] Server: Start requested. ``` ### 4 All invalid paths, should fallback to using bundled executable: ```json { "ruff.nativeServer": true, "ruff.importStrategy": "useBundled", "ruff.path": [ "/Users/dhruv/random/ruff", "/Users/dhruv/another/random/ruff" ] } ``` Logs: ``` 2024-07-02 15:59:44.215 [info] Could not find executable in 'path': /Users/dhruv/random/ruff, /Users/dhruv/another/random/ruff 2024-07-02 15:59:44.215 [info] Using bundled executable: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff 2024-07-02 15:59:44.215 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff server --preview 2024-07-02 15:59:44.215 [info] Server: Start requested. ``` ### 5 No paths given, should fallback to using bundled executable: ```json { "ruff.nativeServer": true, "ruff.importStrategy": "useBundled", "ruff.path": [] } ``` Logs: ``` 2024-07-02 15:45:34.349 [info] Using bundled executable: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff 2024-07-02 15:45:34.349 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/bundled/libs/bin/ruff server --preview 2024-07-02 15:45:34.349 [info] Server: Start requested. ``` ### 6 No paths, not using bundled executable, let the Python script decide on which executable to use: ```json { "ruff.nativeServer": true, "ruff.path": [] } ``` Logs: ``` 2024-07-02 15:50:13.326 [info] Server run command: /Users/dhruv/work/astral/ruff-vscode/.venv/bin/python /Users/dhruv/work/astral/ruff-vscode/bundled/tool/ruff_server.py server --preview 2024-07-02 15:50:13.326 [info] Server: Start requested. ``` --- src/common/constants.ts | 54 +++++++++++++++++++++++++++-- src/common/server.ts | 77 ++++++++++++++++++++++++----------------- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/src/common/constants.ts b/src/common/constants.ts index 2f7a238..579ef92 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -1,19 +1,67 @@ import * as path from "path"; const folderName = path.basename(__dirname); + +/** + * Path to the root directory of this extension. + */ export const EXTENSION_ROOT_DIR = folderName === "common" ? path.dirname(path.dirname(__dirname)) : path.dirname(__dirname); + +/** + * Name of the `ruff` binary based on the current platform. + */ +export const RUFF_BINARY_NAME = process.platform == "win32" ? "ruff.exe" : "ruff"; + +/** + * Path to the directory containing the bundled Python scripts. + */ export const BUNDLED_PYTHON_SCRIPTS_DIR = path.join(EXTENSION_ROOT_DIR, "bundled"); -export const SERVER_SCRIPT_PATH = path.join(BUNDLED_PYTHON_SCRIPTS_DIR, "tool", `server.py`); + +/** + * Path to the `ruff` executable that is bundled with the extension. + */ +export const BUNDLED_RUFF_EXECUTABLE = path.join( + BUNDLED_PYTHON_SCRIPTS_DIR, + "libs", + "bin", + RUFF_BINARY_NAME, +); + +/** + * Path to the Python script that starts the `ruff-lsp` language server. + */ +export const RUFF_LSP_SERVER_SCRIPT_PATH = path.join( + BUNDLED_PYTHON_SCRIPTS_DIR, + "tool", + `server.py`, +); + export const DEBUG_SERVER_SCRIPT_PATH = path.join( BUNDLED_PYTHON_SCRIPTS_DIR, "tool", `_debug_server.py`, ); -export const EXPERIMENTAL_SERVER_SCRIPT_PATH = path.join( + +/** + * Path to the Python script that starts the native language server with an + * appropriate `ruff` executable. + * + * This should only be used as a fallback if the user has not specified either + * the `path` setting or is not using the bundled import strategy. + */ +export const NATIVE_SERVER_SCRIPT_PATH = path.join( BUNDLED_PYTHON_SCRIPTS_DIR, "tool", "ruff_server.py", ); -export const RUFF_SERVER_CMD = "server"; + +/** + * The subcommand for the `ruff` binary that starts the language server. + */ +export const RUFF_SERVER_SUBCOMMAND = "server"; + +/** + * Required arguments for the `ruff server` command. + */ export const RUFF_SERVER_REQUIRED_ARGS = ["--preview"]; diff --git a/src/common/server.ts b/src/common/server.ts index 9c0efca..29f0663 100644 --- a/src/common/server.ts +++ b/src/common/server.ts @@ -1,19 +1,20 @@ import * as fsapi from "fs-extra"; -import { Disposable, env, l10n, LanguageStatusSeverity, LogOutputChannel } from "vscode"; +import { Disposable, l10n, LanguageStatusSeverity, LogOutputChannel } from "vscode"; import { State } from "vscode-languageclient"; import { + Executable, LanguageClient, LanguageClientOptions, RevealOutputChannelOn, ServerOptions, } from "vscode-languageclient/node"; import { - BUNDLED_PYTHON_SCRIPTS_DIR, + BUNDLED_RUFF_EXECUTABLE, DEBUG_SERVER_SCRIPT_PATH, RUFF_SERVER_REQUIRED_ARGS, - RUFF_SERVER_CMD, - SERVER_SCRIPT_PATH, - EXPERIMENTAL_SERVER_SCRIPT_PATH, + RUFF_SERVER_SUBCOMMAND, + RUFF_LSP_SERVER_SCRIPT_PATH, + NATIVE_SERVER_SCRIPT_PATH, } from "./constants"; import { traceError, traceInfo, traceVerbose } from "./log/logging"; import { getDebuggerPath } from "./python"; @@ -32,6 +33,40 @@ export type IInitOptions = { globalSettings: ISettings; }; +function findNativeServerExecutable(settings: ISettings): Executable { + // 'path' setting takes priority over everything. + if (settings.path.length > 0) { + for (const path of settings.path) { + if (fsapi.existsSync(path)) { + traceInfo(`Using 'path' setting: ${path}`); + return { + command: path, + args: [RUFF_SERVER_SUBCOMMAND, ...RUFF_SERVER_REQUIRED_ARGS], + options: { cwd: settings.cwd, env: process.env }, + }; + } + } + traceInfo(`Could not find executable in 'path': ${settings.path.join(", ")}`); + } + + if (settings.importStrategy === "useBundled") { + traceInfo(`Using bundled executable: ${BUNDLED_RUFF_EXECUTABLE}`); + return { + command: BUNDLED_RUFF_EXECUTABLE, + args: [RUFF_SERVER_SUBCOMMAND, ...RUFF_SERVER_REQUIRED_ARGS], + options: { cwd: settings.cwd, env: process.env }, + }; + } + + // Otherwise, we'll call a Python script that tries to locate a binary, + // falling back to the bundled version if no local executable is found. + return { + command: settings.interpreter[0], + args: [NATIVE_SERVER_SCRIPT_PATH, RUFF_SERVER_SUBCOMMAND, ...RUFF_SERVER_REQUIRED_ARGS], + options: { cwd: settings.cwd, env: process.env }, + }; +} + async function createNativeServer( settings: ISettings, serverId: string, @@ -39,33 +74,11 @@ async function createNativeServer( outputChannel: LogOutputChannel, initializationOptions: IInitOptions, ): Promise { - let serverOptions: ServerOptions; - // If the user provided a binary path, we'll try to call that path directly. - if (settings.path[0]) { - const command = settings.path[0]; - const cwd = settings.cwd; - const args = [RUFF_SERVER_CMD, ...RUFF_SERVER_REQUIRED_ARGS]; - serverOptions = { - command, - args, - options: { cwd, env: process.env }, - }; - - traceInfo(`Server run command: ${[command, ...args].join(" ")}`); + let serverOptions = findNativeServerExecutable(settings); + if (serverOptions.args) { + traceInfo(`Server run command: ${[serverOptions.command, ...serverOptions.args].join(" ")}`); } else { - // Otherwise, we'll call a Python script that tries to locate - // a binary, falling back to the bundled version if no local executable is found. - const command = settings.interpreter[0]; - const cwd = settings.cwd; - const args = [EXPERIMENTAL_SERVER_SCRIPT_PATH, RUFF_SERVER_CMD, ...RUFF_SERVER_REQUIRED_ARGS]; - - serverOptions = { - command, - args, - options: { cwd, env: process.env }, - }; - - traceInfo(`Server run command: ${[command, ...args].join(" ")}`); + traceInfo(`Server run command: ${serverOptions.command}`); } const clientOptions = { @@ -112,7 +125,7 @@ async function createServer( const args = newEnv.USE_DEBUGPY === "False" || !isDebugScript - ? settings.interpreter.slice(1).concat([SERVER_SCRIPT_PATH]) + ? settings.interpreter.slice(1).concat([RUFF_LSP_SERVER_SCRIPT_PATH]) : settings.interpreter.slice(1).concat([DEBUG_SERVER_SCRIPT_PATH]); traceInfo(`Server run command: ${[command, ...args].join(" ")}`);