-
Notifications
You must be signed in to change notification settings - Fork 402
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
Refactor rustc_compile_action #330
Conversation
Functionally, this code should be identical. However, it (hopefully) is easier to read, and makes subcomponents of the rustc command (parsing args, env, etc) easier to generalize.
# Make bin crate data deps available to tests. | ||
for data in getattr(ctx.attr, "data", []): | ||
if CrateInfo in data: | ||
dep_crate_info = data[CrateInfo] | ||
if dep_crate_info.type == "bin": | ||
env["CARGO_BIN_EXE_" + dep_crate_info.output.basename] = dep_crate_info.output.short_path | ||
|
||
# Update environment with user provided variables. | ||
env.update(crate_info.rustc_env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulled up into _rustc_compile_arguments
cc_toolchain = find_cpp_toolchain(ctx) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was unused, and removed.
# We awkwardly construct this command because we cannot reference $PWD from ctx.actions.run(executable=toolchain.rustc) | ||
out_dir = _create_out_dir_action(ctx, build_info.out_dir if build_info else None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulled up into _rustc_compile_inputs
rust/private/rustc.bzl
Outdated
toolchain, | ||
) | ||
|
||
compile_inputs, out_dir = _rustc_compile_inputs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think each of these subcommands should start w/ verbs, eg.
collect_deps
collect_inputs
construct_arguments
etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Functionally, this code should be identical.
However, it (hopefully) is easier to read, and makes
subcomponents of the rustc command (parsing args, env, etc)
easier to generalize.