Injecting Environment Variables to GDB and GDBSERVER processes

[asimgunes]

Hi,

I like to propose a new feature for cdt-gdb-adapter which enables injecting environment variables to gdb and gdbserver processes.

The new variables are located as environment and target.environment. The root environment injects environment variables to gdb process, where target.environment injects environment variables to gdbserver process.

Similar to cwd and target.cwd behaviour, gdbserver process also includes environment definition. (Which means gdbserver is injecting both environment and target.environment).

In this implementation, we also support construction of new values with using the old values. This is a common scenario for PATH environment variable in most cases. The following configuration will append a new path to the PATH variable:

PATH: '%PATH%;C:\some\new\path'

or

PATH: '$PATH:/some/new/path'

New value construction is not limited to the PATH variable, the logic could be used in any variable and the following formats are supported:

%VAR_NAME% format: TEST_VAR: "%TEST_VAR%;Some other text"

$VAR_NAME format: TEST_VAR: "$TEST_VAR;Some other text"

${env.VAR_NAME} format: TEST_VAR: "${env.TEST_VAR};Some other text"

I believe this could be a useful scenario in some cases where developers need to inject some environment variables seamlessly and improve the user experience.

To sum up, I would be happy if you can review this request, and happy to hear any feedback.

Kind regards. Asim Gunes

[asimgunes]

Some additional update notes:

1. I observe that there is a problem in the Windows integration tests. As far as I see, new tests are working fine but one old test is failing in the report.

2. I implemented new tests for utility functions, which checks the inject logic and text building formats, however, I couldn't find the proper way to test if the environment variables is injected/passed in to the spawn function. (Since CdtDebugClient is already creating a new process while running the tests, libraries like sinon could not create stub functions to check the parameters.), I do my tests manually and locally, out of scope of this update. I am open for suggestions on this point.

[jonahgraham]

For windows tests are broken, see #299

For tests I think we can use getenv C function in a new test class to see what environment ends up in the inferior. See https://www.tutorialspoint.com/c_standard_library/c_function_getenv.htm Eg you can call setenv on a few env vars and assign them to c variables, insert a breakpoint after those calls and ensure the C vars have expected values.

The other thing you can do is use cdt-gdb-tests/executeCommand to send arbitrary gdb commands to check the effect has worked as expected.

I'll do a review of the code itself soon.

[jonahgraham]

I tried adding ${env:PATH} to my launch.json and it was expanded as I expected with no special handling.

I experimented with case sensitivity and multiple vars in a single expansion and it all worked as I would expect according to https://code.visualstudio.com/docs/editor/variables-reference#_environment-variables

[jonahgraham]

@asimgunes please let me know when you are ready for me to re-review this, with a comment on pressing the "re-request review" button.

[asimgunes]

Hi @jonahgraham,

I think this pull request is ready to re-check right now.

To summarize, following changes performed:

1. Custom variable injection is removed so we leave the VSCode to handle ${env:VARNAME} format and we do not perform any string editing operation.
2. We cover the Windows environment variables handling scenario with the following rules: 2.1. We looked for case-insensitive environment variables key, if we found any, we update the value of this variable while keeping the existing casing. 2.2. If we do not found the variable than we insert the variable with the provided definition. 2.3. We ignore this check/control if operating system is not Windows.
3. Update the cdt-gdb-tests/executeCommand command and inject console output as well as the command result. I need to update this implementation due to executing command and getting results. (The previous implementation was neither getting the result nor getting the console output. I added both of them.)
4. Tests are updated to check environment and target.environment settings are working properly. We get the gdb process environment variables via gdb commands and inferior variables from the application itself.

Note: Both commands tried: show environment <VARNAME> and call (char *) getenv("<VARNAME>") to get the variables and both of them return the result in console output. Thats the reason for cdt-gdb-tests/executeCommand command update.

[asimgunes]

Hi @jonahgraham,

I made some updates about your comments, Could you please re-check the updates when you are available?

Kind regards.

[jonahgraham]

@asimgunes I made some changes to the code in line with my review comments. Please have a review of that and let me know if it looks good to you

[jonahgraham]

I have now fixed the Windows issue, rebasing this on main will pull in the fix for #299

[asimgunes]

Hi @jonahgraham,

Thanks for the updates, they all seem fine and I like the idea of lineTags. I also rebased the current request and all tests seem to be passed.

From my point of view, update seem fine.

[jonahgraham]

@asimgunes this is now merged - are there other changes incoming, or should I do a publish to npm and version bump?

Will you provide the corresponding change to cdt-gdb-vscode?

[asimgunes]

Hi @jonahgraham,

Thanks for the merge, there is no upcoming change from my side, and sure I can provide an update about related changes to the cdt-gdb-vscode, once you publish the recent cdt-gdb-adapter to npm.

[jonahgraham]

@asimgunes - I have published new version - https://www.npmjs.com/package/cdt-gdb-adapter/v/0.0.29 - can you consume that update when you make the cdt-gdb-vscode changes?

[jonahgraham]

can you consume that update when you make the cdt-gdb-vscode changes?

I am making the change to cdt-gdb-vscode now to pull in the fix in #306