From 1904b460edd7e2d5b9a4a17f87bcc67e70f17b68 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 13:11:18 +0200 Subject: [PATCH 01/11] allow rust_compile_action to force all dependencies to be treated as direct depdencies --- rust/private/rustc.bzl | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 2b8151dcd4..237555236c 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -433,7 +433,8 @@ def construct_arguments( out_dir, build_env_files, build_flags_files, - emit = ["dep-info", "link"]): + emit = ["dep-info", "link"], + force_only_direct_deps = False): """Builds an Args object containing common rustc flags Args: @@ -453,6 +454,8 @@ def construct_arguments( build_env_files (list): Files containing rustc environment variables, for instance from `cargo_build_script` actions. build_flags_files (list): The output files of a `cargo_build_script` actions containing rustc build flags emit (list): Values for the --emit flag to rustc. + force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + to the commandline as opposed to -L. Returns: tuple: A tuple of the following items @@ -583,7 +586,7 @@ def construct_arguments( _add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, crate_info.type, toolchain, cc_toolchain, feature_configuration) # These always need to be added, even if not linking this crate. - add_crate_link_flags(rustc_flags, dep_info) + add_crate_link_flags(rustc_flags, dep_info, force_only_direct_deps) needs_extern_proc_macro_flag = "proc-macro" in [crate_info.type, crate_info.wrapped_crate_type] and \ crate_info.edition != "2015" @@ -632,7 +635,8 @@ def rustc_compile_action( crate_info, output_hash = None, rust_flags = [], - environ = {}): + environ = {}, + force_only_direct_deps = False): """Create and run a rustc compile action based on the current rule's attributes Args: @@ -643,6 +647,8 @@ def rustc_compile_action( output_hash (str, optional): The hashed path of the crate root. Defaults to None. rust_flags (list, optional): Additional flags to pass to rustc. Defaults to []. environ (dict, optional): A set of makefile expandable environment variables for the action + force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + to the commandline as opposed to -L. Returns: list: A list of the following providers: @@ -690,6 +696,7 @@ def rustc_compile_action( out_dir = out_dir, build_env_files = build_env_files, build_flags_files = build_flags_files, + force_only_direct_deps = force_only_direct_deps, ) if hasattr(attr, "version") and attr.version != "0.0.0": @@ -942,22 +949,28 @@ def _get_dir_names(files): dirs[f.dirname] = None return dirs.keys() -def add_crate_link_flags(args, dep_info): +def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): """Adds link flags to an Args object reference Args: args (Args): An arguments object reference dep_info (DepInfo): The current target's dependency info + force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + to the commandline as opposed to -L. """ # nb. Crates are linked via --extern regardless of their crate_type args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag) - args.add_all( - dep_info.transitive_crates, - map_each = _get_crate_dirname, - uniquify = True, - format_each = "-Ldependency=%s", - ) + + if force_only_direct_deps: + args.add_all(dep_info.transitive_crates, uniquify = True, map_each = _crate_to_link_flag) + else: + args.add_all( + dep_info.transitive_crates, + map_each = _get_crate_dirname, + uniquify = True, + format_each = "-Ldependency=%s", + ) def _crate_to_link_flag(crate_info): """A helper macro used by `add_crate_link_flags` for adding crate link flags to a Arg object From 925110543d6574133f30d3eceef8e0043d786a43 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 14:31:18 +0200 Subject: [PATCH 02/11] Add a test --- rust/private/rustc.bzl | 33 ++++--- test/unit/force_only_direct_deps/BUILD.bazel | 4 + test/unit/force_only_direct_deps/direct.rs | 5 + .../force_only_direct_deps_test.bzl | 58 ++++++++++++ .../unit/force_only_direct_deps/generator.bzl | 91 +++++++++++++++++++ .../unit/force_only_direct_deps/transitive.rs | 1 + 6 files changed, 180 insertions(+), 12 deletions(-) create mode 100644 test/unit/force_only_direct_deps/BUILD.bazel create mode 100644 test/unit/force_only_direct_deps/direct.rs create mode 100644 test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl create mode 100644 test/unit/force_only_direct_deps/generator.bzl create mode 100644 test/unit/force_only_direct_deps/transitive.rs diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 237555236c..0fe98242b8 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -959,29 +959,38 @@ def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): to the commandline as opposed to -L. """ - # nb. Crates are linked via --extern regardless of their crate_type - args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag) - if force_only_direct_deps: - args.add_all(dep_info.transitive_crates, uniquify = True, map_each = _crate_to_link_flag) - else: - args.add_all( - dep_info.transitive_crates, - map_each = _get_crate_dirname, + args.add_all(depset( + transitive = [dep_info.direct_crates, + dep_info.transitive_crates] + ), uniquify = True, - format_each = "-Ldependency=%s", + map_each = _crate_to_link_flag, ) + else: + # nb. Crates are linked via --extern regardless of their crate_type + args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag) + args.add_all( + dep_info.transitive_crates, + map_each = _get_crate_dirname, + uniquify = True, + format_each = "-Ldependency=%s", + ) def _crate_to_link_flag(crate_info): """A helper macro used by `add_crate_link_flags` for adding crate link flags to a Arg object Args: - crate_info (CrateInfo): A CrateInfo provider from the current rule + crate_info (CrateInfo|AliasableDepInfo): A CrateInfo or a AliasableDepInfo provider Returns: - list: Link flags for the current crate info + list: Link flags for the given crate_info """ - return ["--extern={}={}".format(crate_info.name, crate_info.dep.output.path)] + + # This is AliasableDepInfo + if hasattr(crate_info, "dep"): + crate_info = crate_info.dep + return ["--extern={}={}".format(crate_info.name, crate_info.output.path)] def _get_crate_dirname(crate): """A helper macro used by `add_crate_link_flags` for getting the directory name of the current crate's output path diff --git a/test/unit/force_only_direct_deps/BUILD.bazel b/test/unit/force_only_direct_deps/BUILD.bazel new file mode 100644 index 0000000000..054dfaac5e --- /dev/null +++ b/test/unit/force_only_direct_deps/BUILD.bazel @@ -0,0 +1,4 @@ +load(":force_only_direct_deps_test.bzl", "force_only_direct_deps_test_suite") + +############################ UNIT TESTS ############################# +force_only_direct_deps_test_suite(name = "force_only_direct_deps_test_suite") diff --git a/test/unit/force_only_direct_deps/direct.rs b/test/unit/force_only_direct_deps/direct.rs new file mode 100644 index 0000000000..394a1fc0a5 --- /dev/null +++ b/test/unit/force_only_direct_deps/direct.rs @@ -0,0 +1,5 @@ +use transitive; + +pub fn direct_fn() { + transitive::transitive_fn(); +} \ No newline at end of file diff --git a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl new file mode 100644 index 0000000000..fa05934eac --- /dev/null +++ b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl @@ -0,0 +1,58 @@ +"""Unittest to verify that we can treat all dependencies as direct dependencies""" + +load("@bazel_skylib//lib:unittest.bzl", "analysistest") +load("//rust:defs.bzl", "rust_library") +load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_argv_contains_prefix") +load("//test/unit/force_only_direct_deps:generator.bzl", "generator") + +def _force_only_direct_deps_rustc_flags_test(ctx): + env = analysistest.begin(ctx) + tut = analysistest.target_under_test(env) + action = tut.actions[1] + argv = action.argv + assert_action_mnemonic(env, action, "Rustc") + assert_argv_contains_prefix( + env, action, + "--extern=transitive=bazel-out/k8-fastbuild/bin/test/unit/force_only_direct_deps/libtransitive") + return analysistest.end(env) + +force_only_direct_deps_test = analysistest.make(_force_only_direct_deps_rustc_flags_test) + +def _force_only_direct_deps_test(): + rust_library( + name = "direct", + srcs = ["direct.rs"], + edition = "2018", + deps = [":transitive"] + ) + + rust_library( + name = "transitive", + srcs = ["transitive.rs"], + edition = "2018", + ) + + generator( + name = "generate", + deps = [":direct"], + ) + + force_only_direct_deps_test( + name = "force_only_direct_deps_rustc_flags_test", + target_under_test = ":generate", + ) + +def force_only_direct_deps_test_suite(name): + """Entry-point macro called from the BUILD file. + + Args: + name: Name of the macro. + """ + _force_only_direct_deps_test() + + native.test_suite( + name = name, + tests = [ + ":force_only_direct_deps_rustc_flags_test", + ], + ) diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl new file mode 100644 index 0000000000..50a47ede4a --- /dev/null +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -0,0 +1,91 @@ +load("//rust/private:common.bzl", "rust_common") +load("//rust/private:providers.bzl", "BuildInfo", "CrateInfo", "DepInfo", "DepVariantInfo") +load("//rust/private:rustc.bzl", "rustc_compile_action") + +load( + "//rust/private:utils.bzl", + "determine_output_hash", + "find_toolchain", +) + +def _generator_impl(ctx): + rs_file = ctx.actions.declare_file(ctx.label.name + "_generated.rs") + ctx.actions.run_shell( + outputs = [rs_file], + command = """cat < {} +use direct; +use transitive; + +pub fn call_both() {} + direct::direct_fn(); + transitive::transitive_fn(); +{} +EOF +""".format(rs_file.path, "{", "}"), + mnemonic = "WriteRsFile" + ) + + toolchain = ctx.toolchains[Label("//rust:toolchain")] + # Determine unique hash for this rlib + output_hash = determine_output_hash(rs_file) + + crate_name = ctx.label.name + "_generated" + crate_type = "rlib" + + rust_lib_name = "{prefix}{name}-{lib_hash}{extension}".format( + prefix = "lib", + name = crate_name, + lib_hash = output_hash, + extension = ".rlib", + ) + + deps = [DepVariantInfo( + crate_info = dep[CrateInfo] if CrateInfo in dep else None, + dep_info = dep[DepInfo] if DepInfo in dep else None, + build_info = dep[BuildInfo] if BuildInfo in dep else None, + cc_info = dep[CcInfo] if CcInfo in dep else None, + ) for dep in ctx.attr.deps] + + rust_lib = ctx.actions.declare_file(rust_lib_name) + return rustc_compile_action( + ctx = ctx, + attr = ctx.attr, + toolchain = toolchain, + crate_info = rust_common.create_crate_info( + name = crate_name, + type = crate_type, + root = rs_file.path, + srcs = depset([rs_file]), + deps = depset(deps), + proc_macro_deps = depset([]), + aliases = {}, + output = rust_lib, + owner = ctx.label, + edition = "2018", + compile_data = depset([]), + rustc_env = {}, + is_test = False, + ), + output_hash = output_hash, + force_only_direct_deps = True, + ) + +generator = rule( + implementation = _generator_impl, + attrs = { + "deps": attr.label_list(), + "_cc_toolchain" : attr.label( + default = "@bazel_tools//tools/cpp:current_cc_toolchain", + ), + "_error_format": attr.label(default = "@rules_rust//:error_format"), + "_process_wrapper": attr.label( + default = Label("@rules_rust//util/process_wrapper"), + executable = True, + allow_single_file = True, + cfg = "exec", + ), + }, + toolchains = ["@rules_rust//rust:toolchain", "@bazel_tools//tools/cpp:toolchain_type"], + incompatible_use_toolchain_transition = True, + fragments = ["cpp"], +) \ No newline at end of file diff --git a/test/unit/force_only_direct_deps/transitive.rs b/test/unit/force_only_direct_deps/transitive.rs new file mode 100644 index 0000000000..ddfaeda015 --- /dev/null +++ b/test/unit/force_only_direct_deps/transitive.rs @@ -0,0 +1 @@ +pub fn transitive_fn() {} \ No newline at end of file From 03805e889d311470c3e1a589870969302adf56f0 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 14:36:17 +0200 Subject: [PATCH 03/11] Add newlines --- test/unit/force_only_direct_deps/direct.rs | 2 +- test/unit/force_only_direct_deps/generator.bzl | 2 +- test/unit/force_only_direct_deps/transitive.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/force_only_direct_deps/direct.rs b/test/unit/force_only_direct_deps/direct.rs index 394a1fc0a5..d02fcdc76c 100644 --- a/test/unit/force_only_direct_deps/direct.rs +++ b/test/unit/force_only_direct_deps/direct.rs @@ -2,4 +2,4 @@ use transitive; pub fn direct_fn() { transitive::transitive_fn(); -} \ No newline at end of file +} diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl index 50a47ede4a..c0bb50eac4 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -88,4 +88,4 @@ generator = rule( toolchains = ["@rules_rust//rust:toolchain", "@bazel_tools//tools/cpp:toolchain_type"], incompatible_use_toolchain_transition = True, fragments = ["cpp"], -) \ No newline at end of file +) diff --git a/test/unit/force_only_direct_deps/transitive.rs b/test/unit/force_only_direct_deps/transitive.rs index ddfaeda015..2ac85dfb21 100644 --- a/test/unit/force_only_direct_deps/transitive.rs +++ b/test/unit/force_only_direct_deps/transitive.rs @@ -1 +1 @@ -pub fn transitive_fn() {} \ No newline at end of file +pub fn transitive_fn() {} From bcb24005e940654ea00cd56d19ef013d24d6cc30 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 14:51:26 +0200 Subject: [PATCH 04/11] Consider aliases of direct deps --- rust/private/rustc.bzl | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 0fe98242b8..5840760287 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -977,20 +977,24 @@ def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): format_each = "-Ldependency=%s", ) -def _crate_to_link_flag(crate_info): +def _crate_to_link_flag(crate): """A helper macro used by `add_crate_link_flags` for adding crate link flags to a Arg object Args: - crate_info (CrateInfo|AliasableDepInfo): A CrateInfo or a AliasableDepInfo provider + crate (CrateInfo|AliasableDepInfo): A CrateInfo or an AliasableDepInfo provider Returns: list: Link flags for the given crate_info """ - # This is AliasableDepInfo - if hasattr(crate_info, "dep"): - crate_info = crate_info.dep - return ["--extern={}={}".format(crate_info.name, crate_info.output.path)] + # This is AliasableDepInfo, we should use the alias as a crate name + if hasattr(crate, "dep"): + name = crate.name + crate_info = crate.dep + else: + name = crate.name + crate_info = crate + return ["--extern={}={}".format(name, crate_info.output.path)] def _get_crate_dirname(crate): """A helper macro used by `add_crate_link_flags` for getting the directory name of the current crate's output path From 9a90c9e44758340669c9d8cbb18c948443f246a3 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 15:05:51 +0200 Subject: [PATCH 05/11] Run buildifier --- rust/private/rustc.bzl | 9 ++++++--- .../force_only_direct_deps_test.bzl | 8 +++++--- test/unit/force_only_direct_deps/generator.bzl | 8 ++++---- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 5840760287..c14d0ffed0 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -960,9 +960,12 @@ def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): """ if force_only_direct_deps: - args.add_all(depset( - transitive = [dep_info.direct_crates, - dep_info.transitive_crates] + args.add_all( + depset( + transitive = [ + dep_info.direct_crates, + dep_info.transitive_crates, + ], ), uniquify = True, map_each = _crate_to_link_flag, diff --git a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl index fa05934eac..9f049f08d8 100644 --- a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl +++ b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl @@ -12,8 +12,10 @@ def _force_only_direct_deps_rustc_flags_test(ctx): argv = action.argv assert_action_mnemonic(env, action, "Rustc") assert_argv_contains_prefix( - env, action, - "--extern=transitive=bazel-out/k8-fastbuild/bin/test/unit/force_only_direct_deps/libtransitive") + env, + action, + "--extern=transitive=bazel-out/k8-fastbuild/bin/test/unit/force_only_direct_deps/libtransitive", + ) return analysistest.end(env) force_only_direct_deps_test = analysistest.make(_force_only_direct_deps_rustc_flags_test) @@ -23,7 +25,7 @@ def _force_only_direct_deps_test(): name = "direct", srcs = ["direct.rs"], edition = "2018", - deps = [":transitive"] + deps = [":transitive"], ) rust_library( diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl index c0bb50eac4..9eba9f45ae 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -1,7 +1,6 @@ load("//rust/private:common.bzl", "rust_common") load("//rust/private:providers.bzl", "BuildInfo", "CrateInfo", "DepInfo", "DepVariantInfo") load("//rust/private:rustc.bzl", "rustc_compile_action") - load( "//rust/private:utils.bzl", "determine_output_hash", @@ -22,14 +21,15 @@ pub fn call_both() {} {} EOF """.format(rs_file.path, "{", "}"), - mnemonic = "WriteRsFile" + mnemonic = "WriteRsFile", ) toolchain = ctx.toolchains[Label("//rust:toolchain")] + # Determine unique hash for this rlib output_hash = determine_output_hash(rs_file) - crate_name = ctx.label.name + "_generated" + crate_name = ctx.label.name crate_type = "rlib" rust_lib_name = "{prefix}{name}-{lib_hash}{extension}".format( @@ -74,7 +74,7 @@ generator = rule( implementation = _generator_impl, attrs = { "deps": attr.label_list(), - "_cc_toolchain" : attr.label( + "_cc_toolchain": attr.label( default = "@bazel_tools//tools/cpp:current_cc_toolchain", ), "_error_format": attr.label(default = "@rules_rust//:error_format"), From 13e1d604cd107b994b88bc8cfbf1dc2eb48673b9 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 15:26:05 +0200 Subject: [PATCH 06/11] Fix visibility issues --- rust/rust_common.bzl | 19 ++++++++++++++++++- .../unit/force_only_direct_deps/generator.bzl | 19 ++++++++++--------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/rust/rust_common.bzl b/rust/rust_common.bzl index 4777ba1430..0553a8bfbf 100644 --- a/rust/rust_common.bzl +++ b/rust/rust_common.bzl @@ -14,7 +14,24 @@ """Module with Rust definitions required to write custom Rust rules.""" -load("//rust/private:providers.bzl", _ClippyInfo = "ClippyInfo", _CrateInfo = "CrateInfo") +load("//rust/private:common.bzl", _rust_common = "rust_common") +load( + "//rust/private:providers.bzl", + _BuildInfo = "BuildInfo", + _ClippyInfo = "ClippyInfo", + _CrateInfo = "CrateInfo", + _DepInfo = "DepInfo", + _DepVariantInfo = "DepVariantInfo", +) +load("//rust/private:rustc.bzl", _rustc_compile_action = "rustc_compile_action") +BuildInfo = _BuildInfo CrateInfo = _CrateInfo ClippyInfo = _ClippyInfo +DepInfo = _DepInfo +DepVariantInfo = _DepVariantInfo + +rust_common = struct( + create_crate_info = _rust_common.create_crate_info, + rustc_compile_action = _rustc_compile_action, +) diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl index 9eba9f45ae..3c799aa770 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -1,10 +1,12 @@ -load("//rust/private:common.bzl", "rust_common") -load("//rust/private:providers.bzl", "BuildInfo", "CrateInfo", "DepInfo", "DepVariantInfo") -load("//rust/private:rustc.bzl", "rustc_compile_action") +"""A custom rule that threats all its dependencies as direct dependencies.""" + load( - "//rust/private:utils.bzl", - "determine_output_hash", - "find_toolchain", + "//rust:rust_common.bzl", + "BuildInfo", + "CrateInfo", + "DepInfo", + "DepVariantInfo", + "rust_common", ) def _generator_impl(ctx): @@ -27,8 +29,7 @@ EOF toolchain = ctx.toolchains[Label("//rust:toolchain")] # Determine unique hash for this rlib - output_hash = determine_output_hash(rs_file) - + output_hash = repr(hash(rs_file.path)) crate_name = ctx.label.name crate_type = "rlib" @@ -47,7 +48,7 @@ EOF ) for dep in ctx.attr.deps] rust_lib = ctx.actions.declare_file(rust_lib_name) - return rustc_compile_action( + return rust_common.rustc_compile_action( ctx = ctx, attr = ctx.attr, toolchain = toolchain, From f7729b7dacad0f61b6b95853e90c967fa2d3ae55 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 15:38:42 +0200 Subject: [PATCH 07/11] Pass a source as crate root instead of the source path --- test/unit/force_only_direct_deps/generator.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl index 3c799aa770..5bfa067e2f 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -55,7 +55,7 @@ EOF crate_info = rust_common.create_crate_info( name = crate_name, type = crate_type, - root = rs_file.path, + root = rs_file, srcs = depset([rs_file]), deps = depset(deps), proc_macro_deps = depset([]), From ee5350ccffe934dbf277a5e2431abab4c44dab86 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 15:47:04 +0200 Subject: [PATCH 08/11] Correct 'use' statements --- test/unit/force_only_direct_deps/direct.rs | 4 ++-- test/unit/force_only_direct_deps/generator.bzl | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/force_only_direct_deps/direct.rs b/test/unit/force_only_direct_deps/direct.rs index d02fcdc76c..eae45b3f72 100644 --- a/test/unit/force_only_direct_deps/direct.rs +++ b/test/unit/force_only_direct_deps/direct.rs @@ -1,5 +1,5 @@ -use transitive; +use transitive::transitive_fn; pub fn direct_fn() { - transitive::transitive_fn(); + transitive_fn(); } diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_only_direct_deps/generator.bzl index 5bfa067e2f..7965d03b14 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_only_direct_deps/generator.bzl @@ -14,12 +14,12 @@ def _generator_impl(ctx): ctx.actions.run_shell( outputs = [rs_file], command = """cat < {} -use direct; -use transitive; +use direct::direct_fn; +use transitive::transitive_fn; pub fn call_both() {} - direct::direct_fn(); - transitive::transitive_fn(); + direct_fn(); + transitive_fn(); {} EOF """.format(rs_file.path, "{", "}"), From 525960548eb4cfa5f772d42b975ca489349949f0 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 16:37:16 +0200 Subject: [PATCH 09/11] Disable clippy for the generator target --- test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl index 9f049f08d8..5c88fba462 100644 --- a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl +++ b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl @@ -37,6 +37,7 @@ def _force_only_direct_deps_test(): generator( name = "generate", deps = [":direct"], + tags = ["noclippy"], ) force_only_direct_deps_test( From a87ad72d6639bf04dcd362394361164c274177f2 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Wed, 13 Oct 2021 16:42:44 +0200 Subject: [PATCH 10/11] Change assertion to be platform independent --- .../unit/force_only_direct_deps/force_only_direct_deps_test.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl index 5c88fba462..5aaf5c55ce 100644 --- a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl +++ b/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl @@ -14,7 +14,7 @@ def _force_only_direct_deps_rustc_flags_test(ctx): assert_argv_contains_prefix( env, action, - "--extern=transitive=bazel-out/k8-fastbuild/bin/test/unit/force_only_direct_deps/libtransitive", + "--extern=transitive", ) return analysistest.end(env) From 8413b2183846aed38c4fb5821205cbbccc30d1a3 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Thu, 14 Oct 2021 14:29:09 +0200 Subject: [PATCH 11/11] Address comments --- rust/private/rustc.bzl | 22 +++++++++---------- rust/rust_common.bzl | 19 +--------------- test/unit/force_all_deps_direct/BUILD.bazel | 4 ++++ .../direct.rs | 0 .../force_all_deps_direct_test.bzl} | 18 +++++++-------- .../generator.bzl | 20 ++++++++--------- .../transitive.rs | 0 test/unit/force_only_direct_deps/BUILD.bazel | 4 ---- 8 files changed, 35 insertions(+), 52 deletions(-) create mode 100644 test/unit/force_all_deps_direct/BUILD.bazel rename test/unit/{force_only_direct_deps => force_all_deps_direct}/direct.rs (100%) rename test/unit/{force_only_direct_deps/force_only_direct_deps_test.bzl => force_all_deps_direct/force_all_deps_direct_test.bzl} (71%) rename test/unit/{force_only_direct_deps => force_all_deps_direct}/generator.bzl (86%) rename test/unit/{force_only_direct_deps => force_all_deps_direct}/transitive.rs (100%) delete mode 100644 test/unit/force_only_direct_deps/BUILD.bazel diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index c14d0ffed0..545fdd933f 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -434,7 +434,7 @@ def construct_arguments( build_env_files, build_flags_files, emit = ["dep-info", "link"], - force_only_direct_deps = False): + force_all_deps_direct = False): """Builds an Args object containing common rustc flags Args: @@ -454,7 +454,7 @@ def construct_arguments( build_env_files (list): Files containing rustc environment variables, for instance from `cargo_build_script` actions. build_flags_files (list): The output files of a `cargo_build_script` actions containing rustc build flags emit (list): Values for the --emit flag to rustc. - force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern to the commandline as opposed to -L. Returns: @@ -586,7 +586,7 @@ def construct_arguments( _add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, crate_info.type, toolchain, cc_toolchain, feature_configuration) # These always need to be added, even if not linking this crate. - add_crate_link_flags(rustc_flags, dep_info, force_only_direct_deps) + add_crate_link_flags(rustc_flags, dep_info, force_all_deps_direct) needs_extern_proc_macro_flag = "proc-macro" in [crate_info.type, crate_info.wrapped_crate_type] and \ crate_info.edition != "2015" @@ -636,7 +636,7 @@ def rustc_compile_action( output_hash = None, rust_flags = [], environ = {}, - force_only_direct_deps = False): + force_all_deps_direct = False): """Create and run a rustc compile action based on the current rule's attributes Args: @@ -647,7 +647,7 @@ def rustc_compile_action( output_hash (str, optional): The hashed path of the crate root. Defaults to None. rust_flags (list, optional): Additional flags to pass to rustc. Defaults to []. environ (dict, optional): A set of makefile expandable environment variables for the action - force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern to the commandline as opposed to -L. Returns: @@ -696,7 +696,7 @@ def rustc_compile_action( out_dir = out_dir, build_env_files = build_env_files, build_flags_files = build_flags_files, - force_only_direct_deps = force_only_direct_deps, + force_all_deps_direct = force_all_deps_direct, ) if hasattr(attr, "version") and attr.version != "0.0.0": @@ -949,17 +949,17 @@ def _get_dir_names(files): dirs[f.dirname] = None return dirs.keys() -def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): +def add_crate_link_flags(args, dep_info, force_all_deps_direct = False): """Adds link flags to an Args object reference Args: args (Args): An arguments object reference dep_info (DepInfo): The current target's dependency info - force_only_direct_deps (bool, optional): Whether to pass the transitive rlibs with --external + force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern to the commandline as opposed to -L. """ - if force_only_direct_deps: + if force_all_deps_direct: args.add_all( depset( transitive = [ @@ -971,7 +971,7 @@ def add_crate_link_flags(args, dep_info, force_only_direct_deps = False): map_each = _crate_to_link_flag, ) else: - # nb. Crates are linked via --extern regardless of their crate_type + # nb. Direct crates are linked via --extern regardless of their crate_type args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag) args.add_all( dep_info.transitive_crates, @@ -987,7 +987,7 @@ def _crate_to_link_flag(crate): crate (CrateInfo|AliasableDepInfo): A CrateInfo or an AliasableDepInfo provider Returns: - list: Link flags for the given crate_info + list: Link flags for the given provider """ # This is AliasableDepInfo, we should use the alias as a crate name diff --git a/rust/rust_common.bzl b/rust/rust_common.bzl index 0553a8bfbf..4777ba1430 100644 --- a/rust/rust_common.bzl +++ b/rust/rust_common.bzl @@ -14,24 +14,7 @@ """Module with Rust definitions required to write custom Rust rules.""" -load("//rust/private:common.bzl", _rust_common = "rust_common") -load( - "//rust/private:providers.bzl", - _BuildInfo = "BuildInfo", - _ClippyInfo = "ClippyInfo", - _CrateInfo = "CrateInfo", - _DepInfo = "DepInfo", - _DepVariantInfo = "DepVariantInfo", -) -load("//rust/private:rustc.bzl", _rustc_compile_action = "rustc_compile_action") +load("//rust/private:providers.bzl", _ClippyInfo = "ClippyInfo", _CrateInfo = "CrateInfo") -BuildInfo = _BuildInfo CrateInfo = _CrateInfo ClippyInfo = _ClippyInfo -DepInfo = _DepInfo -DepVariantInfo = _DepVariantInfo - -rust_common = struct( - create_crate_info = _rust_common.create_crate_info, - rustc_compile_action = _rustc_compile_action, -) diff --git a/test/unit/force_all_deps_direct/BUILD.bazel b/test/unit/force_all_deps_direct/BUILD.bazel new file mode 100644 index 0000000000..12f22c5177 --- /dev/null +++ b/test/unit/force_all_deps_direct/BUILD.bazel @@ -0,0 +1,4 @@ +load(":force_all_deps_direct_test.bzl", "force_all_deps_direct_test_suite") + +############################ UNIT TESTS ############################# +force_all_deps_direct_test_suite(name = "force_all_deps_direct_test_suite") diff --git a/test/unit/force_only_direct_deps/direct.rs b/test/unit/force_all_deps_direct/direct.rs similarity index 100% rename from test/unit/force_only_direct_deps/direct.rs rename to test/unit/force_all_deps_direct/direct.rs diff --git a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl b/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl similarity index 71% rename from test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl rename to test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl index 5aaf5c55ce..7e0964aa08 100644 --- a/test/unit/force_only_direct_deps/force_only_direct_deps_test.bzl +++ b/test/unit/force_all_deps_direct/force_all_deps_direct_test.bzl @@ -3,9 +3,9 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest") load("//rust:defs.bzl", "rust_library") load("//test/unit:common.bzl", "assert_action_mnemonic", "assert_argv_contains_prefix") -load("//test/unit/force_only_direct_deps:generator.bzl", "generator") +load("//test/unit/force_all_deps_direct:generator.bzl", "generator") -def _force_only_direct_deps_rustc_flags_test(ctx): +def _force_all_deps_direct_rustc_flags_test(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) action = tut.actions[1] @@ -18,9 +18,9 @@ def _force_only_direct_deps_rustc_flags_test(ctx): ) return analysistest.end(env) -force_only_direct_deps_test = analysistest.make(_force_only_direct_deps_rustc_flags_test) +force_all_deps_direct_test = analysistest.make(_force_all_deps_direct_rustc_flags_test) -def _force_only_direct_deps_test(): +def _force_all_deps_direct_test(): rust_library( name = "direct", srcs = ["direct.rs"], @@ -40,22 +40,22 @@ def _force_only_direct_deps_test(): tags = ["noclippy"], ) - force_only_direct_deps_test( - name = "force_only_direct_deps_rustc_flags_test", + force_all_deps_direct_test( + name = "force_all_deps_direct_rustc_flags_test", target_under_test = ":generate", ) -def force_only_direct_deps_test_suite(name): +def force_all_deps_direct_test_suite(name): """Entry-point macro called from the BUILD file. Args: name: Name of the macro. """ - _force_only_direct_deps_test() + _force_all_deps_direct_test() native.test_suite( name = name, tests = [ - ":force_only_direct_deps_rustc_flags_test", + ":force_all_deps_direct_rustc_flags_test", ], ) diff --git a/test/unit/force_only_direct_deps/generator.bzl b/test/unit/force_all_deps_direct/generator.bzl similarity index 86% rename from test/unit/force_only_direct_deps/generator.bzl rename to test/unit/force_all_deps_direct/generator.bzl index 7965d03b14..e8ab4450f1 100644 --- a/test/unit/force_only_direct_deps/generator.bzl +++ b/test/unit/force_all_deps_direct/generator.bzl @@ -1,13 +1,13 @@ """A custom rule that threats all its dependencies as direct dependencies.""" -load( - "//rust:rust_common.bzl", - "BuildInfo", - "CrateInfo", - "DepInfo", - "DepVariantInfo", - "rust_common", -) +# buildifier: disable=bzl-visibility +load("//rust/private:common.bzl", "rust_common") + +# buildifier: disable=bzl-visibility +load("//rust/private:providers.bzl", "BuildInfo", "CrateInfo", "DepInfo", "DepVariantInfo") + +# buildifier: disable=bzl-visibility +load("//rust/private:rustc.bzl", "rustc_compile_action") def _generator_impl(ctx): rs_file = ctx.actions.declare_file(ctx.label.name + "_generated.rs") @@ -48,7 +48,7 @@ EOF ) for dep in ctx.attr.deps] rust_lib = ctx.actions.declare_file(rust_lib_name) - return rust_common.rustc_compile_action( + return rustc_compile_action( ctx = ctx, attr = ctx.attr, toolchain = toolchain, @@ -68,7 +68,7 @@ EOF is_test = False, ), output_hash = output_hash, - force_only_direct_deps = True, + force_all_deps_direct = True, ) generator = rule( diff --git a/test/unit/force_only_direct_deps/transitive.rs b/test/unit/force_all_deps_direct/transitive.rs similarity index 100% rename from test/unit/force_only_direct_deps/transitive.rs rename to test/unit/force_all_deps_direct/transitive.rs diff --git a/test/unit/force_only_direct_deps/BUILD.bazel b/test/unit/force_only_direct_deps/BUILD.bazel deleted file mode 100644 index 054dfaac5e..0000000000 --- a/test/unit/force_only_direct_deps/BUILD.bazel +++ /dev/null @@ -1,4 +0,0 @@ -load(":force_only_direct_deps_test.bzl", "force_only_direct_deps_test_suite") - -############################ UNIT TESTS ############################# -force_only_direct_deps_test_suite(name = "force_only_direct_deps_test_suite")