Skip to content

Commit

Permalink
fix undefined variables in the wrap_outputs script
Browse files Browse the repository at this point in the history
BUILD_TMPDIR and EXT_BUILD_DEPS were used in the wrapper script, but
they weren't actually defined. This wasn't caught because the script
prelude (which sets bash -u) wasn't used either.

This fix:
- Adds the missing script prelude to the wrapper script
- Generalizes get_env_prelude slightly so built_tools can use it
  directly, without needing to override variables
- Passes the env prelude to wrapped_outputs so it can set those same
  variables
- Prints out the paths to the build and deps dir at the end of a failed
  script, to make it easier to debug failed builds.
  • Loading branch information
novas0x2a committed Sep 6, 2024
1 parent b25485b commit 9dd0b84
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 16 deletions.
16 changes: 10 additions & 6 deletions foreign_cc/built_tools/private/built_tools_framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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$$",
Expand All @@ -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],
Expand Down
30 changes: 20 additions & 10 deletions foreign_cc/private/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -290,22 +290,21 @@ 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
of environment variables
"""
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",
]
Expand Down Expand Up @@ -439,7 +438,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:
Expand Down Expand Up @@ -483,7 +483,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)
Expand Down Expand Up @@ -587,7 +593,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))
Expand All @@ -602,34 +608,38 @@ 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$$",
"##echo## \"_____ END BUILD LOGS _____\"",
"##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),
Expand Down

0 comments on commit 9dd0b84

Please sign in to comment.