https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/118547
>From 984beaa4b26f97a925d8c3f3bc97a9e9b61511ec Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 3 Dec 2024 12:59:13 -0800 Subject: [PATCH 1/2] [lldb-dap] Support finding the lldb-dap binary Support finding the lldb-dap binary with `xcrun` on Darwin or in PATH on all other platforms. Unfortunately, this PR is larger than I would like because it removes the `lldbDapOptions`. I believe these options are not necessary, and as previously implemented, they caused a spurious warning with this change. The problem was that the options were created before the custom factory. By moving the creation logic into the factory, we make sure it's only called after the factory has been registered. The upside is that this simplifies the code and removes a level of indirection. --- .../lldb-dap/src-ts/debug-adapter-factory.ts | 117 +++++++++++++++--- lldb/tools/lldb-dap/src-ts/extension.ts | 76 ++---------- 2 files changed, 112 insertions(+), 81 deletions(-) diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 2be21bfdf0dd69..d5616b2f8f6167 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -1,4 +1,7 @@ +import * as path from "path"; +import * as util from "util"; import * as vscode from "vscode"; + import { LLDBDapOptions } from "./types"; /** @@ -8,15 +11,7 @@ import { LLDBDapOptions } from "./types"; export class LLDBDapDescriptorFactory implements vscode.DebugAdapterDescriptorFactory { - private lldbDapOptions: LLDBDapOptions; - - constructor(lldbDapOptions: LLDBDapOptions) { - this.lldbDapOptions = lldbDapOptions; - } - - static async isValidDebugAdapterPath( - pathUri: vscode.Uri, - ): Promise<Boolean> { + static async isValidFile(pathUri: vscode.Uri): Promise<Boolean> { try { const fileStats = await vscode.workspace.fs.stat(pathUri); if (!(fileStats.type & vscode.FileType.File)) { @@ -28,6 +23,70 @@ export class LLDBDapDescriptorFactory return true; } + static async findDAPExecutable(): Promise<string | undefined> { + let executable = "lldb-dap"; + if (process.platform === "win32") { + executable = "lldb-dap.exe"; + } + + // Prefer lldb-dap from Xcode on Darwin. + if (process.platform === "darwin") { + try { + const exec = util.promisify(require("child_process").execFile); + let { stdout, stderr } = await exec("/usr/bin/xcrun", [ + "-find", + executable, + ]); + if (stdout) { + return stdout.toString().trimEnd(); + } + } catch (error) {} + } + + // Find lldb-dap in the user's path. + let env_path = + process.env["PATH"] || + (process.platform === "win32" ? process.env["Path"] : null); + if (!env_path) { + return undefined; + } + + const paths = env_path.split(path.delimiter); + for (const p of paths) { + const exe_path = path.join(p, executable); + if ( + await LLDBDapDescriptorFactory.isValidFile(vscode.Uri.file(exe_path)) + ) { + return exe_path; + } + } + + return undefined; + } + + static async getDAPExecutable( + session: vscode.DebugSession, + ): Promise<string | undefined> { + const config = vscode.workspace.getConfiguration( + "lldb-dap", + session.workspaceFolder, + ); + + // Prefer the explicitly specified path in the extension's configuration. + const configPath = config.get<string>("executable-path"); + if (configPath && configPath.length !== 0) { + return configPath; + } + + // Try finding the lldb-dap binary. + const foundPath = await LLDBDapDescriptorFactory.findDAPExecutable(); + if (foundPath) { + return foundPath; + } + + return undefined; + } + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, @@ -36,14 +95,42 @@ export class LLDBDapDescriptorFactory "lldb-dap", session.workspaceFolder, ); - const customPath = config.get<string>("executable-path"); - const path: string = customPath || executable!!.command; - const fileUri = vscode.Uri.file(path); - if (!(await LLDBDapDescriptorFactory.isValidDebugAdapterPath(fileUri))) { - LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(fileUri.path); + const log_path = config.get<string>("log-path"); + let env: { [key: string]: string } = {}; + if (log_path) { + env["LLDBDAP_LOG"] = log_path; + } + const configEnvironment = + config.get<{ [key: string]: string }>("environment") || {}; + const dapPath = await LLDBDapDescriptorFactory.getDAPExecutable(session); + const dbgOptions = { + env: { + ...executable?.options?.env, + ...configEnvironment, + ...env, + }, + }; + if (dapPath) { + const fileUri = vscode.Uri.file(dapPath); + if (!(await LLDBDapDescriptorFactory.isValidFile(fileUri))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(fileUri.path); + return undefined; + } + return new vscode.DebugAdapterExecutable(dapPath, [], dbgOptions); + } else if (executable) { + const fileUri = vscode.Uri.file(executable.command); + if (!(await LLDBDapDescriptorFactory.isValidFile(fileUri))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(fileUri.path); + return undefined; + } + return new vscode.DebugAdapterExecutable( + executable.command, + executable.args, + dbgOptions, + ); } - return this.lldbDapOptions.createDapExecutableCommand(session, executable); + return undefined; } /** diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index 36d3dfba18c142..4924641d3fa291 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -1,74 +1,22 @@ +import * as path from "path"; +import * as util from "util"; import * as vscode from "vscode"; -import { LLDBDapOptions } from "./types"; -import { DisposableContext } from "./disposable-context"; -import { LLDBDapDescriptorFactory } from "./debug-adapter-factory"; - -/** - * This creates the configurations for this project if used as a standalone - * extension. - */ -function createDefaultLLDBDapOptions(): LLDBDapOptions { - return { - debuggerType: "lldb-dap", - async createDapExecutableCommand( - session: vscode.DebugSession, - packageJSONExecutable: vscode.DebugAdapterExecutable | undefined, - ): Promise<vscode.DebugAdapterExecutable | undefined> { - const config = vscode.workspace.getConfiguration( - "lldb-dap", - session.workspaceFolder, - ); - const path = config.get<string>("executable-path"); - const log_path = config.get<string>("log-path"); - let env: { [key: string]: string } = {}; - if (log_path) { - env["LLDBDAP_LOG"] = log_path; - } - const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; - if (path) { - const dbgOptions = { - env: { - ...configEnvironment, - ...env, - } - }; - return new vscode.DebugAdapterExecutable(path, [], dbgOptions); - } else if (packageJSONExecutable) { - return new vscode.DebugAdapterExecutable( - packageJSONExecutable.command, - packageJSONExecutable.args, - { - ...packageJSONExecutable.options, - env: { - ...packageJSONExecutable.options?.env, - ...configEnvironment, - ...env, - }, - }, - ); - } else { - return undefined; - } - }, - }; -} +import { LLDBDapDescriptorFactory } from "./debug-adapter-factory"; +import { DisposableContext } from "./disposable-context"; +import { LLDBDapOptions } from "./types"; /** * This class represents the extension and manages its life cycle. Other extensions * using it as as library should use this class as the main entry point. */ export class LLDBDapExtension extends DisposableContext { - private lldbDapOptions: LLDBDapOptions; - - constructor(lldbDapOptions: LLDBDapOptions) { + constructor() { super(); - this.lldbDapOptions = lldbDapOptions; - this.pushSubscription( vscode.debug.registerDebugAdapterDescriptorFactory( - this.lldbDapOptions.debuggerType, - new LLDBDapDescriptorFactory(this.lldbDapOptions), + "lldb-dap", + new LLDBDapDescriptorFactory(), ), ); @@ -81,9 +29,7 @@ export class LLDBDapExtension extends DisposableContext { if (dapPath) { const fileUri = vscode.Uri.file(dapPath); - if ( - await LLDBDapDescriptorFactory.isValidDebugAdapterPath(fileUri) - ) { + if (await LLDBDapDescriptorFactory.isValidFile(fileUri)) { return; } } @@ -98,7 +44,5 @@ export class LLDBDapExtension extends DisposableContext { * This is the entry point when initialized by VS Code. */ export function activate(context: vscode.ExtensionContext) { - context.subscriptions.push( - new LLDBDapExtension(createDefaultLLDBDapOptions()), - ); + context.subscriptions.push(new LLDBDapExtension()); } >From 54e4105a3fa24844cb5feb21ce493c7b19d49198 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Tue, 3 Dec 2024 16:06:23 -0800 Subject: [PATCH 2/2] Address code review feedback --- .../lldb-dap/src-ts/debug-adapter-factory.ts | 155 +++++++++--------- lldb/tools/lldb-dap/src-ts/extension.ts | 9 +- lldb/tools/lldb-dap/src-ts/types.ts | 23 --- 3 files changed, 85 insertions(+), 102 deletions(-) delete mode 100644 lldb/tools/lldb-dap/src-ts/types.ts diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index d5616b2f8f6167..55c2f3e9f7deb5 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -1,92 +1,99 @@ import * as path from "path"; import * as util from "util"; import * as vscode from "vscode"; +import * as child_process from "child_process"; +import * as fs from "node:fs/promises"; -import { LLDBDapOptions } from "./types"; +export async function isExecutable(path: string): Promise<Boolean> { + try { + await fs.access(path, fs.constants.X_OK); + } catch { + return false; + } + return true; +} -/** - * This class defines a factory used to find the lldb-dap binary to use - * depending on the session configuration. - */ -export class LLDBDapDescriptorFactory - implements vscode.DebugAdapterDescriptorFactory -{ - static async isValidFile(pathUri: vscode.Uri): Promise<Boolean> { +async function findWithXcrun(executable: string): Promise<string | undefined> { + if (process.platform === "darwin") { try { - const fileStats = await vscode.workspace.fs.stat(pathUri); - if (!(fileStats.type & vscode.FileType.File)) { - return false; + const exec = util.promisify(child_process.execFile); + let { stdout, stderr } = await exec("/usr/bin/xcrun", [ + "-find", + executable, + ]); + if (stdout) { + return stdout.toString().trimEnd(); } - } catch (err) { - return false; - } - return true; + } catch (error) {} } + return undefined; +} - static async findDAPExecutable(): Promise<string | undefined> { - let executable = "lldb-dap"; - if (process.platform === "win32") { - executable = "lldb-dap.exe"; - } +async function findInPath(executable: string): Promise<string | undefined> { + const env_path = + process.platform === "win32" ? process.env["Path"] : process.env["PATH"]; + if (!env_path) { + return undefined; + } - // Prefer lldb-dap from Xcode on Darwin. - if (process.platform === "darwin") { - try { - const exec = util.promisify(require("child_process").execFile); - let { stdout, stderr } = await exec("/usr/bin/xcrun", [ - "-find", - executable, - ]); - if (stdout) { - return stdout.toString().trimEnd(); - } - } catch (error) {} + const paths = env_path.split(path.delimiter); + for (const p of paths) { + const exe_path = path.join(p, executable); + if (await isExecutable(exe_path)) { + return exe_path; } + } + return undefined; +} - // Find lldb-dap in the user's path. - let env_path = - process.env["PATH"] || - (process.platform === "win32" ? process.env["Path"] : null); - if (!env_path) { - return undefined; - } +async function findDAPExecutable(): Promise<string | undefined> { + const executable = process.platform === "win32" ? "lldb-dap.exe" : "lldb-dap"; - const paths = env_path.split(path.delimiter); - for (const p of paths) { - const exe_path = path.join(p, executable); - if ( - await LLDBDapDescriptorFactory.isValidFile(vscode.Uri.file(exe_path)) - ) { - return exe_path; - } - } + // Prefer lldb-dap from Xcode on Darwin. + const xcrun_dap = findWithXcrun(executable); + if (xcrun_dap) { + return xcrun_dap; + } - return undefined; + // Find lldb-dap in the user's path. + const path_dap = findInPath(executable); + if (path_dap) { + return path_dap; } - static async getDAPExecutable( - session: vscode.DebugSession, - ): Promise<string | undefined> { - const config = vscode.workspace.getConfiguration( - "lldb-dap", - session.workspaceFolder, - ); + return undefined; +} - // Prefer the explicitly specified path in the extension's configuration. - const configPath = config.get<string>("executable-path"); - if (configPath && configPath.length !== 0) { - return configPath; - } +async function getDAPExecutable( + session: vscode.DebugSession, +): Promise<string | undefined> { + const config = vscode.workspace.getConfiguration( + "lldb-dap", + session.workspaceFolder, + ); - // Try finding the lldb-dap binary. - const foundPath = await LLDBDapDescriptorFactory.findDAPExecutable(); - if (foundPath) { - return foundPath; - } + // Prefer the explicitly specified path in the extension's configuration. + const configPath = config.get<string>("executable-path"); + if (configPath && configPath.length !== 0) { + return configPath; + } - return undefined; + // Try finding the lldb-dap binary. + const foundPath = await findDAPExecutable(); + if (foundPath) { + return foundPath; } + return undefined; +} + +/** + * This class defines a factory used to find the lldb-dap binary to use + * depending on the session configuration. + */ +export class LLDBDapDescriptorFactory + implements vscode.DebugAdapterDescriptorFactory +{ async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, @@ -103,7 +110,7 @@ export class LLDBDapDescriptorFactory } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; - const dapPath = await LLDBDapDescriptorFactory.getDAPExecutable(session); + const dapPath = await getDAPExecutable(session); const dbgOptions = { env: { ...executable?.options?.env, @@ -112,16 +119,14 @@ export class LLDBDapDescriptorFactory }, }; if (dapPath) { - const fileUri = vscode.Uri.file(dapPath); - if (!(await LLDBDapDescriptorFactory.isValidFile(fileUri))) { - LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(fileUri.path); + if (!(await isExecutable(dapPath))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); return undefined; } return new vscode.DebugAdapterExecutable(dapPath, [], dbgOptions); } else if (executable) { - const fileUri = vscode.Uri.file(executable.command); - if (!(await LLDBDapDescriptorFactory.isValidFile(fileUri))) { - LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(fileUri.path); + if (!(await isExecutable(executable.command))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(executable.command); return undefined; } return new vscode.DebugAdapterExecutable( diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index 4924641d3fa291..71fd48298f8f53 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -2,9 +2,11 @@ import * as path from "path"; import * as util from "util"; import * as vscode from "vscode"; -import { LLDBDapDescriptorFactory } from "./debug-adapter-factory"; +import { + LLDBDapDescriptorFactory, + isExecutable, +} from "./debug-adapter-factory"; import { DisposableContext } from "./disposable-context"; -import { LLDBDapOptions } from "./types"; /** * This class represents the extension and manages its life cycle. Other extensions @@ -28,8 +30,7 @@ export class LLDBDapExtension extends DisposableContext { .get<string>("executable-path"); if (dapPath) { - const fileUri = vscode.Uri.file(dapPath); - if (await LLDBDapDescriptorFactory.isValidFile(fileUri)) { + if (await isExecutable(dapPath)) { return; } } diff --git a/lldb/tools/lldb-dap/src-ts/types.ts b/lldb/tools/lldb-dap/src-ts/types.ts deleted file mode 100644 index 63a8c73982caf3..00000000000000 --- a/lldb/tools/lldb-dap/src-ts/types.ts +++ /dev/null @@ -1,23 +0,0 @@ -import * as vscode from "vscode"; - -/** - * Callback used to generate the actual command to be executed to launch the lldb-dap binary. - * - * @param session - The information of the debug session to be launched. - * - * @param packageJSONExecutable - An optional {@link vscode.DebugAdapterExecutable executable} for - * lldb-dap if specified in the package.json file. - */ -export type LLDBDapCreateDAPExecutableCommand = ( - session: vscode.DebugSession, - packageJSONExecutable: vscode.DebugAdapterExecutable | undefined, -) => Promise<vscode.DebugAdapterExecutable | undefined>; - -/** - * The options that this extension accepts. - */ -export interface LLDBDapOptions { - createDapExecutableCommand: LLDBDapCreateDAPExecutableCommand; - // The name of the debugger type as specified in the package.json file. - debuggerType: string; -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits