-
Notifications
You must be signed in to change notification settings - Fork 316
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
Pass environment variables from BlazeCidrLauncher down to bazel #5771
Pass environment variables from BlazeCidrLauncher down to bazel #5771
Conversation
ef8d38c
to
8b5c0f8
Compare
checked #2042 description and I think I have to remove the reference to it since environment variables are fixed for C++ targets only. |
BlazeCidrLauncher is aware of environment variables configured in run configuration window. So far it looks like no one just forwarded these variables down to executable process for some unknown reason. fixes bazelbuild#5770
8b5c0f8
to
1b0855f
Compare
@@ -234,11 +241,6 @@ public CidrDebugProcess createDebugProcess(CommandLineState state, XDebugSession | |||
commandLine.addParameters(handlerState.getExeFlagsState().getFlagsForExternalProcesses()); | |||
commandLine.addParameters(handlerState.getTestArgs()); | |||
|
|||
EnvironmentVariablesData envState = handlerState.getEnvVarsState().getData(); |
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.
why is it no longer needed for the debug process?
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.
If I understand correctly, it's been moved to createProcess, which is called later in this method
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.
I think the call to createProcess
is outside the if (!BlazeGDBServerProvider.shouldUseGdbserver())
which returns in line https://github.com/bazelbuild/intellij/pull/5771/files#diff-00e836fdd8bc72a7c8afc8a99218bf627c22fdde6649d55acefc02abe392b990R259
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.
well I missed this comment as the change was merged and wish I addressed back then. my apologies here.
Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number: #5770, #2042
Description of this change
BlazeCidrLauncher is aware of environment variables configured in run configuration window.
So far it looks like no one just forwarded these variables down to executable process for some
unknown reason.
fixes #5770, fixes #2042