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

Pass environment variables from BlazeCidrLauncher down to bazel #5771

Merged

Conversation

ujohnny
Copy link
Collaborator

@ujohnny ujohnny commented Nov 24, 2023

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

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

@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Nov 24, 2023
@ujohnny ujohnny force-pushed the enovozhilov/env-vars-to-bazel-run-debug branch from ef8d38c to 8b5c0f8 Compare November 29, 2023 12:48
@ujohnny ujohnny changed the title WIP: Pass environment variables from BlazeCidrLauncher down to bazel Pass environment variables from BlazeCidrLauncher down to bazel Nov 29, 2023
@ujohnny
Copy link
Collaborator Author

ujohnny commented Nov 29, 2023

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
@ujohnny ujohnny force-pushed the enovozhilov/env-vars-to-bazel-run-debug branch from 8b5c0f8 to 1b0855f Compare November 30, 2023 12:02
@tpasternak tpasternak merged commit a515688 into bazelbuild:master Dec 11, 2023
6 checks passed
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Dec 11, 2023
@@ -234,11 +241,6 @@ public CidrDebugProcess createDebugProcess(CommandLineState state, XDebugSession
commandLine.addParameters(handlerState.getExeFlagsState().getFlagsForExternalProcesses());
commandLine.addParameters(handlerState.getTestArgs());

EnvironmentVariablesData envState = handlerState.getEnvVarsState().getData();
Copy link
Collaborator

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?

Copy link
Collaborator

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

https://github.com/bazelbuild/intellij/pull/5771/files#diff-00e836fdd8bc72a7c8afc8a99218bf627c22fdde6649d55acefc02abe392b990R263

Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
4 participants