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

Propagate test envs to xml generation action #12659

Closed

Conversation

meteorcloudy
Copy link
Member

@meteorcloudy meteorcloudy commented Dec 8, 2020

Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes #4137
Fixes #12579

@meteorcloudy
Copy link
Member Author

fyi @ulfjack

Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system library and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
return new SimpleSpawn(
action,
args,
ImmutableMap.of(
"PATH", "/usr/bin:/bin",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure we don't need the PATH and the TEST_BINARY?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two are always set by the test action. Because are passing the test action envs to the xml generation action, we don't need to explicitly set them anymore.

ImmutableMap.Builder<String, String> envBuilder = ImmutableMap.builder();
envBuilder.putAll(testEnv).put("TEST_NAME", action.getTestName());
if (!action.isSharded()) {
envBuilder.put("TEST_SHARD_INDEX", "0");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do we write those two when the action is not sharded?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Np, we don't have those for the actual test action, only set for the xml generation action.

@ulfjack
Copy link
Contributor

ulfjack commented Dec 14, 2020

Thanks!

@bazel-io bazel-io closed this in 3551898 Dec 15, 2020
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Jul 15, 2021
Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
Fixes bazelbuild#12579

Closes bazelbuild#12659.

PiperOrigin-RevId: 347596753
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Jul 15, 2021
Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
Fixes bazelbuild#12579

Closes bazelbuild#12659.

PiperOrigin-RevId: 347596753
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Jul 15, 2021
Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
Fixes bazelbuild#12579

Closes bazelbuild#12659.

PiperOrigin-RevId: 347596753
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Jul 15, 2021
Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
Fixes bazelbuild#12579

Closes bazelbuild#12659.

PiperOrigin-RevId: 347596753
coeuvre pushed a commit to coeuvre/bazel that referenced this pull request Jul 16, 2021
Previously, we hardcode the envs of the xml generation action, which
caused problem for process-wrapper because it's dynamically linked to
some system libraries and the required PATH or LD_LIBRARY_PATH are not
set.

This change propagate the envs we set for the actual test action to the
xml file generation action to make sure the env vars are correctly set and
can also be controlled by --action_env and --test_env.

Fixes bazelbuild#4137
Fixes bazelbuild#12579

Closes bazelbuild#12659.

PiperOrigin-RevId: 347596753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generate-xml.sh fails to execute ENV not propagated to process-wrapper
3 participants