Improve proc_macro_deps ergonomics, take 3

[dzbarsky]

We can stop forcing users to sort their deps into deps and proc_macro_deps; instead all deps are supplied to deps in the public macros and the rules sort it out.

Best reviewed commit by commit.

I kept the proc_macro_deps attribute named the same just in case, but it would be nice to rename to exec_configured_deps at some point to reflect reality.

Fixes #2893

[dzbarsky]

cleaned up provider references while I was here and discovered dep_info was cheking for the CrateInfo provider but using DepInfo one :)

[thesayyn]

I love this.

[UebelAndre]

How does this handle transitioning proc_macro_deps to exec?

rules_rust/rust/private/rust.bzl

Lines 726 to 736 in 12762a7

	# Previously `proc_macro_deps` were a part of `deps`, and then proc_macro_host_transition was
	# used into cfg="host" using `@local_config_platform//:host`.
	# This fails for remote execution, which needs cfg="exec", and there isn't anything like
	# `@local_config_platform//:exec` exposed.
	"proc_macro_deps": attr.label_list(
	doc = dedent("""\
	List of `rust_proc_macro` targets used to help build this library target.
	"""),
	cfg = "exec",
	providers = [rust_common.crate_info],
	),

[dzbarsky]

How does this handle transitioning proc_macro_deps to exec?

rules_rust/rust/private/rust.bzl

Lines 726 to 736 in 12762a7

	# Previously `proc_macro_deps` were a part of `deps`, and then proc_macro_host_transition was
	# used into cfg="host" using `@local_config_platform//:host`.
	# This fails for remote execution, which needs cfg="exec", and there isn't anything like
	# `@local_config_platform//:exec` exposed.
	"proc_macro_deps": attr.label_list(
	doc = dedent("""\
	List of `rust_proc_macro` targets used to help build this library target.
	"""),
	cfg = "exec",
	providers = [rust_common.crate_info],
	),

That line you linked is still there, that's how it works :). Basically the wrapper macro sends all deps to both dep (target configured) and proc_macro_deps (exec configured) on the underlying rules, and then 'filter_deps' discards the unnecessary ones.

The first commit is the best one to look at, the rest are fairly mechanical

[UebelAndre]

That line you linked is still there, that's how it works :). Basically the wrapper macro sends all deps to both dep (target configured) and proc_macro_deps (exec configured) on the underlying rules, and then 'filter_deps' discards the unnecessary ones.

The first commit is the best one to look at, the rest are fairly mechanical

Ah, I see now. This directionally seems wrong (even though I see the ergonomics of it). Will this impact query and cquery results? What's the impact on analysis to be duplicating dependencies? And what inspired this? Is it actually an ergonomics issue?

[dzbarsky]

Will this impact query and cquery results?

query results are unaffected, cquery will show the deps in both configs now. That does seem a tad bit unfortunate - if we think it's a blocker perhaps the wrapper macro could only perform the expansion logic if an extra param (defaulted to False) were passed, and otherwise was a no-op passthrough. Would that be more palatable?

What's the impact on analysis to be duplicating dependencies?

I wasn't able to measure any significant difference in our repo; there is very minimal processing of the extra deps and rules_rust already does a ton of expensive analysis work so it's lost in the noise

And what inspired this? Is it actually an ergonomics issue?

I'm prototyping some cargo-related repository rules / module extensions and it's hard to emit the deps for crates based on metadata sources at module extension time, since we have no way of knowing whether a crate is a proc-macro until we download the crate source. With this PR, we are able to use the metadata from the crates registry to lay out the dependency tree and defer the decision of whether a given crate generates a rust_library or a rust_proc_macro until the crate's own repository rule runs. Without it, we were maintaining an allowlist , which I'd classify as unergonomic :)

https://bazelbuild.slack.com/archives/CSV56UT0F/p1726290118173529 also brought up the point that needing to treat proc_macros specially also complicated BUILD generation logic

This directionally seems wrong

Could you elaborate more on this? To me it feels good to not force every consumer to categorize deps into the right slots, after all we allow CcInfo-carrying targets into deps instead of forcing them to be specified as cdeps. cc @fmeum as he suggested this approach