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.

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

Add ability to set environment variables when creating a Debug Adapter #56646

Closed
glennsarti opened this Issue Aug 17, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@glennsarti
Copy link

glennsarti commented Aug 17, 2018

I currently have Debug Adapter implemented in Ruby language. Ruby has some configuration which is only possible to set via environment variables, such as GEM_HOME.

To get around this I have to write a large Typescript file which spawns my real Debug Adapter in ruby, and then just proxies stdin/stdout between the two. This is awkward, prone to error and quite limited as the debug adapters don't have access to the vscode configuration classes (separate process).

Ideally I should be able to set path, arguments and environment for debug adapter runtime.

Proposed changes

Currently the Debug Adpater runtime configuration only offers runtime path, and arguments, as can been see in;

https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/node/debugAdapter.ts#L299

Note that we cannot configure the spawn options, which is where the environment variables would go. Also other IO options but I'm not as interested in them.

Given that the DebugAdapterExecutable class is currently in a proposed state, I would like to add additional functionality to do this now.

Proposed changes (pseudo code style!)

	/**
	 * Represents a debug adapter executable and optional arguments passed to it.
	 */
	export class DebugAdapterExecutable {
		/**
		 * The command path of the debug adapter executable.
		 * A command must be either an absolute path or the name of an executable looked up via the PATH environment variable.
		 * The special value 'node' will be mapped to VS Code's built-in node runtime.
		 */
		readonly command: string;

		/**
		 * Optional arguments passed to the debug adapter executable.
		 */
		readonly args: string[];

		/**
		 * Optional environment variables set for the debug adapter executable.
		 */
		readonly env: object;   <---- Map<string, string> or something

		/**
		 * Create a new debug adapter specification.
		 */
		constructor(command: string, args?: string[], env?: Object);  <---- Map<string, string> or something
	}

And spawn option change

let spawn_options: cp.SpawnOptions = {};
spawn_options.env = mergeEnvironmentVariables(process.env, this.adapterExecutable.command.env);
this.serverProcess = cp.spawn(this.adapterExecutable.command, this.adapterExecutable.args, spawn_options);

Where mergeEnvironmentVariables would take a set of existing env vars (process.env) and then merge the new set of env vars into it.

Obviously there will be some other code changes to actually read in the env vars from the extension package.json. Or perhaps only offer this IF implementing via debugAdapterExecutable and not via package.json?

export interface IAdapterExecutable {
readonly command?: string;
readonly args?: string[];
}

Other options

Saying that environment should be configured BEFORE you start VSCode isn't always feasible or responsible. That means I can't use different runtimes depending on user settings.

Configuring the runtime environment BEFORE using code for MY debug adapter may break OTHER extensions unexpectedly. Given we're spawning a new process, it is feasible to have the VScode environment setup for Editing and the debug adapter setup for debugging.

@vscodebot

This comment has been minimized.

Copy link

vscodebot bot commented Aug 17, 2018

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@glennsarti

This comment has been minimized.

Copy link
Author

glennsarti commented Aug 17, 2018

Note that this is different to setting the environment of the debugee. This is about spawning the Debug Adapter itself, which can then spawn debugee processes.

@glennsarti

This comment has been minimized.

Copy link
Author

glennsarti commented Aug 17, 2018

Neither of the issues mentioned above are duplicates. I searched the current and closed issues and found nothing about this. Only about passing env vars into the debugee, not for the DA itself.

@isidorn isidorn assigned weinand and unassigned isidorn Aug 17, 2018

@isidorn isidorn added the debug label Aug 17, 2018

@weinand weinand added this to the On Deck milestone Aug 18, 2018

@glennsarti

This comment has been minimized.

Copy link
Author

glennsarti commented Aug 23, 2018

@weinand @isidorn Given this is for a proposed API change, would I be able to raise a PR myself for it?

@weinand

This comment has been minimized.

Copy link
Member

weinand commented Aug 23, 2018

@glennsarti thanks for the offer, but I'm currently working on this API anyways. So I will definitely consider this feature.

@weinand weinand modified the milestones: On Deck, September 2018 Sep 13, 2018

@weinand weinand closed this in be713f7 Sep 15, 2018

@weinand weinand added the on-testplan label Sep 21, 2018

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.