Skip to content

Commit

Permalink
Pass default_shell_env through actions
Browse files Browse the repository at this point in the history
Through the investigation of
bazelbuild/bazel#12049 one of the things I
discovered was that when using `actions.run` there are 2 options for
environment variables. `use_default_shell_env = True` is recommended,
but cannot be used if you want to also pass `env` to the actions. To
support Xcode version selection we have to pass `env` with those
variables. Without the default shell env, we only get the environment
variables defined in the crosstool, but not those passed with
`--action_env`. This now adds variables passed as
`--action_env=FOO=BAR`, but not those passed as `--action_env=FOO`
(where the value is supposed to pass through).

This is useful to ensure a few things:

1. The default PATH things are executed with includes /usr/local/bin.
   This can result in pollution of binaries from homebrew. Previously
   there was no way to limit this
2. This should be a good replacement for using custom Swift toolchains.
   Currently those environment variables only apply to some actions
   (excluding those from bazel) using `--action_env=TOOLCHAINS=foo`
   should work better than the current solution (this change can be made
   as a followup)
  • Loading branch information
keith authored and brentleyjones committed Jun 4, 2021
1 parent 3d6141f commit c5b0e71
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 5 deletions.
17 changes: 13 additions & 4 deletions swift/internal/linking.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Implementation of linking logic for Swift."""

load("@bazel_skylib//lib:collections.bzl", "collections")
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@bazel_skylib//lib:partial.bzl", "partial")
load(
"@bazel_tools//tools/build_defs/cc:action_names.bzl",
Expand All @@ -27,13 +28,15 @@ def _register_static_library_link_action(
cc_feature_configuration,
objects,
output,
env,
swift_toolchain):
"""Registers an action that creates a static library.
Args:
actions: The object used to register actions.
cc_feature_configuration: The C++ feature configuration to use when
constructing the action.
env: Extra environment to run archive command with.
objects: A list of `File`s denoting object (`.o`) files that will be
linked.
output: A `File` to which the output library will be written.
Expand Down Expand Up @@ -68,10 +71,13 @@ def _register_static_library_link_action(
else:
args.add_all(objects)

env = cc_common.get_environment_variables(
action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME,
feature_configuration = cc_feature_configuration,
variables = archiver_variables,
env = dicts.add(
env,
cc_common.get_environment_variables(
action_name = CPP_LINK_STATIC_LIBRARY_ACTION_NAME,
feature_configuration = cc_feature_configuration,
variables = archiver_variables,
),
)

execution_requirements_list = cc_common.get_execution_requirements(
Expand Down Expand Up @@ -105,6 +111,7 @@ def create_linker_input(
library_name,
objects,
owner,
env,
swift_toolchain,
additional_inputs = [],
user_link_flags = []):
Expand All @@ -126,6 +133,7 @@ def create_linker_input(
objects: A list of `File`s denoting object (`.o`) files that will be
linked.
owner: The `Label` of the target that owns this linker input.
env: Extra environment to launch the link command with.
swift_toolchain: The Swift toolchain provider to use when constructing
the action.
additional_inputs: A list of extra `File` inputs passed to the linking
Expand Down Expand Up @@ -154,6 +162,7 @@ def create_linker_input(
cc_feature_configuration = cc_feature_configuration,
objects = objects,
output = static_library,
env = env,
swift_toolchain = swift_toolchain,
)
else:
Expand Down
1 change: 1 addition & 0 deletions swift/internal/swift_grpc_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ def _swift_grpc_library_impl(ctx):
feature_configuration = feature_configuration,
),
compilation_outputs = compilation_outputs,
env = ctx.configuration.default_shell_env,
is_dynamic = False,
is_static = True,
library_name = ctx.label.name,
Expand Down
1 change: 1 addition & 0 deletions swift/internal/swift_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ def _swift_library_impl(ctx):
feature_configuration = feature_configuration,
),
compilation_outputs = compilation_outputs,
env = ctx.configuration.default_shell_env,
is_dynamic = False,
is_static = True,
library_name = ctx.label.name,
Expand Down
1 change: 1 addition & 0 deletions swift/internal/swift_module_alias.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def _swift_module_alias_impl(ctx):
feature_configuration = feature_configuration,
),
compilation_outputs = compilation_outputs,
env = ctx.configuration.default_shell_env,
is_dynamic = False,
is_static = True,
library_name = ctx.label.name,
Expand Down
1 change: 1 addition & 0 deletions swift/internal/swift_protoc_gen_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx):
feature_configuration = feature_configuration,
),
compilation_outputs = compilation_outputs,
env = aspect_ctx.configuration.default_shell_env,
is_dynamic = False,
is_static = True,
# Prevent conflicts with C++ protos in the same output directory,
Expand Down
5 changes: 4 additions & 1 deletion swift/internal/xcode_swift_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,10 @@ def _xcode_swift_toolchain_impl(ctx):
if _is_xcode_at_least_version(xcode_config, "12.5"):
requested_features.append(SWIFT_FEATURE_SUPPORTS_SYSTEM_MODULE_FLAG)

env = _xcode_env(platform = platform, xcode_config = xcode_config)
env = dicts.add(
ctx.configuration.default_shell_env,
_xcode_env(platform = platform, xcode_config = xcode_config),
)
execution_requirements = xcode_config.execution_info()
generated_header_rewriter = ctx.executable.generated_header_rewriter

Expand Down

0 comments on commit c5b0e71

Please sign in to comment.