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

execution_requirements keys are filtered like tags #24616

Open
dws opened this issue Dec 10, 2024 · 2 comments
Open

execution_requirements keys are filtered like tags #24616

dws opened this issue Dec 10, 2024 · 2 comments
Labels
team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: bug untriaged

Comments

@dws
Copy link
Contributor

dws commented Dec 10, 2024

Description of the bug:

The implementation of --incompatible_allow_tags_propagation deliberately only propagates certain tags to the ExecutionInfo of an action. The filtering on tags appears to be accomplished via legalExecInfoKeys() in src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java.

The surprise is that the keys of the execution_requirements dict parameter to ctx.actions.run() and cts.actions.run_shell() are also filtered in this way.

By contrast, the keys of the implicit rule attribute exec_properties are not filtered.

I can understand wanting to limit what tags propagate into ExecutionInfo. But since execution_requirements is explicitly there only for augmenting ExecutionInfo it is most surprising and counterintuitive that its keys would be filtered, especially because the keys of exec_properties are not filtered.

Which category does this issue belong to?

Core

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

With the following BUILD.bazel:

load(":repro.bzl", "repro_run", "repro_run_shell")

# The tags are filtered by legalExecInfoKeys(), so only
# "supports-repro-tag" makes it into ExecutionInfo, as intended.

REPRO_TAGS = [
    "RENEGADE-repro-tag",       # does not satisfy legalExecInfoKeys()
    "supports-repro-tag",       # satisfies legalExecInfoKeys()
]

# Both entries in this exec_properties dict make it into ExecutionInfo,
# as expected.

REPRO_EXEC_PROPERTIES = {
    "RENEGADE-repro-exec-property": "", # does not satisfy legalExecInfoKeys()
    "supports-repro-exec-property": "", # satisfies legalExecInfoKeys()
}

# The surprise is that the keys of this execution_requirements
# dict get filtered by legalExecInfoKeys(), so only
# "supports-repro-execution-requirement" makes it into ExecutionInfo.

REPRO_EXECUTION_REQUIREMENTS = {
    "RENEGADE-repro-execution-requirement": "", # does not satisfy legalExecInfoKeys()
    "supports-repro-execution-requirement": "", # satisfies legalExecInfoKeys()
}

repro_run(
    name = "repro-run",
    out = "repro-run.out",
    tags = REPRO_TAGS,
    exec_properties = REPRO_EXEC_PROPERTIES,
    execution_requirements = REPRO_EXECUTION_REQUIREMENTS,
)

repro_run_shell(
    name = "repro-run-shell",
    out = "repro-run-shell.out",
    tags = REPRO_TAGS,
    exec_properties = REPRO_EXEC_PROPERTIES,
    execution_requirements = REPRO_EXECUTION_REQUIREMENTS,
)

and the following repro.bzl:

def _repro_run_impl(ctx):

    output = ctx.outputs.out

    ctx.actions.run(
        outputs = [output],
        executable = "/usr/bin/touch",
        arguments = [output.path],
        execution_requirements = ctx.attr.execution_requirements,
    )

    return DefaultInfo(files = depset([output]))

repro_run = rule(
    implementation = _repro_run_impl,
    attrs = {
        "out": attr.output(),
        "execution_requirements": attr.string_dict(),
    },
)

def _repro_run_shell_impl(ctx):

    output = ctx.outputs.out

    ctx.actions.run_shell(
        outputs = [output],
        command = "/usr/bin/touch " + output.path,
        execution_requirements = ctx.attr.execution_requirements,
    )

    return DefaultInfo(files = depset([output]))


repro_run_shell = rule(
    implementation = _repro_run_shell_impl,
    attrs = {
        "out": attr.output(),
        "execution_requirements": attr.string_dict(),
    },
)

and an empty MODULE.bazel file, I see the following:

$ bazel --quiet aquery //:all | grep ExecutionInfo
  ExecutionInfo: {RENEGADE-repro-exec-property: '', supports-repro-exec-property: '', supports-repro-execution-requirement: '', supports-repro-tag: ''}
  ExecutionInfo: {RENEGADE-repro-exec-property: '', supports-repro-exec-property: '', supports-repro-execution-requirement: '', supports-repro-tag: ''}

but I would expect to see the following:

$ bazel --quiet aquery //:all | grep ExecutionInfo
  ExecutionInfo: {RENEGADE-repro-exec-property: '', RENEGADE-repro-execution-requirement: '', supports-repro-exec-property: '', supports-repro-execution-requirement: '', supports-repro-tag: ''}
  ExecutionInfo: {RENEGADE-repro-exec-property: '', RENEGADE-repro-execution-requirement: '', supports-repro-exec-property: '', supports-repro-execution-requirement: '', supports-repro-tag: ''}

In other words, I would expect to see legalExecInfoKeys() apply only to
tags, and not to the keys of the execution_requirements parameter to
ctx.actions.run() or ctx.actions.run_shell().

Which operating system are you running Bazel on?

Ubuntu 20.04

What is the output of bazel info release?

release 8.0.0

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

No response

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

No response

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

This also occurs in Bazel 7.2.1. I did not try to see just how far back this behavior goes.

Have you found anything relevant by searching the web?

No

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

No response

@github-actions github-actions bot added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Dec 10, 2024
@fmeum
Copy link
Collaborator

fmeum commented Dec 10, 2024

Does this also happen with the propagation flag unflipped?

@dws
Copy link
Contributor Author

dws commented Dec 10, 2024

Does this also happen with the propagation flag unflipped?

Yes.

With --noincompatible_allow_tags_propagation the tags no longer get added to the ExecutionInfo, but the execution_requirements dict keys are still filtered.

$ ../bazel-8.0.0-linux-x86_64 --quiet aquery --noincompatible_allow_tags_propagation //:all | grep ExecutionInfo
  ExecutionInfo: {RENEGADE-repro-exec-property: '', supports-repro-exec-property: '', supports-repro-execution-requirement: ''}
  ExecutionInfo: {RENEGADE-repro-exec-property: '', supports-repro-exec-property: '', supports-repro-execution-requirement: ''}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Core Skyframe, bazel query, BEP, options parsing, bazelrc type: bug untriaged
Projects
None yet
Development

No branches or pull requests

5 participants