bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.21k stars 4.07k forks source link

Breaking change in Bazel CQuery due to Starlarkification of providers #20473

Open guw opened 11 months ago

guw commented 11 months ago

Description of the bug:

The following cquery no longer works with Bazel 7:

bazel cquery "@bazel_tools//tools/jdk:current_java_toolchain" --output starlark --starlark:expr 'providers(target)["JavaToolchainInfo"].source_version'

It looks like that providers(target)["JavaToolchainInfo"] needs to be replaced with providers(target)['@@_builtins//:common/java/java_toolchain.bzl%JavaToolchainInfo'].source_version but it feels odd to require this verbosity now. Especially the label syntax looks subjective to further changes. It should be possible to select a provider without relying on the location.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

echo "7.0.0.rc5" > .bazelversion

run the query

Have you found anything relevant by searching the web?

https://bazelbuild.slack.com/archives/CDCE4DFAP/p1702038149142419?thread_ts=1701990164.906629&cid=CDCE4DFAP

Any other information, logs, or outputs that you want to share?

This needs to be documented. Ideally all those queries would continue to work, though.

meisterT commented 11 months ago

cc @gregestren

meisterT commented 11 months ago

@guw did you already bisect which change caused this change in behavior?

meteorcloudy commented 11 months ago

Maybe this is just some breaking change after we starlarkifying JavaToolchainInfo? @comius

meteorcloudy commented 11 months ago

https://github.com/bazelbuild/bazel/commit/8eecb297f965aa4ae0df732f5de4daac265f83fd /cc @hvadehra

hvadehra commented 11 months ago

Yes, this will be a side-effect of moving native providers to Starlark.

The providers() function uses the Starlark provider's key.

Not sure anything else would make sense for identifying a provider written in Starlark. So we should just add this to the release notes as a breaking change.

meteorcloudy commented 11 months ago

Thanks for confirming!

So we should just add this to the release notes as a breaking change.

Please do!

guw commented 11 months ago

Not sure anything else would make sense for identifying a provider written in Starlark. So we should just add this to the release notes as a breaking change.

I think that would be super helpful (although inconvenient). Would it be possible to provide a different function/way?

comius commented 11 months ago

I’m not happy with this, but this is WAI and will change at least one more time when the rules are moved out of builtins.

Making it same as before would require adding hacks to the implementation.

Any bzl file that needs a provider, has to import it via a load. This is also going to change from a global symbol for even JavaInfo to a load from rules_java.

Starlark query expressions are the only location where a provider can’t be loaded and you refer to it by a string. For correctness, also bzl file needs to be in the string (otherwise the map might not have unique keys).

Possible workaround in user space is to either use both strings that represent the provider. Or maybe even filter the map for keys that end with JavaTolchainInfo

gregestren commented 11 months ago

@gu2 can you think of any clean user experience that wouldn't have the ambiguity of filtering the map? (i.e. risk of resolving to the wrong JavaToolchainInfo if not fully qualified)?

guw commented 11 months ago

It depends on how real ambiguity is @gregestren. It would love to have a find_provider(target, "JavaToolchainInfo") helper that fails if there is none or multiple. Still trying to wrap my head around how to do this in a single line.

Perhaps there is a better way. The full query I am doing is:

bazel cquery --output starlark --starlark:expr "providers(target)['JavaToolchainInfo'].source_version + '::' + providers(target)['JavaToolchainInfo'].target_version + '::' + providers(target)['JavaToolchainInfo'].java_runtime.java_home" @bazel_tools//tools/jdk:current_java_toolchain

I would have to do the filtering three times in a single line. I don't think --starlark:expr allows assignments. I haven't it figured out yet.

All I want is identify JAVA_HOME and source and target version so I can setup the IDE properly. Perhaps there is a better way to query this?

hvadehra commented 11 months ago

I would have to do the filtering three times in a single line. I don't think --starlark:expr allows assignments. I haven't it figured out yet.

Is it necessary for the starlark code to be specified on the command line? If not, you could use --starlark:file instead. There are examples in the docs.

guw commented 11 months ago

Oh, that looks doable.

BTW, that example doc needs fixing as well:

providers(target) returns a map whose keys are names of providers (for example, "DefaultInfo").

It should mention the specifics around keys instability from Bazel version to version.

guw commented 11 months ago

Thanks @hvadehra. Using --starlark:file works much better for this.

tpasternak commented 10 months ago

so the issue remains open, does it mean it will be fxied at some point, or we should treat @@_builtins//:common/java/java_info.bzl%JavaInfo provider name as the official way to get it in queries?

hvadehra commented 10 months ago

we should treat @@_builtins//:common/java/java_info.bzl%JavaInfo provider name as the official way to get it in queries

Yes, for now that will the key. As noted in https://github.com/bazelbuild/bazel/issues/20473#issuecomment-1847422010 it will change (at least) once more when the java rules move out of @builtins and into @rules_java (likely in Bazel 8). Given this issue, we should keep the provider location(s) unchanged after that point in time.