-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conversation
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
de33fde
to
044fef4
Compare
return new SimpleSpawn( | ||
action, | ||
args, | ||
ImmutableMap.of( | ||
"PATH", "/usr/bin:/bin", |
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.
Are you sure we don't need the PATH
and the TEST_BINARY
?
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.
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"); |
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.
We do we write those two when the action is not sharded?
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.
Np, we don't have those for the actual test action, only set for the xml generation action.
4c97d40
to
3683879
Compare
Thanks! |
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
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
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
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
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
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