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.

Sign up for GitHub

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

Jump to bottom

Pass environment variables from BlazeCidrLauncher down to bazel #5771

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Pass environment variables from BlazeCidrLauncher down to bazel #5771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...m/google/idea/blaze/base/run/confighandler/BlazeCommandGenericRunConfigurationRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import com.intellij.execution.ExecutionResult;
import com.intellij.execution.Executor;
import com.intellij.execution.configurations.CommandLineState;
import com.intellij.execution.configurations.GeneralCommandLine;
import com.intellij.execution.configurations.RunProfileState;
import com.intellij.execution.filters.Filter;
import com.intellij.execution.filters.TextConsoleBuilderImpl;
Expand Down Expand Up @@ -193,7 +194,7 @@ private ProcessHandler getScopedProcessHandler(
throws ExecutionException {
return new ScopedBlazeProcessHandler(
project,
blazeCommand,
new GeneralCommandLine(blazeCommand.toList()),
workspaceRoot,
new ScopedBlazeProcessHandler.ScopedProcessHandlerDelegate() {
@Override
Expand Down
14 changes: 3 additions & 11 deletions base/src/com/google/idea/blaze/base/run/processhandler/ScopedBlazeProcessHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,24 +63,16 @@ public interface ScopedProcessHandlerDelegate {
/**
* Construct a process handler and a context to be used for the life of the process.
*
* @param blazeCommand the blaze command to run
* @param command the blaze command to run
* @param workspaceRoot workspace root
* @param scopedProcessHandlerDelegate delegate methods that will be run with the process's
* context.
* @throws ExecutionException
*/
public ScopedBlazeProcessHandler(
Project project,
BlazeCommand blazeCommand,
WorkspaceRoot workspaceRoot,
ScopedProcessHandlerDelegate scopedProcessHandlerDelegate)
throws ExecutionException {
this(project, blazeCommand.toList(), workspaceRoot, scopedProcessHandlerDelegate);
}

public ScopedBlazeProcessHandler(
Project project,
List<String> command,
GeneralCommandLine command,
WorkspaceRoot workspaceRoot,
ScopedProcessHandlerDelegate scopedProcessHandlerDelegate)
throws ExecutionException {
Expand Down Expand Up @@ -132,7 +124,7 @@ public void processWillTerminate(ProcessEvent event, boolean willBeDestroyed) {
}

private static class CommandLineWithRemappedPath extends GeneralCommandLine {
CommandLineWithRemappedPath(List<String> command) {
CommandLineWithRemappedPath(GeneralCommandLine command) {
super(command);
}

Expand Down
2 changes: 1 addition & 1 deletion base/src/com/google/idea/blaze/base/run/state/EnvironmentVariablesState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private static class Editor implements RunConfigurationStateEditor {
private final EnvironmentVariablesComponent component = new EnvironmentVariablesComponent();

private Editor() {
component.setText("&Environment variables (only set when debugging)");
component.setText("&Environment variables");
}

@Override
Expand Down
14 changes: 8 additions & 6 deletions clwb/src/com/google/idea/blaze/clwb/run/BlazeCidrLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,16 @@ private ProcessHandler createProcess(CommandLineState state, List<String> extraB
WorkspaceRoot workspaceRoot = WorkspaceRoot.fromProject(project);
final BlazeCommand command = commandBuilder.build();

final GeneralCommandLine commandLine = new GeneralCommandLine(command.toList());

EnvironmentVariablesData envState = handlerState.getEnvVarsState().getData();
commandLine.withParentEnvironmentType(
envState.isPassParentEnvs() ? ParentEnvironmentType.SYSTEM : ParentEnvironmentType.NONE);
commandLine.getEnvironment().putAll(envState.getEnvs());

return new ScopedBlazeProcessHandler(
project,
command,
commandLine,
workspaceRoot,
new ScopedBlazeProcessHandler.ScopedProcessHandlerDelegate() {
@Override
Expand Down Expand Up @@ -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

commandLine.withParentEnvironmentType(
envState.isPassParentEnvs() ? ParentEnvironmentType.SYSTEM : ParentEnvironmentType.NONE);
commandLine.getEnvironment().putAll(envState.getEnvs());

if (CppBlazeRules.RuleTypes.CC_TEST.getKind().equals(configuration.getTargetKind())) {
convertBlazeTestFilterToExecutableFlag().ifPresent(commandLine::addParameters);
}
Expand Down
3 changes: 2 additions & 1 deletion java/src/com/google/idea/blaze/java/run/BlazeJavaRunProfileState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import com.intellij.execution.ExecutionException;
import com.intellij.execution.ExecutionResult;
import com.intellij.execution.Executor;
import com.intellij.execution.configurations.GeneralCommandLine;
import com.intellij.execution.filters.TextConsoleBuilderImpl;
import com.intellij.execution.process.ProcessHandler;
import com.intellij.execution.process.ProcessListener;
Expand Down Expand Up @@ -281,7 +282,7 @@ private ProcessHandler getScopedProcessHandler(
throws ExecutionException {
return new ScopedBlazeProcessHandler(
project,
command,
new GeneralCommandLine(command),
workspaceRoot,
new ScopedBlazeProcessHandler.ScopedProcessHandlerDelegate() {
@Override
Expand Down
Loading