Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand arg locations #809

Merged
merged 31 commits into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
74e3b8a
Expand locations in rustc_flags.
sayrer Jun 27, 2021
20eb0fb
Merge branch 'expand_arg_locations' of https://github.com/grafica/rul…
sayrer Jun 27, 2021
1625b23
Address comments.
sayrer Jun 28, 2021
8993cb3
Fix buildifier.
sayrer Jun 28, 2021
ae38ad9
Merge branch 'main' into expand_arg_locations
sayrer Jun 28, 2021
cd0077e
Rename expand_location to expand_env_locations.
sayrer Jun 28, 2021
28b80f9
Merge branch 'expand_arg_locations' of https://github.com/grafica/rul…
sayrer Jun 28, 2021
bd00e63
Separate test.
sayrer Jun 28, 2021
f34f71f
Whitespace.
sayrer Jun 28, 2021
16418c0
Try to fix Clippy warning.
sayrer Jun 28, 2021
c92bbb8
Rewrite to satisfy clippy.
sayrer Jun 28, 2021
0ce9dad
Remove clippy annotation.
sayrer Jun 28, 2021
eddbd6f
Remove unnecessary comment.
sayrer Jun 29, 2021
7bf37a5
Rename location expansion functions.
sayrer Jun 29, 2021
1405620
Add a brief note about expansions.
sayrer Jun 30, 2021
f0da18d
Fix import order.
sayrer Jun 30, 2021
e3b17e8
Document location expansion.
sayrer Jun 30, 2021
5c1cd61
Regenerate documentation
sayrer Jun 30, 2021
cee8af9
Unit test for location expansion.
sayrer Jun 30, 2021
d9f2db3
Make this file pass rustfmt.
sayrer Jun 30, 2021
609a8bc
Whitespace.
sayrer Jun 30, 2021
b3baa9c
Merge branch 'main' into expand_arg_locations
sayrer Jun 30, 2021
ab82e9c
Merge branch 'main' into expand_arg_locations
sayrer Jul 1, 2021
9e6bb39
Address review comments.
sayrer Jul 1, 2021
a0666c5
Merge branch 'expand_arg_locations' of https://github.com/grafica/rul…
sayrer Jul 1, 2021
afee0a2
Merge branch 'main' into expand_arg_locations
sayrer Jul 1, 2021
5ad52d3
Regenerate documentation
sayrer Jul 1, 2021
93cd938
Merge branch 'expand_arg_locations' of https://github.com/grafica/rul…
sayrer Jul 1, 2021
53b981c
Add two empty lines.
sayrer Jul 1, 2021
c9e4cd5
A single newline
sayrer Jul 1, 2021
844d523
Merge branch 'main' into expand_arg_locations
sayrer Jul 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cargo/cargo_build_script.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ load("//rust:rust.bzl", "rust_binary")
load("//rust/private:rustc.bzl", "BuildInfo", "get_compilation_mode_opts", "get_linker_and_args")

# buildifier: disable=bzl-visibility
load("//rust/private:utils.bzl", "expand_locations", "find_cc_toolchain", "find_toolchain", "name_to_crate_name")
load("//rust/private:utils.bzl", "expand_dict_value_locations", "find_cc_toolchain", "find_toolchain", "name_to_crate_name")

def get_cc_compile_env(cc_toolchain, feature_configuration):
"""Gather cc environment variables from the given `cc_toolchain`
Expand Down Expand Up @@ -117,7 +117,7 @@ def _build_script_impl(ctx):
for f in ctx.attr.crate_features:
env["CARGO_FEATURE_" + f.upper().replace("-", "_")] = "1"

env.update(expand_locations(
env.update(expand_dict_value_locations(
ctx,
ctx.attr.build_script_env,
getattr(ctx.attr, "data", []) +
Expand Down
14 changes: 7 additions & 7 deletions docs/defs.md

Large diffs are not rendered by default.

14 changes: 7 additions & 7 deletions docs/flatten.md

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions examples/flag_locations/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
load(
"@rules_rust//rust:rust.bzl",
"rust_test",
)

# generate a file containing cfg flags
genrule(
hlopko marked this conversation as resolved.
Show resolved Hide resolved
name = "flag_generator",
outs = ["generated_flag.data"],
cmd = "echo --cfg=test_flag > $@",
)

rust_test(
name = "test",
srcs = [
"main.rs",
],
data = [":flag_generator"],
edition = "2018",
rustc_flags = [
"@$(location :flag_generator)",
],
)
9 changes: 9 additions & 0 deletions examples/flag_locations/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#[test]
fn test() {
// we should able to read rustc args from a generated file
sayrer marked this conversation as resolved.
Show resolved Hide resolved
if cfg!(test_flag) {
return;
}

unreachable!();
}
13 changes: 10 additions & 3 deletions rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ load(
"crate_name_from_attr",
"dedent",
"determine_output_hash",
"expand_locations",
"expand_dict_value_locations",
"find_toolchain",
)

Expand Down Expand Up @@ -340,7 +340,7 @@ def _create_test_launcher(ctx, toolchain, output, providers):

# Expand the environment variables and write them to a file
environ_file = ctx.actions.declare_file(launcher_filename + ".launchfiles/env")
environ = expand_locations(
environ = expand_dict_value_locations(
ctx,
getattr(ctx.attr, "env", {}),
data,
Expand Down Expand Up @@ -620,7 +620,14 @@ _common_attrs = {
"""),
),
"rustc_flags": attr.string_list(
doc = "List of compiler flags passed to `rustc`.",
doc = dedent("""\
sayrer marked this conversation as resolved.
Show resolved Hide resolved
List of compiler flags passed to `rustc`.

These strings are subject to Make variable expansion for predefined
source/output path variables like `$location`, `$execpath`, and `$rootpath`.
This expansion is useful if you wish to pass a generated file of
arguments to rustc: `@$(location //:package:target)`.
sayrer marked this conversation as resolved.
Show resolved Hide resolved
"""),
),
# TODO(stardoc): How do we provide additional documentation to an inherited attribute?
# "name": attr.string(
Expand Down
19 changes: 13 additions & 6 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ load("//rust/private:common.bzl", "rust_common")
load(
"//rust/private:utils.bzl",
"crate_name_from_attr",
"expand_locations",
"expand_dict_value_locations",
"expand_list_element_locations",
"find_cc_toolchain",
"get_lib_name",
"get_preferred_artifact",
Expand Down Expand Up @@ -438,9 +439,16 @@ def construct_arguments(

# Tell Rustc where to find the standard library
args.add_all(rust_lib_paths, before_each = "-L", format_each = "%s")

args.add_all(rust_flags)
args.add_all(getattr(attr, "rustc_flags", []))

data_paths = getattr(attr, "data", []) + getattr(attr, "compile_data", [])
args.add_all(
expand_list_element_locations(
ctx,
getattr(attr, "rustc_flags", []),
data_paths,
),
)
add_edition_flags(args, crate_info)

# Link!
Expand Down Expand Up @@ -471,11 +479,10 @@ def construct_arguments(
env["CARGO_BIN_EXE_" + dep_crate_info.output.basename] = dep_crate_info.output.short_path

# Update environment with user provided variables.
env.update(expand_locations(
env.update(expand_dict_value_locations(
ctx,
crate_info.rustc_env,
getattr(attr, "data", []) +
getattr(attr, "compile_data", []),
data_paths,
))

# This empty value satisfies Clippy, which otherwise complains about the
Expand Down
23 changes: 22 additions & 1 deletion rust/private/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def _expand_location(ctx, env, data):
env = env.replace(directive, "${pwd}/" + directive)
return ctx.expand_location(env, data)

def expand_locations(ctx, env, data):
def expand_dict_value_locations(ctx, env, data):
"""Performs location-macro expansion on string values.

$(execroot ...) and $(location ...) are prefixed with ${pwd},
Expand All @@ -179,6 +179,27 @@ def expand_locations(ctx, env, data):
"""
return dict([(k, _expand_location(ctx, v, data)) for (k, v) in env.items()])

def expand_list_element_locations(ctx, args, data):
"""Performs location-macro expansion on a list of string values.

$(execroot ...) and $(location ...) are prefixed with ${pwd},
which process_wrapper and build_script_runner will expand at run time
to the absolute path.

See `expand_locations` for detailed documentation.
sayrer marked this conversation as resolved.
Show resolved Hide resolved

sayrer marked this conversation as resolved.
Show resolved Hide resolved
Args:
ctx (ctx): The rule's context object
args (list): A list we iterate over
data (sequence of Targets): The targets which may be referenced by
location macros. This is expected to be the `data` attribute of
the target, though may have other targets or attributes mixed in.

Returns:
list: A list of arguments with expanded location macros
"""
return [_expand_location(ctx, arg, data) for arg in args]

def name_to_crate_name(name):
"""Converts a build target's name into the name of its associated crate.

Expand Down
4 changes: 4 additions & 0 deletions test/unit/location_expansion/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
load(":location_expansion_test.bzl", "location_expansion_test_suite")

############################ UNIT TESTS #############################
location_expansion_test_suite(name = "location_expansion_test_suite")
54 changes: 54 additions & 0 deletions test/unit/location_expansion/location_expansion_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""Unittest to verify location expansion in rustc flags"""

load("@bazel_skylib//lib:unittest.bzl", "analysistest")
load("//rust:defs.bzl", "rust_library")
load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_argv_contains")

def _location_expansion_rustc_flags_test(ctx):
env = analysistest.begin(ctx)
tut = analysistest.target_under_test(env)
action = tut.actions[0]
argv = action.argv
assert_action_mnemonic(env, action, "Rustc")
assert_argv_contains(env, action, "test/unit/location_expansion/mylibrary.rs")
expected = "@${pwd}/" + ctx.bin_dir.path + "/test/unit/location_expansion/generated_flag.data"
assert_argv_contains(env, action, expected)
return analysistest.end(env)

location_expansion_rustc_flags_test = analysistest.make(_location_expansion_rustc_flags_test)

def _location_expansion_test():
native.genrule(
name = "flag_generator",
outs = ["generated_flag.data"],
cmd = "echo --cfg=test_flag > $@",
)

rust_library(
name = "mylibrary",
srcs = ["mylibrary.rs"],
rustc_flags = [
"@$(location :flag_generator)",
sayrer marked this conversation as resolved.
Show resolved Hide resolved
],
compile_data = [":flag_generator"],
)

location_expansion_rustc_flags_test(
name = "location_expansion_rustc_flags_test",
target_under_test = ":mylibrary",
)

def location_expansion_test_suite(name):
"""Entry-point macro called from the BUILD file.
Args:
name: Name of the macro.
"""
_location_expansion_test()

native.test_suite(
name = name,
tests = [
":location_expansion_rustc_flags_test",
],
)
3 changes: 3 additions & 0 deletions test/unit/location_expansion/mylibrary.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn hello_world() {
sayrer marked this conversation as resolved.
Show resolved Hide resolved
println!("Hello, world.")
}
2 changes: 1 addition & 1 deletion util/launcher/launcher_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn environ() -> BTreeMap<String, String> {
let file = File::open(env_path).expect("Failed to load the environment file");

// Variables will have the `${pwd}` variable replaced which is rendered by
// `@rules_rust//rust/private:util.bzl::expand_locations`
// `@rules_rust//rust/private:util.bzl::expand_dict_value_locations`
let pwd = std::env::current_dir().expect("Failed to get current working directory");
let pwd_str = pwd.to_string_lossy();

Expand Down