Allow rustc_compile_action to threat all dependencies as direct depenencies
#970
Conversation
rust/private/rustc.bzl
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
Did you mean --extern (both here and in the PR description)?
rust/private/rustc.bzl
Outdated
| @@ -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): | |||
There was a problem hiding this comment.
Wdyt about force_all_deps_direct?
rust/private/rustc.bzl
Outdated
| map_each = _crate_to_link_flag, | ||
| ) | ||
| else: | ||
| # nb. Crates are linked via --extern regardless of their crate_type |
There was a problem hiding this comment.
while at it, would you mind changing the comment to say "Direct crates are linked via --extern...)? At its current form it's a bit misleading considering the previous block.
rust/private/rustc.bzl
Outdated
|
|
||
| Returns: | ||
| list: Link flags for the current crate info | ||
| list: Link flags for the given crate_info |
There was a problem hiding this comment.
Nit: do you want to mention AliasableDepInfo in the return documentation too? Or just say "for the given provider".
rust/rust_common.bzl
Outdated
| DepInfo = _DepInfo | ||
| DepVariantInfo = _DepVariantInfo | ||
|
|
||
| rust_common = struct( |
There was a problem hiding this comment.
This is not really necessary for this PR, is it?
| @@ -0,0 +1,92 @@ | |||
| """A custom rule that threats all its dependencies as direct dependencies.""" | |||
|
|
|||
| load( | |||
There was a problem hiding this comment.
Feel free to add # buildifier: disable=bzl-visibility above the problematic load to silence the buildifier.
| action = tut.actions[1] | ||
| argv = action.argv | ||
| assert_action_mnemonic(env, action, "Rustc") | ||
| assert_argv_contains_prefix( |
There was a problem hiding this comment.
Nit: Add assertion checking that there is no -Ldependency flag emitted.
There was a problem hiding this comment.
We actually need the -Ldependency when resolving imports.
A code generator may not know which dependencies are direct and which transitive. When the new
force_only_direct_depsparameter is set toTrue, the commandline will contain--extern=crate_name=...for all the transtive dependencies too.