Skip to content

Commit

Permalink
Refactor CrateInfo construction (#2161)
Browse files Browse the repository at this point in the history 
## Description

This PR addresses #2013
to ensure rust-analyzer can access all the env vars the rust rules pass
to `rustc` at compile time. This is a large refactoring in almost all
the rules and aspects so I aim this PR to just fix `rust_library` rule.
The follow-up PR will address the remaining rules and aspects.

## Summary
* Create `create_crate_info_dict` function in `rust/private/utils.bzl`
to create a mutable dict repsenting CrateInfo
* Move `_determine_lib_name`, `get_edition`, `_transform_sources`
functions to `rust/private/utils.bzl` to avoid cyclic dependency when
`create_crate_info_dict` function use these
* Introduce optional `create_crate_info_callback` attribute to
`rustc_compile_action` to allow creating `CrateInfo` inside
`rustc_compile_action`. This optional attribute allows scoping the
refactoring in this PR to just `rust_library` rule.
* Introduce optional `skip_expanding_rustc_env` attribute to
`rustc_compile_action` to skip expanding `rustc_env` attr. This is
useful for `clippy` aspect and `rust_test` rule because the `CrateInfo`
provided from the depended `rust_library` already expands all the env
vars before returning the provider downstream.

## Notes
* `rustc_env_attr` is a temporary field in `CrateInfo` and needed by the
rules and aspects not migrated to create `CrateInfo` in
`rustc_compile_action` yet. It will be remove in the next PR after
`CrateInfo.rustc_env` is always expanded.
* `create_crate_info_callback` will be removed from
`rustc_compile_action` after all `CrateInfo`s are created inside
`rustc_compile_action`.

---------

Co-authored-by: scentini <[email protected]>
  • Loading branch information
@scentini
Vinh Tran and scentini authored Oct 6, 2023
1 parent a458757 commit 8b548d2
Show file tree
Hide file tree
Showing 15 changed files with 279 additions and 182 deletions.
3 changes: 2 additions & 1 deletion docs/flatten.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,7 @@ A toolchain for [rustfmt](https://rust-lang.github.io/rustfmt/)
<pre>
CrateInfo(<a href="#CrateInfo-aliases">aliases</a>, <a href="#CrateInfo-compile_data">compile_data</a>, <a href="#CrateInfo-compile_data_targets">compile_data_targets</a>, <a href="#CrateInfo-data">data</a>, <a href="#CrateInfo-deps">deps</a>, <a href="#CrateInfo-edition">edition</a>, <a href="#CrateInfo-is_test">is_test</a>, <a href="#CrateInfo-metadata">metadata</a>, <a href="#CrateInfo-name">name</a>,
<a href="#CrateInfo-output">output</a>, <a href="#CrateInfo-owner">owner</a>, <a href="#CrateInfo-proc_macro_deps">proc_macro_deps</a>, <a href="#CrateInfo-root">root</a>, <a href="#CrateInfo-rustc_env">rustc_env</a>, <a href="#CrateInfo-rustc_env_files">rustc_env_files</a>, <a href="#CrateInfo-srcs">srcs</a>, <a href="#CrateInfo-type">type</a>,
<a href="#CrateInfo-wrapped_crate_type">wrapped_crate_type</a>)
<a href="#CrateInfo-wrapped_crate_type">wrapped_crate_type</a>, <a href="#CrateInfo-_rustc_env_attr">_rustc_env_attr</a>)
</pre>

A provider containing general Crate information.
Expand Down Expand Up @@ -1478,6 +1478,7 @@ A provider containing general Crate information.
| <a id="CrateInfo-srcs"></a>srcs | depset[File]: All source Files that are part of the crate. |
| <a id="CrateInfo-type"></a>type | str: The type of this crate (see [rustc --crate-type](https://doc.rust-lang.org/rustc/command-line-arguments.html#--crate-type-a-list-of-types-of-crates-for-the-compiler-to-emit)). |
| <a id="CrateInfo-wrapped_crate_type"></a>wrapped_crate_type | str, optional: The original crate type for targets generated using a previously defined crate (typically tests using the <code>rust_test::crate</code> attribute) |
| <a id="CrateInfo-_rustc_env_attr"></a>_rustc_env_attr | Dict[String, String]: Additional <code>"key": "value"</code> environment variables to set for rustc. |


<a id="DepInfo"></a>
Expand Down
3 changes: 2 additions & 1 deletion docs/providers.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<pre>
CrateInfo(<a href="#CrateInfo-aliases">aliases</a>, <a href="#CrateInfo-compile_data">compile_data</a>, <a href="#CrateInfo-compile_data_targets">compile_data_targets</a>, <a href="#CrateInfo-data">data</a>, <a href="#CrateInfo-deps">deps</a>, <a href="#CrateInfo-edition">edition</a>, <a href="#CrateInfo-is_test">is_test</a>, <a href="#CrateInfo-metadata">metadata</a>, <a href="#CrateInfo-name">name</a>,
<a href="#CrateInfo-output">output</a>, <a href="#CrateInfo-owner">owner</a>, <a href="#CrateInfo-proc_macro_deps">proc_macro_deps</a>, <a href="#CrateInfo-root">root</a>, <a href="#CrateInfo-rustc_env">rustc_env</a>, <a href="#CrateInfo-rustc_env_files">rustc_env_files</a>, <a href="#CrateInfo-srcs">srcs</a>, <a href="#CrateInfo-type">type</a>,
<a href="#CrateInfo-wrapped_crate_type">wrapped_crate_type</a>)
<a href="#CrateInfo-wrapped_crate_type">wrapped_crate_type</a>, <a href="#CrateInfo-_rustc_env_attr">_rustc_env_attr</a>)
</pre>

A provider containing general Crate information.
Expand Down Expand Up @@ -40,6 +40,7 @@ A provider containing general Crate information.
| <a id="CrateInfo-srcs"></a>srcs | depset[File]: All source Files that are part of the crate. |
| <a id="CrateInfo-type"></a>type | str: The type of this crate (see [rustc --crate-type](https://doc.rust-lang.org/rustc/command-line-arguments.html#--crate-type-a-list-of-types-of-crates-for-the-compiler-to-emit)). |
| <a id="CrateInfo-wrapped_crate_type"></a>wrapped_crate_type | str, optional: The original crate type for targets generated using a previously defined crate (typically tests using the <code>rust_test::crate</code> attribute) |
| <a id="CrateInfo-_rustc_env_attr"></a>_rustc_env_attr | Dict[String, String]: Additional <code>"key": "value"</code> environment variables to set for rustc. |


<a id="DepInfo"></a>
Expand Down
1 change: 1 addition & 0 deletions proto/prost/private/prost.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ def _compile_rust(ctx, attr, crate_name, src, deps, edition):
edition = edition,
is_test = False,
rustc_env = {},
_rustc_env_attr = {},
compile_data = depset([]),
compile_data_targets = depset([]),
owner = ctx.label,
Expand Down
1 change: 1 addition & 0 deletions proto/protobuf/proto.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ def _rust_proto_compile(protos, descriptor_sets, imports, crate_name, ctx, is_gr
metadata = rust_metadata,
edition = proto_toolchain.edition,
rustc_env = {},
_rustc_env_attr = {},
is_test = False,
compile_data = depset([target.files for target in getattr(ctx.attr, "compile_data", [])]),
compile_data_targets = depset(getattr(ctx.attr, "compile_data", [])),
Expand Down
1 change: 1 addition & 0 deletions rust/private/clippy.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def _clippy_aspect_impl(target, ctx):
build_env_files = build_env_files,
build_flags_files = build_flags_files,
emit = ["dep-info", "metadata"],
skip_expanding_rustc_env = True,
)

if crate_info.is_test:
Expand Down
2 changes: 2 additions & 0 deletions rust/private/providers.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ CrateInfo = provider(
"str, optional: The original crate type for targets generated using a previously defined " +
"crate (typically tests using the `rust_test::crate` attribute)"
),
# TODO: Remove `_rustc_env_attr` after refactoring rust_test to only rely on rustc_env
"_rustc_env_attr": "Dict[String, String]: Additional `\"key\": \"value\"` environment variables to set for rustc.",
},
)

Expand Down
201 changes: 31 additions & 170 deletions rust/private/rust.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,22 @@

"""Rust rule implementations"""

load("@bazel_skylib//lib:paths.bzl", "paths")
load("//rust/private:common.bzl", "rust_common")
load("//rust/private:providers.bzl", "BuildInfo")
load("//rust/private:rustc.bzl", "rustc_compile_action")
load(
"//rust/private:utils.bzl",
"can_build_metadata",
"compute_crate_name",
"crate_root_src",
"create_crate_info_dict",
"dedent",
"determine_output_hash",
"expand_dict_value_locations",
"find_toolchain",
"get_edition",
"get_import_macro_deps",
"transform_deps",
"transform_sources",
)
# TODO(marco): Separate each rule into its own file.

Expand Down Expand Up @@ -66,129 +67,6 @@ def _assert_correct_dep_mapping(ctx):
),
)

def _determine_lib_name(name, crate_type, toolchain, lib_hash = None):
"""See https://github.com/bazelbuild/rules_rust/issues/405
Args:
name (str): The name of the current target
crate_type (str): The `crate_type`
toolchain (rust_toolchain): The current `rust_toolchain`
lib_hash (str, optional): The hashed crate root path
Returns:
str: A unique library name
"""
extension = None
prefix = ""
if crate_type in ("dylib", "cdylib", "proc-macro"):
extension = toolchain.dylib_ext
elif crate_type == "staticlib":
extension = toolchain.staticlib_ext
elif crate_type in ("lib", "rlib"):
# All platforms produce 'rlib' here
extension = ".rlib"
prefix = "lib"
elif crate_type == "bin":
fail("crate_type of 'bin' was detected in a rust_library. Please compile " +
"this crate as a rust_binary instead.")

if not extension:
fail(("Unknown crate_type: {}. If this is a cargo-supported crate type, " +
"please file an issue!").format(crate_type))

prefix = "lib"
if toolchain.target_triple and toolchain.target_os == "windows" and crate_type not in ("lib", "rlib"):
prefix = ""
if toolchain.target_arch == "wasm32" and crate_type == "cdylib":
prefix = ""

return "{prefix}{name}{lib_hash}{extension}".format(
prefix = prefix,
name = name,
lib_hash = "-" + lib_hash if lib_hash else "",
extension = extension,
)

def get_edition(attr, toolchain, label):
"""Returns the Rust edition from either the current rule's attirbutes or the current `rust_toolchain`
Args:
attr (struct): The current rule's attributes
toolchain (rust_toolchain): The `rust_toolchain` for the current target
label (Label): The label of the target being built
Returns:
str: The target Rust edition
"""
if getattr(attr, "edition"):
return attr.edition
elif not toolchain.default_edition:
fail("Attribute `edition` is required for {}.".format(label))
else:
return toolchain.default_edition

def _symlink_for_non_generated_source(ctx, src_file, package_root):
"""Creates and returns a symlink for non-generated source files.
This rule uses the full path to the source files and the rule directory to compute
the relative paths. This is needed, instead of using `short_path`, because of non-generated
source files in external repositories possibly returning relative paths depending on the
current version of Bazel.
Args:
ctx (struct): The current rule's context.
src_file (File): The source file.
package_root (File): The full path to the directory containing the current rule.
Returns:
File: The created symlink if a non-generated file, or the file itself.
"""

if src_file.is_source or src_file.root.path != ctx.bin_dir.path:
src_short_path = paths.relativize(src_file.path, src_file.root.path)
src_symlink = ctx.actions.declare_file(paths.relativize(src_short_path, package_root))
ctx.actions.symlink(
output = src_symlink,
target_file = src_file,
progress_message = "Creating symlink to source file: {}".format(src_file.path),
)
return src_symlink
else:
return src_file

def _transform_sources(ctx, srcs, crate_root):
"""Creates symlinks of the source files if needed.
Rustc assumes that the source files are located next to the crate root.
In case of a mix between generated and non-generated source files, this
we violate this assumption, as part of the sources will be located under
bazel-out/... . In order to allow for targets that contain both generated
and non-generated source files, we generate symlinks for all non-generated
files.
Args:
ctx (struct): The current rule's context.
srcs (List[File]): The sources listed in the `srcs` attribute
crate_root (File): The file specified in the `crate_root` attribute,
if it exists, otherwise None
Returns:
Tuple(List[File], File): The transformed srcs and crate_root
"""
has_generated_sources = len([src for src in srcs if not src.is_source]) > 0

if not has_generated_sources:
return srcs, crate_root

package_root = paths.dirname(paths.join(ctx.label.workspace_root, ctx.build_file_path))
generated_sources = [_symlink_for_non_generated_source(ctx, src, package_root) for src in srcs if src != crate_root]
generated_root = crate_root
if crate_root:
generated_root = _symlink_for_non_generated_source(ctx, crate_root, package_root)
generated_sources.append(generated_root)

return generated_sources, generated_root

def _rust_library_impl(ctx):
"""The implementation of the `rust_library` rule.
Expand Down Expand Up @@ -265,7 +143,7 @@ def _rust_library_common(ctx, crate_type):
crate_root = getattr(ctx.file, "crate_root", None)
if not crate_root:
crate_root = crate_root_src(ctx.attr.name, ctx.files.srcs, crate_type)
srcs, crate_root = _transform_sources(ctx, ctx.files.srcs, crate_root)
_, crate_root = transform_sources(ctx, ctx.files.srcs, crate_root)

# Determine unique hash for this rlib.
# Note that we don't include a hash for `cdylib` and `staticlib` since they are meant to be consumed externally
Expand All @@ -277,49 +155,13 @@ def _rust_library_common(ctx, crate_type):
else:
output_hash = determine_output_hash(crate_root, ctx.label)

crate_name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name)
rust_lib_name = _determine_lib_name(
crate_name,
crate_type,
toolchain,
output_hash,
)
rust_lib = ctx.actions.declare_file(rust_lib_name)

rust_metadata = None
if can_build_metadata(toolchain, ctx, crate_type) and not ctx.attr.disable_pipelining:
rust_metadata = ctx.actions.declare_file(
paths.replace_extension(rust_lib_name, ".rmeta"),
sibling = rust_lib,
)

deps = transform_deps(ctx.attr.deps)
proc_macro_deps = transform_deps(ctx.attr.proc_macro_deps + get_import_macro_deps(ctx))

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 = crate_root,
srcs = depset(srcs),
deps = depset(deps),
proc_macro_deps = depset(proc_macro_deps),
aliases = ctx.attr.aliases,
output = rust_lib,
metadata = rust_metadata,
edition = get_edition(ctx.attr, toolchain, ctx.label),
rustc_env = ctx.attr.rustc_env,
rustc_env_files = ctx.files.rustc_env_files,
is_test = False,
data = depset(ctx.files.data),
compile_data = depset(ctx.files.compile_data),
compile_data_targets = depset(ctx.attr.compile_data),
owner = ctx.label,
),
output_hash = output_hash,
crate_type = crate_type,
create_crate_info_callback = create_crate_info_dict,
)

def _rust_binary_impl(ctx):
Expand All @@ -343,7 +185,7 @@ def _rust_binary_impl(ctx):
crate_root = getattr(ctx.file, "crate_root", None)
if not crate_root:
crate_root = crate_root_src(ctx.attr.name, ctx.files.srcs, ctx.attr.crate_type)
srcs, crate_root = _transform_sources(ctx, ctx.files.srcs, crate_root)
srcs, crate_root = transform_sources(ctx, ctx.files.srcs, crate_root)

return rustc_compile_action(
ctx = ctx,
Expand All @@ -359,6 +201,7 @@ def _rust_binary_impl(ctx):
aliases = ctx.attr.aliases,
output = output,
edition = get_edition(ctx.attr, toolchain, ctx.label),
_rustc_env_attr = ctx.attr.rustc_env,
rustc_env = ctx.attr.rustc_env,
rustc_env_files = ctx.files.rustc_env_files,
is_test = False,
Expand Down Expand Up @@ -399,7 +242,7 @@ def _rust_test_impl(ctx):
),
)

srcs, crate_root = _transform_sources(ctx, ctx.files.srcs, getattr(ctx.file, "crate_root", None))
srcs, crate_root = transform_sources(ctx, ctx.files.srcs, getattr(ctx.file, "crate_root", None))

# Optionally join compile data
if crate.compile_data:
Expand All @@ -411,8 +254,16 @@ def _rust_test_impl(ctx):
else:
compile_data_targets = depset(ctx.attr.compile_data)
rustc_env_files = ctx.files.rustc_env_files + crate.rustc_env_files
rustc_env = dict(crate.rustc_env)
rustc_env.update(**ctx.attr.rustc_env)

rustc_env = dict(crate._rustc_env_attr)

# crate.rustc_env is already expanded upstream in rust_library rule implementation
data_paths = depset(direct = getattr(ctx.attr, "data", [])).to_list()
rustc_env.update(expand_dict_value_locations(
ctx,
ctx.attr.rustc_env,
data_paths,
))

# Build the test binary using the dependency's srcs.
crate_info = rust_common.create_crate_info(
Expand All @@ -426,6 +277,7 @@ def _rust_test_impl(ctx):
output = output,
edition = crate.edition,
rustc_env = rustc_env,
_rustc_env_attr = ctx.attr.rustc_env,
rustc_env_files = rustc_env_files,
is_test = True,
compile_data = compile_data,
Expand All @@ -439,7 +291,7 @@ def _rust_test_impl(ctx):
if not crate_root:
crate_root_type = "lib" if ctx.attr.use_libtest_harness else "bin"
crate_root = crate_root_src(ctx.attr.name, ctx.files.srcs, crate_root_type)
srcs, crate_root = _transform_sources(ctx, ctx.files.srcs, crate_root)
srcs, crate_root = transform_sources(ctx, ctx.files.srcs, crate_root)

output_hash = determine_output_hash(crate_root, ctx.label)
output = ctx.actions.declare_file(
Expand All @@ -450,6 +302,13 @@ def _rust_test_impl(ctx):
),
)

data_paths = depset(direct = getattr(ctx.attr, "data", [])).to_list()
rustc_env = expand_dict_value_locations(
ctx,
ctx.attr.rustc_env,
data_paths,
)

# Target is a standalone crate. Build the test binary as its own crate.
crate_info = rust_common.create_crate_info(
name = compute_crate_name(ctx.workspace_name, ctx.label, toolchain, ctx.attr.crate_name),
Expand All @@ -461,7 +320,8 @@ def _rust_test_impl(ctx):
aliases = ctx.attr.aliases,
output = output,
edition = get_edition(ctx.attr, toolchain, ctx.label),
rustc_env = ctx.attr.rustc_env,
rustc_env = rustc_env,
_rustc_env_attr = ctx.attr.rustc_env,
rustc_env_files = ctx.files.rustc_env_files,
is_test = True,
compile_data = depset(ctx.files.compile_data),
Expand All @@ -475,6 +335,7 @@ def _rust_test_impl(ctx):
toolchain = toolchain,
crate_info = crate_info,
rust_flags = ["--test"] if ctx.attr.use_libtest_harness else ["--cfg", "test"],
skip_expanding_rustc_env = True,
)
data = getattr(ctx.attr, "data", [])

Expand Down
2 changes: 1 addition & 1 deletion rust/private/rust_analyzer.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def _rust_analyzer_aspect_impl(target, ctx):
rust_analyzer_info = RustAnalyzerInfo(
crate = crate_info,
cfgs = cfgs,
env = getattr(ctx.rule.attr, "rustc_env", {}),
env = crate_info.rustc_env,
deps = dep_infos,
crate_specs = depset(direct = [crate_spec], transitive = [dep.crate_specs for dep in dep_infos]),
proc_macro_dylib_path = find_proc_macro_dylib_path(toolchain, target),
Expand Down
Loading

0 comments on commit 8b548d2

Please sign in to comment.