From f35236e0ac3fab2ff58ef6d312e00d82536aefa6 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sat, 6 Nov 2021 17:48:23 +0100 Subject: [PATCH] feat(bazel): allow integration commands to resolve executables through expansion Gives a tool built using `nodejs_binary` or `sh_binary`, the tool cannot be used as binary in an integration test command currently because: * Bazel location expansion does not respect the executable "FilesToRun" information of the binaries, and rather sees _all_ outputs, erroring that `$(locations X)` need to be used instead. See e.g. https://github.com/bazelbuild/bazel/issues/11820) * The command is currently split by space and this decouples the Make funtion expression incorrectly into a broken expansion. e.g. `$(rootpath X)` will become `["$(rootpath", "X"]`). This commit fixes both things by relying on the Bazel `CommandHelper.java` class which handles the expansion of commands as expected, with similar semantics of a `genrule` as per `GenRuleBase.java`. This allows test authors to conveniently use a `nodejs_binary` or `sh_binary` as command binary because the `CommandHelper` knows exactly which targets provide executables or not. There is no good API for the plain Make expansion of a command itself, so we use a little trick (which is documented sufficiently in the code). --- bazel/integration/index.bzl | 60 ++++++++++++++++--- bazel/integration/test_runner/main.ts | 2 +- bazel/integration/test_runner/runner.ts | 9 +-- .../tests/external_command_script/BUILD.bazel | 15 +++++ .../external_script.sh | 9 +++ 5 files changed, 80 insertions(+), 15 deletions(-) create mode 100644 bazel/integration/tests/external_command_script/BUILD.bazel create mode 100644 bazel/integration/tests/external_command_script/external_script.sh diff --git a/bazel/integration/index.bzl b/bazel/integration/index.bzl index c1bf3693b0..5f2bff31dd 100644 --- a/bazel/integration/index.bzl +++ b/bazel/integration/index.bzl @@ -6,6 +6,14 @@ def _serialize_file(file): return struct(path = file.path, shortPath = file.short_path) +def _create_expanded_value(value, isExpanded): + """Creates a JSON serializable dictionary matching the `BazelExpandedValue` type in + the test runner.""" + return { + "value": value, + "containsExpansion": isExpanded, + } + def _serialize_and_expand_value(ctx, value, description): """Expands Bazel make variable and location expressions for the given value. Returns a JSON serializable dictionary matching the `BazelExpandedValue` type in the test runner.""" @@ -17,14 +25,50 @@ def _serialize_and_expand_value(ctx, value, description): # directly use `ctx.var` but would have switch users from e.g. `$(VAR)` to `{VAR}`. expanded_make_value = ctx.expand_make_variables(description, expanded_location_value, {}) - return { - "value": expanded_make_value, - "containsExpansion": expanded_make_value != value, - } + return _create_expanded_value(expanded_make_value, expanded_make_value != value) + +def _expand_and_split_command(ctx, command): + """Expands a command using the Bazel command helper. The command is then split into the + binary and its arguments, matching the runner `[BazelExpandedValue, ...string[]]` type.""" + + # Instead of manually resolving the command using e.g. `ctx.expand_location`, we use + # the Bazel `resolve_command` helper which internally follows the semantics of a `genrule`, + # allowing for better expansion/resolution of tools provided in the `data` attribute. + # This is necessary so that e.g. executables from a `sh_binary` can be conveniently + # expanded through `$(rootpath