Bazel query is broken for Rust targets

[csmulhern]

bazel version: 7.0.0
rules_rust version: 0.36.1

Sample: example.zip

To reproduce, run:

bazel query "deps(//:main)"

Output:

ERROR: /private/var/tmp/_bazel_cameron/bd7f4772f50608f24ec4c16fe5369dcc/external/rules_rust/util/import/3rdparty/crates/BUILD.bazel:52:6: no such package '@@rules_rust_util_import__quote-1.0.10//': The repository '@@rules_rust_util_import__quote-1.0.10' could not be resolved: Repository '@@rules_rust_util_import__quote-1.0.10' is not defined and referenced by '@@rules_rust//util/import/3rdparty/crates:quote'
ERROR: Evaluation of query "deps(//:main)" failed: preloading transitive closure failed: no such package '@@rules_rust_util_import__quote-1.0.10//': The repository '@@rules_rust_util_import__quote-1.0.10' could not be resolved: Repository '@@rules_rust_util_import__quote-1.0.10' is not defined

It looks like there is an implicit dependency on @rules_rust/util/import for all rust rules, but these dependencies are not included through rules_rust_dependencies.

I'm guessing this dependency is coming from:

rules_rust/rust/private/rust.bzl

Lines 531 to 533 in e6a1ee4

	"_import_macro_dep": attr.label(
	default = Label("//util/import"),
	cfg = "exec",

The simplest fix seems like it would be to add import_deps to rules_rust_dependencies. However, I'm also wondering if it's really necessary for all rules to depend on @rules_rust/util/import.

This issue prevents usage of bazel query with rust targets, and breaks workflows that depend on bazel query, e.g. ibazel.

[illicitonion]

It looks like Bazel 7 made label_flag more eager during query: bazelbuild/bazel@658b3a6

cc @cfredric @hlopko as authors of the feature

cc @gregestren - it looks like rules_rust was depending on not reading through label_flag to avoiding dependencies needing to be set up for a niche use-case - do you know of any handy workarounds?

[scentini]

@gregestren Could you please advise what to do here?

[katre]

It looks like the standard attribute _import_macro_dep is a label-valued attribute, which points to //util/import, which is an alias with a select:

alias(
    name = "import",
    actual = select({
        ":use_fake_import_macro": ":fake_import_macro_impl",
        "//conditions:default": ":import_macro_label",
    }),
    visibility = ["//visibility:public"],
)

The long-standing behavior of select and query has been to overapproxamate the dependencies by including all of them. This is where the label_flag arrives: as the value of the select.

Why are you using an alias/select and a label_flag? Would a custom flag be more useful here?

You're correct: we changed how label_flag works with query, based on the same idea of overapproxamation from the select handling. It looks like this was associated with an internal bug, but the core problem was that increased use of label-typed flags led to failures in dependency analysis with the old behavior, which caused some breakages due to tests not being run when they should have been (and when they would have shown failures).

[illicitonion]

That all makes sense in terms of how we got here, and I don't disagree with the behaviour of select and query here in general :)

The background here in terms of rules_rust is that Google effectively wanted to add an optional implicit dependency to all of its rust code (#1008), and there hasn't historically been a very nice way of doing so. Macros are hard because you can't force other rulesets to use them. Possibly the extensible rules stuff Ivo is working on could help, though still has the "how do you force other rulesets to use it" problem.

That implicit dependency, unfortunately, requires additional repositories to be registered in the workspace. We ideally don't want to do this automatically for the 99% of non-Google users who don't use this feature (and in a pre-bzlmod world it may be hard to do so, as it would possibly require our users add a second initialisation function to their WORKSPACE file, as that repository would depend on symbols we initialise in the function they already call).

This broke query because we didn't register these repositories by default, and @hlopko found the cute workaround that was in place.

So I guess the question is: How should Google be adding this dependency for their own code and not impact anyone else? Is there a reasonable way for them to extend the rules (or will there be soon)? Is there a way to register a dependency which doesn't show up to query unless it's enabled (maybe we could do something ugly like add this target as a dependency of a toolchain and either use an optional toolchain to enable it or have Google enable a custom toolchain which includes the dep where the default toolchain wouldn't)? Or any other cute ideas?

[katre]

The best ways, in rough order:

1. Use rule extensions inside google to add google-only functionality (and make sure that google code only uses those extended rules).
2. Use a new toolchain type which is optional for the new functionality (and gate the code that calls it behind a check for whether the toolchain exists, but this adds complexity to the open source rules).
   1. The benefit to this approach is that any motivated rust user could use the functionality, not just google. I don't actually know what the internal google team is doing here.

[scentini]

I'm happy to try rule extensions if they are suitable to solve this problem; @comius are they in good enough shape to be reliably depended upon? Could you point me to documentation/example usage?

[gregestren]

I didn't read through in detail, but could

rules_rust/rust/private/rust.bzl

Lines 531 to 533 in e6a1ee4

	"_import_macro_dep": attr.label(
	default = Label("//util/import"),
	cfg = "exec",

be replaced by a direct dependency on a label_flag that defaults to a trivial no-op label (which is what query would follow)? But Google could custom-route to its own special dependency by setting the flag differently?

The current selection is on

rules_rust/util/import/BUILD.bazel

Line 72 in e6a1ee4

"@rules_rust//rust/settings:use_real_import_macro": "False",

Can whatever sets that boolean to True just directly set the label flag?

[scentini]

I talked about this issue to @krasimirgg and @hlopko. We may have a need for having the import macro in OSS in the future, at which point we'll need to revisit this again; but for now it's okay to replace the import macro with a dummy in rules_rust and use the real import macro in Google's monorepo.
I think @katre's option #2 is worth giving a shot, unfortunately I don't think we'll have the cycles for it anytime soon, and this does look like a blocker for adopting Bazel 7.0.

[jfirebaugh]

It wasn't immediately clear to me from the discussion above, so for anyone else who may be hitting this issue when trying to upgrade to Bazel 7: it's fixed in 0.38.0.