From 7729ee38f3b4f7e236ceb48856ab5aab0278beb7 Mon Sep 17 00:00:00 2001 From: Mike Lundy Date: Fri, 1 Nov 2024 09:29:46 -0700 Subject: [PATCH] fix undefined variables in the wrap_outputs script (#1278) --- .../private/built_tools_framework.bzl | 16 ++++++---- foreign_cc/private/framework.bzl | 30 ++++++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/foreign_cc/built_tools/private/built_tools_framework.bzl b/foreign_cc/built_tools/private/built_tools_framework.bzl index 14313783..6c9b2236 100644 --- a/foreign_cc/built_tools/private/built_tools_framework.bzl +++ b/foreign_cc/built_tools/private/built_tools_framework.bzl @@ -70,15 +70,13 @@ def built_tool_rule_impl(ctx, script_lines, out_dir, mnemonic, additional_tools root = detect_root(ctx.attr.srcs) lib_name = ctx.attr.name - env_prelude = get_env_prelude(ctx, lib_name, [], "") + env_prelude = get_env_prelude(ctx, out_dir.path, []) cc_toolchain = find_cpp_toolchain(ctx) - script = env_prelude + [ + script = [ "##script_prelude##", - "export EXT_BUILD_ROOT=##pwd##", - "export INSTALLDIR=$$EXT_BUILD_ROOT$$/{}".format(out_dir.path), - "export BUILD_TMPDIR=$$INSTALLDIR$$.build_tmpdir", + ] + env_prelude + [ "##rm_rf## $$INSTALLDIR$$", "##rm_rf## $$BUILD_TMPDIR$$", "##mkdirs## $$INSTALLDIR$$", @@ -97,7 +95,13 @@ def built_tool_rule_impl(ctx, script_lines, out_dir, mnemonic, additional_tools "", ]) - wrapped_outputs = wrap_outputs(ctx, lib_name, mnemonic, script_text) + wrapped_outputs = wrap_outputs( + ctx, + lib_name = lib_name, + configure_name = mnemonic, + env_prelude = env_prelude, + script_text = script_text, + ) tools = depset( [wrapped_outputs.wrapper_script_file, wrapped_outputs.script_file], diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl index 1bb947b6..33129b8f 100644 --- a/foreign_cc/private/framework.bzl +++ b/foreign_cc/private/framework.bzl @@ -298,14 +298,13 @@ dependencies.""", def _is_msvc_var(var): return var == "INCLUDE" or var == "LIB" -def get_env_prelude(ctx, lib_name, data_dependencies, target_root): +def get_env_prelude(ctx, installdir, data_dependencies): """Generate a bash snippet containing environment variable definitions Args: ctx (ctx): The rule's context object - lib_name (str): The name of the target being built + installdir (str): The path from the root target's directory in the build output data_dependencies (list): A list of targets representing dependencies - target_root (str): The path from the root target's directory in the build output Returns: tuple: A list of environment variables to define in the build script and a dict @@ -313,7 +312,7 @@ def get_env_prelude(ctx, lib_name, data_dependencies, target_root): """ env_snippet = [ "export EXT_BUILD_ROOT=##pwd##", - "export INSTALLDIR=$$EXT_BUILD_ROOT$$/" + target_root + "/" + lib_name, + "export INSTALLDIR=$$EXT_BUILD_ROOT$$/" + installdir, "export BUILD_TMPDIR=$$INSTALLDIR$$.build_tmpdir", "export EXT_BUILD_DEPS=$$INSTALLDIR$$.ext_build_deps", ] @@ -447,7 +446,8 @@ def cc_external_rule_impl(ctx, attrs): # Also add legacy dependencies while they're still available data_dependencies += ctx.attr.tools_deps + ctx.attr.additional_tools - env_prelude = get_env_prelude(ctx, lib_name, data_dependencies, target_root) + installdir = target_root + "/" + lib_name + env_prelude = get_env_prelude(ctx, installdir, data_dependencies) postfix_script = [attrs.postfix_script] if not attrs.postfix_script: @@ -491,7 +491,13 @@ def cc_external_rule_impl(ctx, attrs): convert_shell_script(ctx, script_lines), "", ]) - wrapped_outputs = wrap_outputs(ctx, lib_name, attrs.configure_name, script_text) + wrapped_outputs = wrap_outputs( + ctx, + lib_name = lib_name, + configure_name = attrs.configure_name, + script_text = script_text, + env_prelude = env_prelude, + ) rule_outputs = outputs.declared_outputs + [installdir_copy.file] cc_toolchain = find_cpp_toolchain(ctx) @@ -595,7 +601,7 @@ WrappedOutputs = provider( ) # buildifier: disable=function-docstring -def wrap_outputs(ctx, lib_name, configure_name, script_text, build_script_file = None): +def wrap_outputs(ctx, lib_name, configure_name, script_text, env_prelude, build_script_file = None): extension = script_extension(ctx) build_log_file = ctx.actions.declare_file("{}_foreign_cc/{}.log".format(lib_name, configure_name)) build_script_file = ctx.actions.declare_file("{}_foreign_cc/build_script{}".format(lib_name, extension)) @@ -610,15 +616,13 @@ def wrap_outputs(ctx, lib_name, configure_name, script_text, build_script_file = cleanup_on_success_function = create_function( ctx, "cleanup_on_success", - "rm -rf $BUILD_TMPDIR $EXT_BUILD_DEPS", + "rm -rf $$BUILD_TMPDIR$$ $$EXT_BUILD_DEPS$$", ) cleanup_on_failure_function = create_function( ctx, "cleanup_on_failure", "\n".join([ "##echo## \"rules_foreign_cc: Build failed!\"", - "##echo## \"rules_foreign_cc: Keeping temp build directory $$BUILD_TMPDIR$$ and dependencies directory $$EXT_BUILD_DEPS$$ for debug.\"", - "##echo## \"rules_foreign_cc: Please note that the directories inside a sandbox are still cleaned unless you specify '--sandbox_debug' Bazel command line flag.\"", "##echo## \"rules_foreign_cc: Printing build logs:\"", "##echo## \"_____ BEGIN BUILD LOGS _____\"", "##cat## $$BUILD_LOG$$", @@ -626,18 +630,24 @@ def wrap_outputs(ctx, lib_name, configure_name, script_text, build_script_file = "##echo## \"rules_foreign_cc: Build wrapper script location: $$BUILD_WRAPPER_SCRIPT$$\"", "##echo## \"rules_foreign_cc: Build script location: $$BUILD_SCRIPT$$\"", "##echo## \"rules_foreign_cc: Build log location: $$BUILD_LOG$$\"", + "##echo## \"rules_foreign_cc: Keeping these below directories for debug, but note that the directories inside a sandbox\"", + "##echo## \"rules_foreign_cc: are still cleaned unless you specify the '--sandbox_debug' Bazel command line flag.\"", + "##echo## \"rules_foreign_cc: Build Dir: $$BUILD_TMPDIR$$\"", + "##echo## \"rules_foreign_cc: Deps Dir: $$EXT_BUILD_DEPS$$\"", "##echo## \"\"", ]), ) trap_function = "##cleanup_function## cleanup_on_success cleanup_on_failure" build_command_lines = [ + "##script_prelude##", "##assert_script_errors##", cleanup_on_success_function, cleanup_on_failure_function, # the call trap is defined inside, in a way how the shell function should be called # see, for instance, linux_commands.bzl trap_function, + ] + env_prelude + [ "export BUILD_WRAPPER_SCRIPT=\"{}\"".format(wrapper_script_file.path), "export BUILD_SCRIPT=\"{}\"".format(build_script_file.path), "export BUILD_LOG=\"{}\"".format(build_log_file.path),