add support for envFile #358
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,27 +25,30 @@ function couldBeOutput(line: string) { | |
| const trace = false; | ||
|
|
||
| export class MI2 extends EventEmitter implements IBackend { | ||
| constructor(public application: string, public preargs: string[], public extraargs: string[], procEnv: any, envFile: string, public extraCommands: string[] = []) { | ||
| super(); | ||
|
|
||
| if (!(envFile || procEnv)) | ||
| return; | ||
|
|
||
| // FIXME: if actuall debugged process is Win32, which we only know _after_ connect, | ||
| // then the keys must be read via hashmap using only upper-case for hashing | ||
| // ... or entered/compared in upper-case --> possibly postpone this | ||
| const env = {}; | ||
| // Duplicate process.env so we don't override it | ||
| for (const key in process.env) | ||
| if (process.env.hasOwnProperty(key)) | ||
| env[key] = process.env[key]; | ||
|
|
||
| // Overwrite with user specified variables | ||
| if (envFile) { | ||
| mergeEnv(env, readEnvFile(envFile)); | ||
| } | ||
| if (procEnv) { | ||
| mergeEnv(env, procEnv); | ||
| } | ||
|
|
||
| this.procEnv = env; | ||
| } | ||
|
|
||
| load(cwd: string, target: string, procArgs: string, separateConsole: string): Thenable<any> { | ||
|
|
@@ -886,3 +889,32 @@ export class MI2 extends EventEmitter implements IBackend { | |
| protected stream; | ||
| protected sshConn; | ||
| } | ||
| function readEnvFile(filename: string): { [key: string]: string } { | ||
|
There was a problem hiding this comment. I would probably recommend taking these functions, along with the functionality in the MI2 constructor above and moving it to its own class (e.g., "Environment"). This would encapsulate this functionality and make it easy to unit test. There was a problem hiding this comment. Yes, that's a general issue with the existing code - the environment variables are expected to be found in the inferior (this "just happens" when GDB starts it locally). If we cannot make this happen all environment settings should be dropped from all remote scenarios. There was a problem hiding this comment. There does appear to be commands in both gdb and lldb for setting environment variables, so using that approach, rather than attempting to set them at the process level might be the way to go. The environment variable setting commands could just be issued during the MI2::initCommands, similarly to how the path mappings are handled (i.e., extra commands). The GDB documentation even says these environment variable settings are sent to gdbserver, so should support remote debugging as well (see here). This should also work with SSH connections too, since the commands are issued to the debugger itself.
(gdb) set environment FOO=BAR
(gdb) unset environment FOO
(lldb) settings set target.env-vars FOO=BAR
(lldb) settings remove target.env-vars FOO |
||
| const env = {}; | ||
| if (!fs.existsSync(filename)) { | ||
| return env; | ||
| } | ||
| const buffer = fs.readFileSync(filename, 'utf8'); | ||
| buffer.split('\n').forEach( line => { | ||
| // using a match which will automatically ignore "wrong" lines including | ||
| // lines that are empty or start with a "#", or //, or ... | ||
| const m = line.match(/^\s*([\w\.\-]+)\s*=\s*(.*)?\s*$/); | ||
| if (m != undefined) { | ||
| env[m[1]] = m[2] || ''; | ||
| } | ||
| }); | ||
|
|
||
| throw new Error("Function not implemented."); | ||
| } | ||
|
|
||
| function mergeEnv(env: {}, buffer: { [key: string]: string; }) { | ||
| for (const key in buffer) { | ||
| if (buffer.hasOwnProperty(key)) { | ||
| if (buffer === undefined) | ||
| delete env[key]; | ||
| else | ||
| env[key] = buffer[key]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not really not know what type of process that will be debugged? The environment support is only applicable to the debugger running on the same machine as the debug adapter (not supported over SSH), so it should be able to determine the system it's running on via
process.platform. Not sure I understand this issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we don't know. I'm regularly using gdb to attach to gdbserver on a different machine myself. Works fine as long as the gdb has support for the target's architecture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But these environment variables are only being applied to the process where
gdbis being run, not the process wheregdbserveris run. I don't think I understand what the expectation of the environment variables would be in a remote scenario like that. Are they expected to be visible to the application being debugged?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likely yes, as long as we use
set environment(or its mi equivalent) this will work both for GDB and (at least on GNU/Linux environments) gdbserver (since GDB 8.1); for running processes wed need to call intoputenv()`, but I think it is fine to say that this is only supported for new inferiors.