Skip to content

Commit

Permalink
Removed make_commands attribute and fixed configure_make
Browse files Browse the repository at this point in the history
  • Loading branch information
UebelAndre committed Jun 15, 2021
1 parent e1f6efe commit e832f6e
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 63 deletions.
5 changes: 3 additions & 2 deletions foreign_cc/boost_build.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ def _boost_build_impl(ctx):
ctx.attr,
configure_name = "BoostBuild",
create_configure_script = _create_configure_script,
make_commands = ["./b2 install {} --prefix=.".format(" ".join(ctx.attr.user_options))],
)
return cc_external_rule_impl(ctx, attrs)

Expand All @@ -26,13 +25,15 @@ def _create_configure_script(configureParameters):
"cd $INSTALLDIR",
"##copy_dir_contents_to_dir## $$EXT_BUILD_ROOT$$/{}/. .".format(root),
"chmod -R +w .",
"##enable_tracing##",
"./bootstrap.sh {}".format(" ".join(ctx.attr.bootstrap_options)),
"./b2 install {} --prefix=.".format(" ".join(ctx.attr.user_options)),
"##disable_tracing##",
]

def _attrs():
attrs = dict(CC_EXTERNAL_RULE_ATTRIBUTES)
attrs.pop("targets")
attrs.pop("make_commands")
attrs.update({
"bootstrap_options": attr.string_list(
doc = "any additional flags to pass to bootstrap.sh",
Expand Down
1 change: 0 additions & 1 deletion foreign_cc/cmake.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ def _get_generator_target(ctx):

def _attrs():
attrs = dict(CC_EXTERNAL_RULE_ATTRIBUTES)
attrs.pop("make_commands")
attrs.update({
"build_args": attr.string_list(
doc = "Arguments for the CMake build command",
Expand Down
20 changes: 9 additions & 11 deletions foreign_cc/configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def _create_configure_script(configureParameters):
attrs = configureParameters.attrs
inputs = configureParameters.inputs

root = detect_root(ctx.attr.lib_source)
install_prefix = _get_install_prefix(ctx)

tools = get_tools_info(ctx)
Expand All @@ -77,23 +76,22 @@ def _create_configure_script(configureParameters):
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("{prefix}{make} -C $$EXT_BUILD_ROOT$$/{root} {target} {args}".format(
prefix = prefix,
make = attrs.make_path,
root = root,
args = args,
target = target,
))
for target in ctx.attr.targets:
# Configure will have generated sources into `$BUILD_TMPDIR` so make sure we `cd` there
make_commands.append("{prefix}{make} -C $$BUILD_TMPDIR$$ {target} {args}".format(
prefix = prefix,
make = attrs.make_path,
args = args,
target = target,
))

configure = create_configure_script(
workspace_name = ctx.workspace_name,
# as default, pass execution OS as target OS
target_os = os_name(ctx),
tools = tools,
flags = flags,
root = root,
root = detect_root(ctx.attr.lib_source),
user_options = ctx.attr.configure_options,
user_vars = dict(ctx.attr.configure_env_vars),
is_debug = is_debug_mode(ctx),
Expand Down
1 change: 0 additions & 1 deletion foreign_cc/make.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def _create_make_script(configureParameters):

def _attrs():
attrs = dict(CC_EXTERNAL_RULE_ATTRIBUTES)
attrs.pop("make_commands")
attrs.update({
"args": attr.string_list(
doc = "A list of arguments to pass to the call to `make`",
Expand Down
4 changes: 0 additions & 4 deletions foreign_cc/ninja.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ def _ninja_impl(ctx):
create_configure_script = _create_ninja_script,
tools_deps = tools_deps,
ninja_path = ninja_data.path,
make_commands = [],
)
return cc_external_rule_impl(ctx, attrs)

Expand Down Expand Up @@ -91,9 +90,6 @@ def _attrs():
"""
attrs = dict(CC_EXTERNAL_RULE_ATTRIBUTES)

# Drop old vars
attrs.pop("make_commands")

attrs.update({
"args": attr.string_list(
doc = "A list of arguments to pass to the call to `ninja`",
Expand Down
48 changes: 4 additions & 44 deletions foreign_cc/private/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ load(
"shebang",
)
load("//foreign_cc/private/framework:platform.bzl", "os_name")
load(
"//toolchains/native_tools:tool_access.bzl",
"get_make_data",
"get_ninja_data",
)
load(
":cc_toolchain_util.bzl",
"LibrariesToLinkInfo",
Expand Down Expand Up @@ -128,11 +123,6 @@ CC_EXTERNAL_RULE_ATTRIBUTES = {
mandatory = False,
default = [],
),
"make_commands": attr.string_list(
doc = "Optional make commands.",
mandatory = False,
default = ["make", "make install"],
),
"out_bin_dir": attr.string(
doc = "Optional name of the output subdirectory with the binary files, defaults to 'bin'.",
mandatory = False,
Expand Down Expand Up @@ -336,9 +326,8 @@ def cc_external_rule_impl(ctx, attrs):
These variables should be used by the calling rule to refer to the created directory structure.
4. calls 'attrs.create_configure_script'
5. calls 'attrs.make_commands'
6. calls 'attrs.postfix_script'
7. replaces absolute paths in possibly created scripts with a placeholder value
5. calls 'attrs.postfix_script'
6. replaces absolute paths in possibly created scripts with a placeholder value
Please see cmake.bzl for example usage.
Expand Down Expand Up @@ -381,8 +370,6 @@ def cc_external_rule_impl(ctx, attrs):

env_prelude, env = _env_prelude(ctx, lib_name, data_dependencies, target_root)

make_commands, build_tools = _generate_make_commands(ctx, attrs)

postfix_script = [attrs.postfix_script]
if not attrs.postfix_script:
postfix_script = []
Expand All @@ -399,7 +386,7 @@ def cc_external_rule_impl(ctx, attrs):
"##mkdirs## $$EXT_BUILD_DEPS$$",
] + _print_env() + _copy_deps_and_tools(inputs) + [
"cd $$BUILD_TMPDIR$$",
] + attrs.create_configure_script(ConfigureParameters(ctx = ctx, attrs = attrs, inputs = inputs)) + make_commands + postfix_script + [
] + attrs.create_configure_script(ConfigureParameters(ctx = ctx, attrs = attrs, inputs = inputs)) + postfix_script + [
# replace references to the root directory when building ($BUILD_TMPDIR)
# and the root where the dependencies were installed ($EXT_BUILD_DEPS)
# for the results which are in $INSTALLDIR (with placeholder)
Expand Down Expand Up @@ -442,7 +429,7 @@ def cc_external_rule_impl(ctx, attrs):
outputs = rule_outputs + [wrapped_outputs.log_file],
tools = depset(
[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,
transitive = [cc_toolchain.all_files] + [data[DefaultInfo].default_runfiles.files for data in data_dependencies],
),
command = wrapped_outputs.wrapper_script_file.path,
execution_requirements = execution_requirements,
Expand Down Expand Up @@ -884,35 +871,8 @@ def _collect_libs(cc_linking):
libs.append(library)
return collections.uniq(libs)

def _generate_make_commands(ctx, attrs):
make_commands = getattr(attrs, "make_commands", [])
tools_deps = []

# Early out if there are no commands set
if not make_commands:
return make_commands, tools_deps

if _uses_tool(attrs.make_commands, "make"):
make_data = get_make_data(ctx)
tools_deps += make_data.deps
make_commands = [_expand_command_path("make", make_data.path, command) for command in make_commands]

if _uses_tool(attrs.make_commands, "ninja"):
ninja_data = get_ninja_data(ctx)
tools_deps += ninja_data.deps
make_commands = [_expand_command_path("ninja", ninja_data.path, command) for command in make_commands]

return make_commands, [tool.files for tool in tools_deps]

def _expand_command_path(binary, path, command):
if command == binary or command.startswith(binary + " "):
return command.replace(binary, path, 1)
else:
return command

def _uses_tool(make_commands, tool):
for command in make_commands:
(before, separator, after) = command.partition(" ")
if before == tool:
return True
return False

0 comments on commit e832f6e

Please sign in to comment.