diff --git a/examples/make_simple/BUILD.bazel b/examples/make_simple/BUILD.bazel index 47d40770..ce8d19f7 100644 --- a/examples/make_simple/BUILD.bazel +++ b/examples/make_simple/BUILD.bazel @@ -3,13 +3,13 @@ load("@rules_foreign_cc//foreign_cc:defs.bzl", "make") make( name = "make_lib", + build_data = ["//make_simple/code:clang_wrapper.sh"], env = { "CLANG_WRAPPER": "$(execpath //make_simple/code:clang_wrapper.sh)", "PREFIX": "$$INSTALLDIR$$", }, lib_source = "//make_simple/code:srcs", out_static_libs = ["liba.a"], - tools_deps = ["//make_simple/code:clang_wrapper.sh"], ) cc_test( diff --git a/foreign_cc/cmake.bzl b/foreign_cc/cmake.bzl index da1c4b42..965675c8 100644 --- a/foreign_cc/cmake.bzl +++ b/foreign_cc/cmake.bzl @@ -158,7 +158,11 @@ load( def _cmake_impl(ctx): cmake_data = get_cmake_data(ctx) - tools_deps = ctx.attr.tools_deps + cmake_data.deps + tools_deps = cmake_data.deps + + # TODO: `tool_deps` is deprecated. Remove + tools_deps += ctx.attr.tools_deps + env = dict(ctx.attr.env) generator, generate_args = _get_generator_target(ctx) @@ -201,15 +205,21 @@ def _create_configure_script(configureParameters): cmake_commands = [] - data = ctx.attr.data + getattr(ctx.attr, "tools_deps", []) configuration = "Debug" if is_debug_mode(ctx) else "Release" + data = ctx.attr.data + ctx.attr.build_data + + # TODO: The following attributes are deprecated. Remove + data += ctx.attr.tools_deps + # Generate a list of arguments for cmake's build command build_args = " ".join([ ctx.expand_location(arg, data) for arg in ctx.attr.build_args ]) + prefix = "{} ".format(ctx.expand_location(attrs.tool_prefix, data)) if attrs.tool_prefix else "" + # Generate commands for all the targets, ensuring there's # always at least 1 call to the default target. for target in ctx.attr.targets or [""]: @@ -219,7 +229,8 @@ def _create_configure_script(configureParameters): # Note that even though directory is always passed, the # following arguments can take precedence. - cmake_commands.append("{cmake} --build {dir} --config {config} {target} {args}".format( + cmake_commands.append("{prefix}{cmake} --build {dir} --config {config} {target} {args}".format( + prefix = prefix, cmake = attrs.cmake_path, dir = ".", args = build_args, @@ -234,7 +245,8 @@ def _create_configure_script(configureParameters): for arg in ctx.attr.install_args ]) - cmake_commands.append("{cmake} --install {dir} --config {config} {args}".format( + cmake_commands.append("{prefix}{cmake} --install {dir} --config {config} {args}".format( + prefix = prefix, cmake = attrs.cmake_path, dir = ".", args = install_args, diff --git a/foreign_cc/configure.bzl b/foreign_cc/configure.bzl index 5324d68c..c9ca72b7 100644 --- a/foreign_cc/configure.bzl +++ b/foreign_cc/configure.bzl @@ -74,10 +74,13 @@ def _create_configure_script(configureParameters): ]) make_commands = [] + prefix = "{} ".format(ctx.expand_location(attrs.tool_prefix, data)) if attrs.tool_prefix else "" + configure_prefix = "{} ".format(ctx.expand_location(ctx.attr.configure_prefix, data)) if ctx.attr.configure_prefix else "" if not ctx.attr.make_commands: for target in ctx.attr.targets: - make_commands.append("{make} -C $$EXT_BUILD_ROOT$$/{root} {target} {args}".format( + make_commands.append("{prefix}{make} -C $$EXT_BUILD_ROOT$$/{root} {target} {args}".format( + prefix = prefix, make = attrs.make_path, root = root, args = args, @@ -94,6 +97,7 @@ def _create_configure_script(configureParameters): user_options = ctx.attr.configure_options, user_vars = dict(ctx.attr.configure_env_vars), is_debug = is_debug_mode(ctx), + configure_prefix = configure_prefix, configure_command = ctx.attr.configure_command, deps = ctx.attr.deps, inputs = inputs, @@ -197,6 +201,9 @@ def _attrs(): "configure_options": attr.string_list( doc = "Any options to be put on the 'configure' command line.", ), + "configure_prefix": attr.string( + doc = "A prefix for the call to the `configure_command`.", + ), "install_prefix": attr.string( doc = ( "Install prefix, i.e. relative path to where to install the result of the build. " + diff --git a/foreign_cc/make.bzl b/foreign_cc/make.bzl index be86512e..09196b25 100644 --- a/foreign_cc/make.bzl +++ b/foreign_cc/make.bzl @@ -43,7 +43,7 @@ def _create_make_script(configureParameters): tools = get_tools_info(ctx) flags = get_flags_info(ctx) - data = ctx.attr.data or list() + data = ctx.attr.data + ctx.attr.build_data # Generate a list of arguments for make args = " ".join([ @@ -52,8 +52,10 @@ def _create_make_script(configureParameters): ]) make_commands = [] + prefix = "{} ".format(ctx.expand_location(attrs.tool_prefix, data)) if attrs.tool_prefix else "" for target in ctx.attr.targets: - make_commands.append("{make} -C $$EXT_BUILD_ROOT$$/{root} {target} {args}".format( + make_commands.append("{prefix}{make} -C $$EXT_BUILD_ROOT$$/{root} {target} {args}".format( + prefix = prefix, make = attrs.make_path, root = root, args = args, diff --git a/foreign_cc/ninja.bzl b/foreign_cc/ninja.bzl index 328d0d29..ef6ac463 100644 --- a/foreign_cc/ninja.bzl +++ b/foreign_cc/ninja.bzl @@ -46,13 +46,14 @@ def _create_ninja_script(configureParameters): str: A string representing a section of a bash script """ ctx = configureParameters.ctx + attrs = configureParameters.attrs script = [] root = detect_root(ctx.attr.lib_source) script.append("##symlink_contents_to_dir## $$EXT_BUILD_ROOT$$/{} $$BUILD_TMPDIR$$".format(root)) - data = ctx.attr.data or list() + data = ctx.attr.data + ctx.attr.build_data # Generate a list of arguments for ninja args = " ".join([ @@ -65,13 +66,16 @@ def _create_ninja_script(configureParameters): if ctx.attr.directory: directory = ctx.expand_location(ctx.attr.directory, data) + prefix = "{} ".format(ctx.expand_location(attrs.tool_prefix, data)) if attrs.tool_prefix else "" + # Generate commands for all the targets, ensuring there's # always at least 1 call to the default target. for target in ctx.attr.targets or [""]: # Note that even though directory is always passed, the # following arguments can take precedence. - script.append("{ninja} -C {dir} {args} {target}".format( - ninja = configureParameters.attrs.ninja_path, + script.append("{prefix}{ninja} -C {dir} {args} {target}".format( + prefix = prefix, + ninja = attrs.ninja_path, dir = directory, args = args, target = target, diff --git a/foreign_cc/private/configure_script.bzl b/foreign_cc/private/configure_script.bzl index 07fdd707..d56b63d8 100644 --- a/foreign_cc/private/configure_script.bzl +++ b/foreign_cc/private/configure_script.bzl @@ -13,6 +13,7 @@ def create_configure_script( user_options, user_vars, is_debug, + configure_prefix, configure_command, deps, inputs, @@ -67,8 +68,9 @@ def create_configure_script( " ".join(autoreconf_options), ).lstrip()) - script.append('{env_vars} "{configure}" --prefix=$$BUILD_TMPDIR$$/$$INSTALL_PREFIX$$ {user_options}'.format( + script.append('{env_vars} {prefix}"{configure}" --prefix=$$BUILD_TMPDIR$$/$$INSTALL_PREFIX$$ {user_options}'.format( env_vars = env_vars_string, + prefix = configure_prefix, configure = configure_path, user_options = " ".join(user_options), )) diff --git a/foreign_cc/private/framework.bzl b/foreign_cc/private/framework.bzl index 6d548a64..d0e2e647 100644 --- a/foreign_cc/private/framework.bzl +++ b/foreign_cc/private/framework.bzl @@ -41,8 +41,7 @@ load( CC_EXTERNAL_RULE_ATTRIBUTES = { "additional_inputs": attr.label_list( doc = ( - "Optional additional inputs to be declared as needed for the shell script action." + - "Not used by the shell script part in cc_external_rule_impl." + "__deprecated__: Please use the `build_data` attribute." ), mandatory = False, allow_files = True, @@ -50,8 +49,7 @@ CC_EXTERNAL_RULE_ATTRIBUTES = { ), "additional_tools": attr.label_list( doc = ( - "Optional additional tools needed for the building. " + - "Not used by the shell script part in cc_external_rule_impl." + "__deprecated__: Please use the `build_data` attribute." ), mandatory = False, allow_files = True, @@ -66,10 +64,18 @@ CC_EXTERNAL_RULE_ATTRIBUTES = { mandatory = False, default = False, ), + "build_data": attr.label_list( + doc = "Files needed by this rule only during build/compile time. May list file or rule targets. Generally allows any target.", + mandatory = False, + allow_files = True, + cfg = "exec", + default = [], + ), "data": attr.label_list( doc = "Files needed by this rule at runtime. May list file or rule targets. Generally allows any target.", mandatory = False, allow_files = True, + cfg = "target", default = [], ), "defines": attr.string_list( @@ -94,7 +100,7 @@ CC_EXTERNAL_RULE_ATTRIBUTES = { "env": attr.string_dict( doc = ( "Environment variables to set during the build. " + - "`$(execpath)` macros may be used to point at files which are listed as data deps, tools_deps, or additional_tools, " + + "`$(execpath)` macros may be used to point at files which are listed as `data`, `deps`, or `build_data`, " + "but unlike with other rules, these will be replaced with absolute paths to those files, " + "because the build does not run in the exec root. " + "No other macros are supported." @@ -178,11 +184,12 @@ CC_EXTERNAL_RULE_ATTRIBUTES = { ), mandatory = False, ), + "tool_prefix": attr.string( + doc = "A prefix for build commands", + mandatory = False, + ), "tools_deps": attr.label_list( - doc = ( - "Optional tools to be copied into the directory structure. " + - "Similar to deps, those directly required for the external building of the library/binaries." - ), + doc = "__deprecated__: Please use the `build_data` attribute.", mandatory = False, allow_files = True, cfg = "exec", @@ -204,6 +211,17 @@ CC_EXTERNAL_RULE_FRAGMENTS = [ "cpp", ] +# buildifier: disable=print +def _print_deprecation_warnings(ctx): + if ctx.attr.tools_deps: + print(ctx.label, "Attribute `tools_deps` is deprecated. Please use `build_data`.") + + if ctx.attr.additional_inputs: + print(ctx.label, "Attribute `additional_inputs` is deprecated. Please use `build_data`.") + + if ctx.attr.additional_tools: + print(ctx.label, "Attribute `additional_tools` is deprecated. Please use `build_data`.") + # buildifier: disable=function-docstring-header # buildifier: disable=function-docstring-args # buildifier: disable=function-docstring-return @@ -337,6 +355,7 @@ def cc_external_rule_impl(ctx, attrs): Returns: A list of providers """ + _print_deprecation_warnings(ctx) lib_name = attrs.lib_name or ctx.attr.name inputs = _define_inputs(attrs) @@ -355,7 +374,10 @@ def cc_external_rule_impl(ctx, attrs): installdir_copy = copy_directory(ctx.actions, "$$INSTALLDIR$$", "copy_{}/{}".format(lib_name, lib_name)) target_root = paths.dirname(installdir_copy.file.dirname) - data_dependencies = ctx.attr.data + ctx.attr.tools_deps + ctx.attr.additional_tools + data_dependencies = ctx.attr.data + ctx.attr.build_data + + # Also add legacy dependencies while they're still available + data_dependencies += ctx.attr.tools_deps + ctx.attr.additional_tools env_prelude, env = _env_prelude(ctx, lib_name, data_dependencies, target_root) @@ -408,6 +430,9 @@ def cc_external_rule_impl(ctx, attrs): if "requires-network" in ctx.attr.tags: execution_requirements = {"requires-network": ""} + # TODO: `additional_tools` is deprecated, remove. + legacy_tools = ctx.files.additional_tools + ctx.files.tools_deps + # The use of `run_shell` here is intended to ensure bash is correctly setup on windows # environments. This should not be replaced with `run` until a cross platform implementation # is found that guarantees bash exists or appropriately errors out. @@ -416,7 +441,7 @@ def cc_external_rule_impl(ctx, attrs): inputs = depset(inputs.declared_inputs), outputs = rule_outputs + [wrapped_outputs.log_file], tools = depset( - [wrapped_outputs.script_file, wrapped_outputs.wrapper_script_file] + ctx.files.data + ctx.files.tools_deps + ctx.files.additional_tools, + [wrapped_outputs.script_file, wrapped_outputs.wrapper_script_file] + ctx.files.data + ctx.files.build_data + legacy_tools, transitive = [cc_toolchain.all_files] + [data[DefaultInfo].default_runfiles.files for data in data_dependencies] + build_tools, ), command = wrapped_outputs.wrapper_script_file.path, @@ -432,9 +457,13 @@ def cc_external_rule_impl(ctx, attrs): # Gather runfiles transitively as per the documentation in: # https://docs.bazel.build/versions/master/skylark/rules.html#runfiles runfiles = ctx.runfiles(files = ctx.files.data) - for target in [ctx.attr.lib_source] + ctx.attr.additional_inputs + ctx.attr.deps + ctx.attr.data: + for target in [ctx.attr.lib_source] + ctx.attr.deps + ctx.attr.data: runfiles = runfiles.merge(target[DefaultInfo].default_runfiles) + # TODO: `additional_inputs` is deprecated, remove. + for legacy in ctx.attr.additional_inputs: + runfiles = runfiles.merge(legacy[DefaultInfo].default_runfiles) + externally_built = ForeignCcArtifactInfo( gen_dir = installdir_copy.file, bin_dir_name = attrs.out_bin_dir, @@ -752,10 +781,12 @@ def _define_inputs(attrs): for file_list in tool.files.to_list(): tools_files += _list(file_list) + # TODO: Remove, `additional_tools` is deprecated. for tool in attrs.additional_tools: for file_list in tool.files.to_list(): tools_files += _list(file_list) + # TODO: Remove, `additional_inputs` is deprecated. for input in attrs.additional_inputs: for file_list in input.files.to_list(): input_files += _list(file_list)