Skip to content

Commit

Permalink
use --platform_suffix - so we do not have to manually symlink just to…
Browse files Browse the repository at this point in the history
… get nicer output names
  • Loading branch information
nickbreen committed Sep 18, 2023
1 parent 27c4a75 commit 0b3b2d9
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 13 deletions.
8 changes: 8 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ platforms(
# Will execute all binaries on the host platform.
sh_test(
name = "host-hellos-test",
size = "small",
srcs = ["hello.test.sh"],
args = ["$(rootpaths :hellos)"],
data = [":hellos"],
Expand All @@ -59,6 +60,7 @@ sh_test(
# Not executed directly, but if done so will fail to build :hello as above.
sh_test(
name = "platform-hello-test",
size = "small",
srcs = ["hello.test.sh"],
args = ["$(rootpath :hello)"],
data = [":hello"],
Expand Down Expand Up @@ -101,6 +103,7 @@ platforms(
# Will extract and execute all binaries on the host platform.
sh_test(
name = "host-tars-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpaths :tars)"],
data = [":tars"],
Expand All @@ -109,6 +112,7 @@ sh_test(
# Not executed directly, but if done so will fail to build :hello as above.
sh_test(
name = "platform-tar-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpath :tar)"],
data = [":tar"],
Expand Down Expand Up @@ -153,6 +157,7 @@ platforms(
# on a host with RPM Utils and manually specifying a --extra_execution_platform
sh_test(
name = "host-rpms-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpaths :rpms)"],
data = [":rpms"],
Expand All @@ -162,6 +167,7 @@ sh_test(
# Not executed directly, but if done so will fail to build :hello as above.
sh_test(
name = "platform-rpm-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpaths :rpm)"], # pkg_rpm produces two RPM files >:(
data = [":rpm"],
Expand Down Expand Up @@ -202,6 +208,7 @@ platforms(
# on a host with RPM Utils and manually specifying a --extra_execution_platform
sh_test(
name = "host-debs-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpaths :debs)"],
data = [":debs"],
Expand All @@ -211,6 +218,7 @@ sh_test(
# Not executed directly, but if done so will fail to build :hello as above.
sh_test(
name = "platform-deb-test",
size = "small",
srcs = ["pkg.test.sh"],
args = ["$(rootpaths :deb)"],
data = [":deb"],
Expand Down
11 changes: 9 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,16 @@ Problems...
declare that it is executable on the host: that seems to propagate to being
instructions to compile `//:hello` for the host too.

3. We avoided having to think about the difference between target and exec platform by
specifying both with the same values. TODO figure that out. I.e. we should be able
3. We've avoided having to think about the difference between target and exec platform
by specifying both with the same values. TODO figure that out. I.e. we should be able
to specify that `//:tars` _targets_ all platforms, but can _execute_ on the host.

4. Adding `sh_test(size = "small")` didn't appear to appease `--test_verbose_timeout_warnings`.
Suspect that the size (among other attributes) does not get propagated through the
platforms rule.

5. Expected that `pkg_tar(tags = ["no-remote-exec"])` would have indicated that it
should execute on the host. Why didn't it?
---

References:
Expand Down
20 changes: 9 additions & 11 deletions defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ load("@bazel_skylib//lib:new_sets.bzl", "sets")
def _platforms_transition(settings, attr):
"""
Explode the build graph, possibly catastrophically for all our target platforms.
Also, transition to extra_execution_platform and extra_toolchains required for the
Also, transition to extra_execution_platform required for the
platform(s); registering them all in WORKSPACE seems to freak out:
- always selects the first execution platform: centos/6
- somehow seems to get confused which toolchain to use despite exec_compatible_with
and target_compatible_with and picks @local_config_cc?
TODO: figure that out; at least be able to explain why.
"""

Expand All @@ -17,20 +15,25 @@ def _platforms_transition(settings, attr):
splits = {split_key: {
"//command_line_option:platforms": [],
"//command_line_option:extra_execution_platforms": [],
"//command_line_option:platform_suffix": None,
} for split_key in split_keys}

for label in attr.target_platforms:
splits[label.name]["//command_line_option:platforms"] += ["%s" % label]
splits[label.name]["//command_line_option:platform_suffix"] = "%s_%s" % (label.package.replace("/", "_"), label.name.replace("/", "_"))
for label in attr.exec_platforms:
splits[label.name]["//command_line_option:extra_execution_platforms"] += ["%s" % label]

# Do we need to default to @local_config_platform//:host if either are empty?

return splits

platforms_transition = transition(
implementation = _platforms_transition,
inputs = [],
outputs = [
"//command_line_option:platforms",
"//command_line_option:platform_suffix",
"//command_line_option:extra_execution_platforms",
],
)
Expand All @@ -39,28 +42,23 @@ def _platform_transition(settings, attr):
return {
"//command_line_option:platforms": ["%s" % attr.platform],
"//command_line_option:extra_execution_platforms": ["%s" % attr.platform],
"//command_line_option:platform_suffix": "%s_%s" % (attr.platform.package.replace("/", "_"), attr.platform.name.replace("/", "_")),
}

platform_transition = transition(
implementation = _platform_transition,
inputs = [],
outputs = [
"//command_line_option:platforms",
"//command_line_option:platform_suffix",
"//command_line_option:extra_execution_platforms",
],
)

def _platforms(ctx):
"""Collect files for each platform."""
ins_and_outs = {
in_file: ctx.actions.declare_file("%s/%s" % (in_dir, in_file.basename))
for in_dir in ctx.split_attr.actual
for in_file in ctx.split_attr.actual[in_dir].files.to_list()
}
for in_file, out_file in ins_and_outs.items():
ctx.actions.symlink(output = out_file, target_file = in_file)

return [DefaultInfo(files = depset(ins_and_outs.values()))]
return [DefaultInfo(files = depset(transitive = [ctx.split_attr.actual[target].files for target in ctx.split_attr.actual]))]

platforms = rule(
implementation = _platforms,
Expand Down

0 comments on commit 0b3b2d9

Please sign in to comment.