Skip to content

Add a conditional implicit dependency on the import macro. #1151

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

Merged
merged 3 commits into from
Feb 22, 2022

Conversation

cfredric
Copy link
Contributor

This is related to #1008 and #1013. This PR aims to improve the UX of the import macro by not requiring every target to explicitly depend on the macro.

This PR adds an implicit attribute to every rust_library and rust_binary rule, which (if the use_real_import_macro flag is enabled) adds the import macro crate to the target's proc_macro_deps. If the use_real_import_macro flag is disabled (which is the default), the implicit attribute points to a no-op shell script, and nothing is added to the target's dependencies.

@hlopko hlopko self-requested a review February 22, 2022 08:49
@@ -399,6 +399,21 @@ def transform_deps(deps):
cc_info = dep[CcInfo] if CcInfo in dep else None,
) for dep in deps]

def create_import_macro_dep(ctx):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: get_import_macro_deps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

def _with_import_macro_bootstrapping_mode_impl(ctx):
target = ctx.attr.target[0]
providers = [target[rust_common.crate_info], target[rust_common.dep_info]]
if hasattr(ctx, "executable") and hasattr(ctx.executable, "target"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remind me why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, this was a holdover from the previous attempt at this rule, which supported executable targets. No longer necessary since this is only used for lib targets.

@hlopko hlopko merged commit 3db49e8 into bazelbuild:main Feb 22, 2022
@cfredric cfredric deleted the implicit_dep branch April 20, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants