From 169476f772132723b2cc819c96209c6fbce3e366 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Fri, 2 Oct 2020 07:15:03 -0700 Subject: [PATCH] Improved internal and external documentation (#419) * Renamed RustProtoProvider -> RustProtoInfo * Partially addressed legacy provider syntax issue * Renamed AliasableDep -> AliasableDepInfo * Renamed relative_path -> relativize * Moved get_lib_name to //rust:private/utils.bzl * Moved _determine_output_hash so it can be used in other places --- proto/proto.bzl | 16 ++++----- rust/private/clippy.bzl | 4 +-- rust/private/rust.bzl | 7 ++-- rust/private/rustc.bzl | 18 +++------- rust/private/rustdoc_test.bzl | 4 +-- rust/private/utils.bzl | 62 +++++++++++++++++++++++++++++------ wasm_bindgen/wasm_bindgen.bzl | 18 ++++++---- 7 files changed, 83 insertions(+), 46 deletions(-) diff --git a/proto/proto.bzl b/proto/proto.bzl index 78aa5ba0b1..8b5a9b4653 100644 --- a/proto/proto.bzl +++ b/proto/proto.bzl @@ -42,9 +42,9 @@ load( _generated_file_stem = "generated_file_stem", ) load("@io_bazel_rules_rust//rust:private/rustc.bzl", "CrateInfo", "rustc_compile_action") -load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain") +load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain", "determine_output_hash") -RustProtoProvider = provider( +RustProtoInfo = provider( fields = { "proto_sources": "List[string]: list of source paths of protos", "transitive_proto_sources": "depset[string]", @@ -99,11 +99,11 @@ def _rust_proto_aspect_impl(target, ctx): for f in target[ProtoInfo].direct_sources ] transitive_sources = [ - f[RustProtoProvider].transitive_proto_sources + f[RustProtoInfo].transitive_proto_sources for f in ctx.rule.attr.deps - if RustProtoProvider in f + if RustProtoInfo in f ] - return RustProtoProvider( + return RustProtoInfo( proto_sources = sources, transitive_proto_sources = depset(transitive = transitive_sources, direct = sources), ) @@ -152,7 +152,7 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, grpc, srcs.append(lib_rs) # And simulate rust_library behavior - output_hash = repr(hash(lib_rs.path)) + output_hash = determine_output_hash(lib_rs) rust_lib = ctx.actions.declare_file("%s/lib%s-%s.rlib" % ( output_dir, crate_name, @@ -182,9 +182,9 @@ def _rust_protogrpc_library_impl(ctx, grpc): """Implementation of the rust_(proto|grpc)_library.""" proto = _expand_provider(ctx.attr.deps, ProtoInfo) transitive_sources = [ - f[RustProtoProvider].transitive_proto_sources + f[RustProtoInfo].transitive_proto_sources for f in ctx.attr.deps - if RustProtoProvider in f + if RustProtoInfo in f ] srcs = depset(transitive = transitive_sources) diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index f282bdb361..2af329e8e0 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -24,7 +24,7 @@ load( "@io_bazel_rules_rust//rust:private/rust.bzl", "crate_root_src", ) -load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain") +load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain", "determine_output_hash") _rust_extensions = [ "rs", @@ -85,7 +85,7 @@ def _clippy_aspect_impl(target, ctx): feature_configuration, crate_info, dep_info, - output_hash = repr(hash(root.path)), + output_hash = determine_output_hash(root), rust_flags = [], out_dir = out_dir, build_env_file = build_env_file, diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index b4500f6608..62c81191da 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -13,13 +13,10 @@ # limitations under the License. load("@io_bazel_rules_rust//rust:private/rustc.bzl", "CrateInfo", "rustc_compile_action") -load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain") +load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain", "determine_output_hash") # TODO(marco): Separate each rule into its own file. -def _determine_output_hash(lib_rs): - return repr(hash(lib_rs.path)) - def _deprecated_attributes(ctx): if getattr(ctx.attr, "out_dir_tar", None): fail(ctx, "".join([ @@ -101,7 +98,7 @@ def _rust_library_impl(ctx): toolchain = find_toolchain(ctx) # Determine unique hash for this rlib - output_hash = _determine_output_hash(lib_rs) + output_hash = determine_output_hash(lib_rs) crate_name = ctx.label.name.replace("-", "_") rust_lib_name = _determine_lib_name( diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index c3d5f9b08e..fe21f0c53b 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@io_bazel_rules_rust//rust:private/utils.bzl", "relative_path") +load("@io_bazel_rules_rust//rust:private/utils.bzl", "get_lib_name", "relativize") load("@io_bazel_rules_rust//rust:private/legacy_cc_starlark_api_shim.bzl", "get_libs_for_static_executable") load( "@bazel_tools//tools/build_defs/cc:action_names.bzl", @@ -51,7 +51,8 @@ BuildInfo = provider( }, ) -AliasableDep = provider( +AliasableDepInfo = provider( + doc = "", fields = { "name": "str", "dep": "CrateInfo", @@ -98,15 +99,6 @@ def get_compilation_mode_opts(ctx, toolchain): return toolchain.compilation_mode_opts[comp_mode] -def get_lib_name(lib): - """Returns the name of a library artifact, eg. libabc.a -> abc""" - libname, ext = lib.basename.split(".", 2) - - if libname.startswith("lib"): - return libname[3:] - else: - return libname - def collect_deps(label, deps, proc_macro_deps, aliases, toolchain): """ Walks through dependencies and collects the transitive dependencies. @@ -153,7 +145,7 @@ def collect_deps(label, deps, proc_macro_deps, aliases, toolchain): if CrateInfo in dep: # This dependency is a rust_library direct_dep = dep[CrateInfo] - aliasable_dep = AliasableDep( + aliasable_dep = AliasableDepInfo( name = aliases.get(dep.label, direct_dep.name), dep = direct_dep, ) @@ -596,7 +588,7 @@ def _compute_rpaths(toolchain, output_dir, dep_info): # Multiple dylibs can be present in the same directory, so deduplicate them. return depset([ - relative_path(output_dir, lib_dir) + relativize(lib_dir, output_dir) for lib_dir in _get_dir_names(dep_info.transitive_dylibs.to_list()) ]) diff --git a/rust/private/rustdoc_test.bzl b/rust/private/rustdoc_test.bzl index ed02aa8bc7..2b2d159858 100644 --- a/rust/private/rustdoc_test.bzl +++ b/rust/private/rustdoc_test.bzl @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@io_bazel_rules_rust//rust:private/rustc.bzl", "CrateInfo", "DepInfo", "get_lib_name") -load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain") +load("@io_bazel_rules_rust//rust:private/rustc.bzl", "CrateInfo", "DepInfo") +load("@io_bazel_rules_rust//rust:private/utils.bzl", "find_toolchain", "get_lib_name") def _rust_doc_test_impl(ctx): if CrateInfo not in ctx.attr.dep: diff --git a/rust/private/utils.bzl b/rust/private/utils.bzl index 0e3e7de527..37703bebad 100644 --- a/rust/private/utils.bzl +++ b/rust/private/utils.bzl @@ -12,18 +12,33 @@ # See the License for the specific language governing permissions and # limitations under the License. -""" -Utility functions not specific to the rust toolchain. -""" +"""Utility functions not specific to the rust toolchain.""" def find_toolchain(ctx): - """Finds the first rust toolchain that is configured.""" + """Finds the first rust toolchain that is configured. + + Args: + ctx (ctx): The ctx object for the current target. + + Returns: + ToolchainContext: A Rust toolchain context. + """ return ctx.toolchains["@io_bazel_rules_rust//rust:toolchain"] -def relative_path(src_path, dest_path): - """Returns the relative path from src_path to dest_path.""" - src_parts = _path_parts(src_path) - dest_parts = _path_parts(dest_path) +# TODO: Replace with bazel-skylib's `path.dirname`. This requires addressing some +# dependency issues or generating docs will break. +def relativize(path, start): + """Returns the relative path from start to path. + + Args: + path (str): The path to relativize. + start (str): The ancestor path against which to relativize. + + Returns: + str: The portion of `path` that is relative to `start`. + """ + src_parts = _path_parts(start) + dest_parts = _path_parts(path) n = 0 done = False for src_part, dest_part in zip(src_parts, dest_parts): @@ -41,14 +56,41 @@ def relative_path(src_path, dest_path): def _path_parts(path): """Takes a path and returns a list of its parts with all "." elements removed. - The main use case of this function is if one of the inputs to _relative() + The main use case of this function is if one of the inputs to relativize() is a relative path, such as "./foo". Args: - path_parts: A list containing parts of a path. + path: A list containing parts of a path. Returns: Returns a list containing the path parts with all "." elements removed. """ path_parts = path.split("/") return [part for part in path_parts if part != "."] + +def get_lib_name(lib): + """Returns the name of a library artifact, eg. libabc.a -> abc + + Args: + lib (File): A library file + + Returns: + str: The name of the library + """ + libname, ext = lib.basename.split(".", 2) + + if libname.startswith("lib"): + return libname[3:] + else: + return libname + +def determine_output_hash(crate_root): + """Generates a hash of the crate root file's path. + + Args: + crate_root (File): The crate's root file (typically `lib.rs`). + + Returns: + str: A string representation of the hash. + """ + return repr(hash(crate_root.path)) \ No newline at end of file diff --git a/wasm_bindgen/wasm_bindgen.bzl b/wasm_bindgen/wasm_bindgen.bzl index dc9d8f4565..06fd2bfae9 100644 --- a/wasm_bindgen/wasm_bindgen.bzl +++ b/wasm_bindgen/wasm_bindgen.bzl @@ -38,13 +38,19 @@ def _rust_wasm_bindgen_impl(ctx): arguments = [args], ) + # TODO: Legacy provider syntax should be updated. See the following guide: + # https://docs.bazel.build/versions/master/skylark/rules.html#migrating-from-legacy-providers return struct( - files = depset([ - ctx.outputs.bindgen_wasm_module, - ctx.outputs.bindgen_typescript_bindings, - ctx.outputs.typescript_bindings, - ctx.outputs.javascript_bindings, - ]), + providers = [ + DefaultInfo( + files = depset([ + ctx.outputs.bindgen_wasm_module, + ctx.outputs.bindgen_typescript_bindings, + ctx.outputs.typescript_bindings, + ctx.outputs.javascript_bindings, + ]), + ), + ], typescript = struct( declarations = depset([ ctx.outputs.typescript_bindings,