From 1b28145983091cbe8639d2816fb03ccb3950bbc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Csomor?= Date: Mon, 4 Mar 2019 11:04:01 +0100 Subject: [PATCH] maprule: use ctx.resolve_tools (#117) In this PR: - In the _resolve_locations function: use the Bash-less ctx.resolve_tools function to resolve the runfiles manifests and inputs of tools, instead of using ctx.resolve_command for the same purpose. - In the _custom_envmap function: no longer resolve $(location) references when creating the envvars from custom_env, because those references were already resolved in _resolve_locations. The ctx.resolve_tools() method was added in this PR: https://github.com/bazelbuild/bazel/pull/7139 See design doc there. --- rules/BUILD | 6 +++ rules/maprule_private.bzl | 79 ++++----------------------------------- rules/maprule_util.bzl | 68 +++++++++++++++++++++++++++++++++ tests/maprule_tests.bzl | 2 +- 4 files changed, 83 insertions(+), 72 deletions(-) create mode 100644 rules/maprule_util.bzl diff --git a/rules/BUILD b/rules/BUILD index e0d349b..c5340f9 100644 --- a/rules/BUILD +++ b/rules/BUILD @@ -21,11 +21,17 @@ bzl_library( srcs = ["maprule_private.bzl"], visibility = ["//visibility:private"], deps = [ + ":maprule_util", "//lib:dicts", "//lib:paths", ], ) +bzl_library( + name = "maprule_util", + srcs = ["maprule_util.bzl"], +) + # Exported for build_test.bzl to make sure of, it is an implementation detail # of the rule and should not be directly used by anything else. exports_files(["empty_test.sh"]) diff --git a/rules/maprule_private.bzl b/rules/maprule_private.bzl index ae44cf9..0b4b3af 100644 --- a/rules/maprule_private.bzl +++ b/rules/maprule_private.bzl @@ -25,6 +25,7 @@ This module exports: load("//lib:dicts.bzl", "dicts") load("//lib:paths.bzl", "paths") +load(":maprule_util.bzl", "resolve_locations") _cmd_maprule_intro = """ Maprule that runs a Windows Command Prompt (`cmd.exe`) command. @@ -390,66 +391,7 @@ def _create_outputs(ctx, ctx_label_name, ctx_attr_outs_templates, strategy, fore else: return outs_dicts, all_output_files, src_placeholders_dicts, None -def _resolve_locations(ctx, strategy, ctx_attr_add_env, ctx_attr_tools): - # ctx.resolve_command returns a Bash command. All we need though is the inputs and runfiles - # manifests (we expand $(location) references with ctx.expand_location below), so ignore the - # tuple's middle element (the resolved command). - inputs_from_tools, _, manifests_from_tools = ctx.resolve_command( - # Pretend that the additional envvars are forming a command, so resolve_command will resolve - # $(location) references in them (which we ignore here) and add their inputs and manifests - # to the results (which we really care about). - command = " ".join(ctx_attr_add_env.values()), - tools = ctx_attr_tools, - ) - - errors = [] - location_expressions = [] - parts = {} - was_anything_to_resolve = False - for k, v in ctx_attr_add_env.items(): - # Look for "$(location ...)" or "$(locations ...)", resolve if found. - # _validate_attributes already ensured that there's at most one $(location/s ...) in "v". - if "$(location" in v: - tokens = v.split("$(location") - was_anything_to_resolve = True - closing_paren = tokens[1].find(")") - location_expressions.append("$(location" + tokens[1][:closing_paren + 1]) - parts[k] = (tokens[0], tokens[1][closing_paren + 1:]) - else: - location_expressions.append("") - - if errors: - return None, None, None, errors - - resolved_add_env = {} - if was_anything_to_resolve: - # Resolve all $(location) expressions in one go. Should be faster than resolving them - # one-by-one. - all_location_expressions = "".join(location_expressions) - all_resolved_locations = ctx.expand_location(all_location_expressions) - resolved_locations = strategy.as_path(all_resolved_locations).split("") - - i = 0 - - # Starlark dictionaries have a deterministic order of iteration, so the element order in - # "resolved_locations" matches the order in "location_expressions", i.e. the previous - # iteration order of "ctx_attr_add_env". - for k, v in ctx_attr_add_env.items(): - if location_expressions[i]: - head, tail = parts[k] - resolved_add_env[k] = head + resolved_locations[i] + tail - else: - resolved_add_env[k] = v - i += 1 - else: - resolved_add_env = ctx_attr_add_env - - if errors: - return None, None, None, errors - else: - return inputs_from_tools, manifests_from_tools, resolved_add_env, None - -def _custom_envmap(ctx, strategy, src_placeholders, outs_dict, add_env): +def _custom_envmap(ctx, strategy, src_placeholders, outs_dict, resolved_add_env): return dicts.add( { "MAPRULE_" + k.upper(): strategy.as_path(v) @@ -460,8 +402,8 @@ def _custom_envmap(ctx, strategy, src_placeholders, outs_dict, add_env): for k, v in outs_dict.items() }, { - "MAPRULE_" + k.upper(): strategy.as_path(ctx.expand_location(v)).format(**src_placeholders) - for k, v in add_env.items() + "MAPRULE_" + k.upper(): v + for k, v in resolved_add_env.items() }, ) @@ -501,20 +443,15 @@ def _maprule_main(ctx, strategy): {"MAPRULE_SRCS": " ".join([strategy.as_path(p.path) for p in common_srcs_list])}, ) - # Resolve $(location) references in "cmd" and in "add_env". - inputs_from_tools, manifests_from_tools, add_env, errors = _resolve_locations( - ctx, - strategy, - ctx.attr.add_env, - ctx.attr.tools, - ) - _fail_if_errors(errors) + # Resolve "tools" runfiles and $(location) references in "add_env". + inputs_from_tools, manifests_from_tools = ctx.resolve_tools(tools = ctx.attr.tools) + add_env = resolve_locations(ctx, strategy, ctx.attr.add_env) # Create actions for each of the "foreach" sources. for src in foreach_srcs: strategy.create_action( ctx, - inputs = depset(direct = [src] + inputs_from_tools, transitive = [common_srcs]), + inputs = depset(direct = [src], transitive = [common_srcs, inputs_from_tools]), outputs = foreach_src_outs_dicts[src].values(), # The custom envmap contains envvars specific to the current "src", such as MAPRULE_SRC. env = common_envmap + _custom_envmap( diff --git a/rules/maprule_util.bzl b/rules/maprule_util.bzl new file mode 100644 index 0000000..260ba86 --- /dev/null +++ b/rules/maprule_util.bzl @@ -0,0 +1,68 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Utilities for maprule.""" + +def resolve_locations(ctx, strategy, d): + """Resolve $(location) references in the values of a dictionary. + + Args: + ctx: the 'ctx' argument of the rule implementation function + strategy: a struct with an 'as_path(string) -> string' function + d: {string: string} dictionary; values may contain $(location) references + for labels declared in the rule's 'srcs' and 'tools' attributes + + Returns: + {string: string} dict, same as 'd' except "$(location)" references are + resolved. + """ + location_expressions = [] + parts = {} + was_anything_to_resolve = False + for k, v in d.items(): + # Look for "$(location ...)" or "$(locations ...)", resolve if found. + # _validate_attributes already ensured that there's at most one $(location/s ...) in "v". + if "$(location" in v: + tokens = v.split("$(location") + was_anything_to_resolve = True + closing_paren = tokens[1].find(")") + location_expressions.append("$(location" + tokens[1][:closing_paren + 1]) + parts[k] = (tokens[0], tokens[1][closing_paren + 1:]) + else: + location_expressions.append("") + + resolved = {} + if was_anything_to_resolve: + # Resolve all $(location) expressions in one go. Should be faster than resolving them + # one-by-one. + all_location_expressions = "".join(location_expressions) + all_resolved_locations = ctx.expand_location(all_location_expressions) + resolved_locations = strategy.as_path(all_resolved_locations).split("") + + i = 0 + + # Starlark dictionaries have a deterministic order of iteration, so the element order in + # "resolved_locations" matches the order in "location_expressions", i.e. the previous + # iteration order of "d". + for k, v in d.items(): + if location_expressions[i]: + head, tail = parts[k] + resolved[k] = head + resolved_locations[i] + tail + else: + resolved[k] = v + i += 1 + else: + resolved = d + + return resolved diff --git a/tests/maprule_tests.bzl b/tests/maprule_tests.bzl index 5261d9a..ddb4418 100644 --- a/tests/maprule_tests.bzl +++ b/tests/maprule_tests.bzl @@ -283,7 +283,7 @@ def _custom_envmap_test(ctx): "out1": _mock_file(ctx, language + "/Foo/Out1"), "out2": _mock_file(ctx, language + "/Foo/Out2"), }, - add_env = {"ENV1": "Env1"}, + resolved_add_env = {"ENV1": "Env1"}, ) _assert_dict_keys( env,