Skip to content
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

The query language lacks a way to query rules that are target_compatible_with #12917

Closed
luna-duclos opened this issue Jan 28, 2021 · 9 comments
Closed
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request

Comments

@luna-duclos
Copy link

luna-duclos commented Jan 28, 2021

Description of the problem / feature request:

The query language lacks a way to express "I want to query all targets that are compatible with a certain platform. This is because even though you can use attr() with cquery to ask for values of the attribute, thats not good enough as the attribute is transitive and dependencies might be incompatible, causing the target to still be incompatible even though it has no attribute itself.

We had to work around this by manually running over the proto output of cquery and checking the target_compatible_with attribute of a rule and its dependencies manually in Go code.

The query language should have a function that allows one to query if a rule is compatible, something along the lines of compatible_with(<rule>, <constraint>), with compatible_with(<rule>) defaulting to @local_config_platform//:host as constraint.

Feature requests: what underlying problem are you trying to solve with this feature?

I'm trying to avoid needing to manually run over the analysis proto manually, which is slow, error prone, easy to get wrong and also restricts my ability to meaningfully use other operators alongside

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

This is a feature request, any use of target_compatible_with in a dependency and then trying to cquery compatible rules in any way will stumble on this.

What operating system are you running Bazel on?

We use bazel on MacOS and Linux.

What's the output of bazel info release?

release 4.0.0

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

N/A

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

N/A

Have you found anything relevant by searching the web?

I've discussed this on the bazel slack and was told I should likely log a feature request

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

@philsc
Copy link
Contributor

philsc commented Jan 28, 2021

@gregestren , @katre

I've not thought about this, but I think it's a good idea to solve this. If you build up your CI target list from a query, there should be a way to filter those out.

@gregestren
Copy link
Contributor

gregestren commented Jan 28, 2021

cquery has new functionality that can query over providers, which incompatible target skipping uses to honor these transitive effects. Let me look up the docs on that.

Caveat is it only works on Starlark-readable providers, which incompatible target skipping isn't yet.

@gregestren
Copy link
Contributor

@gregestren gregestren added team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request untriaged labels Jan 28, 2021
@philsc
Copy link
Contributor

philsc commented Jan 28, 2021

I will experiment with exposing the provider to Starlark and see if cquery can filter those targets out.

@philsc
Copy link
Contributor

philsc commented Jan 28, 2021

That's a cool idea @gregestren , thanks!

@philsc
Copy link
Contributor

philsc commented Jan 29, 2021

I think this could work pretty well:
philsc@3d0c665#diff-a982fd7588855b07d791b1b297ffc662b316c80313943ec6998af93edb331c5e

Re-posting the relevant snippet here for simplicity:

$ cat > compatibility.cquery <<EOF
def format(target):
    if "IncompatiblePlatformProvider" in providers(target):
        result = "incompatible"
    else:
        result = "compatible"
    return "%s is %s" % (target.label, result)
EOF
$ bazel cquery :all  --output=starlark --starlark:file=compatibility.cquery

@luna-duclos , would that serve your use case? For incompatible targets you can return an empty string so it wouldn't print anything.

@luna-duclos
Copy link
Author

It would!

@philsc
Copy link
Contributor

philsc commented Jan 29, 2021

Cool. I'll get that turned into a pull request then.

@gregestren gregestren added P1 I'll work on this now. (Assignee required) and removed untriaged labels Jan 29, 2021
@gregestren
Copy link
Contributor

Happy to review promptly.

philwo pushed a commit that referenced this issue Mar 15, 2021
This patch exposes the `IncompatiblePlatformProvider` to Starlark just
enough that it can be used with `cquery`'s `platforms()` function. I
added an example of this to the documentation. The motivation here is
to let users filter out incompatible targets from queries that
provide things like the list of targets to build for CI.

This patch is minimal on purpose. It does not allow users to
instantiate an `IncompatiblePlatformProvider` in Starlark. This may be
added in a future patch to address a different use case.

Fixes #12917.

Closes #12935.

PiperOrigin-RevId: 356580943
philwo pushed a commit that referenced this issue Mar 15, 2021
This patch exposes the `IncompatiblePlatformProvider` to Starlark just
enough that it can be used with `cquery`'s `platforms()` function. I
added an example of this to the documentation. The motivation here is
to let users filter out incompatible targets from queries that
provide things like the list of targets to build for CI.

This patch is minimal on purpose. It does not allow users to
instantiate an `IncompatiblePlatformProvider` in Starlark. This may be
added in a future patch to address a different use case.

Fixes #12917.

Closes #12935.

PiperOrigin-RevId: 356580943
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants