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

Tags to execution requirements propagation in Starlark #7766

Closed
buchgr opened this issue Mar 19, 2019 · 5 comments
Closed

Tags to execution requirements propagation in Starlark #7766

buchgr opened this issue Mar 19, 2019 · 5 comments
Assignees
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@buchgr
Copy link
Contributor

buchgr commented Mar 19, 2019

Every target in Bazel supports specifying a tags attribute:

cc_binary(
  ...
  tags = ["local"]
)

While tags can be used for a variety of use cases (i.e. query), Bazel also defines a set of tags that have special meaning for how it executes an action. For example, no-sandbox tells Bazel to run this action without a sandboxing strategy and no-remote tells it to not run the action on a remote execution system.

The issue is that a user sets tags on a target and not on an action. So a rule implementation needs to intentionally pass the tags forward to every action it creates (as an execution requirement).

def _impl(ctx):
    ctx.actions.run(
       ...
       execution_requirements = {tag:"" for tag in ctx.attr.tags.keys()},
    )

foo = rule(
    implementation = _impl,
    attrs = {
      ...
    },
)

If a rule doesn't add execution_requirements = ctx.attr.tags.keys() or similar to each action it creates then a user tagging a target as no-remote will have no effect for execution.

We need to answer the following questions:

  • Is it desirable for Starlark rules to by default implicitly pass along a defined set of tags (i.e. the ones supported by Bazel) as an execution requirement to every action?
  • Would enabling this by default be a backwards incompatible change?
  • What are the semantics when a rule also defines their own execution requirements?

cc: @laurentlb @cparsons @nlopezgi @agoulti @jmmv

@buchgr buchgr added type: feature request team-Remote-Exec Issues and PRs for the Execution (Remote) team labels Mar 19, 2019
@benjaminp
Copy link
Collaborator

An interesting special case today is that ctx.actions.run_shell will copy the rule tags over into the execution requirements iff you pass a string command.

@ishikhman
Copy link
Contributor

Design doc link

ishikhman pushed a commit to ishikhman/bazel that referenced this issue Jun 12, 2019
Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see bazelbuild#7766 for a link), set of tags to be propagated to actions as a first iteration.
This change is responsible for that first step for the Starlark Rules.

RELNOTES: tags 'no-remote', 'no-cache', 'no-remote-cache', 'no-remote-exec', 'no-sandbox' are propagated now to the actions from targets.

Closes bazelbuild#7766
bazel-io pushed a commit that referenced this issue Jul 4, 2019
*** Reason for rollback ***

Breaks internal tests

*** Original change description ***

tags propagation: Starlark rules part

Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see [doc](https://docs.google.com/document/d/1X2GtuuNT6UqYYOK5lJWQEdPjAgsbdB3nFjjmjso-XHo/edit#heading=h.5mcn15i0e1ch) and #7766 for details), set o...

***

RELNOTES: None.
PiperOrigin-RevId: 256561364
siberex pushed a commit to siberex/bazel that referenced this issue Jul 4, 2019
Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see [doc](https://docs.google.com/document/d/1X2GtuuNT6UqYYOK5lJWQEdPjAgsbdB3nFjjmjso-XHo/edit#heading=h.5mcn15i0e1ch) and bazelbuild#7766 for details), set of tags to be propagated to actions as a first iteration.
This change is responsible for that first step for the Starlark Rules.

RELNOTES: tags 'no-remote', 'no-cache', 'no-remote-cache', 'no-remote-exec', 'no-sandbox' are propagated now to the actions from targets.

Closes bazelbuild#7766

Closes bazelbuild#8612.

PiperOrigin-RevId: 256369636
siberex pushed a commit to siberex/bazel that referenced this issue Jul 4, 2019
*** Reason for rollback ***

Breaks internal tests

*** Original change description ***

tags propagation: Starlark rules part

Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see [doc](https://docs.google.com/document/d/1X2GtuuNT6UqYYOK5lJWQEdPjAgsbdB3nFjjmjso-XHo/edit#heading=h.5mcn15i0e1ch) and bazelbuild#7766 for details), set o...

***

RELNOTES: None.
PiperOrigin-RevId: 256561364
ishikhman pushed a commit to ishikhman/bazel that referenced this issue Jul 9, 2019
Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see bazelbuild#7766 for a link), set of tags to be propagated to actions as a first iteration.
This change is responsible for that first step for the Starlark Rules.

RELNOTES: tags 'no-remote', 'no-cache', 'no-remote-cache', 'no-remote-exec', 'no-sandbox' are propagated now to the actions from targets when '--incompatible_allow_tags_propagation' flag set to true.

Closes bazelbuild#7766
@ishikhman
Copy link
Contributor

was rolled back

@ishikhman ishikhman reopened this Jul 10, 2019
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see [doc](https://docs.google.com/document/d/1X2GtuuNT6UqYYOK5lJWQEdPjAgsbdB3nFjjmjso-XHo/edit#heading=h.5mcn15i0e1ch) and bazelbuild#7766 for details), set of tags to be propagated to actions as a first iteration.
This change is responsible for that first step for the Starlark Rules.

RELNOTES: tags 'no-remote', 'no-cache', 'no-remote-cache', 'no-remote-exec', 'no-sandbox' are propagated now to the actions from targets.

Closes bazelbuild#7766

Closes bazelbuild#8612.

PiperOrigin-RevId: 256369636
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
*** Reason for rollback ***

Breaks internal tests

*** Original change description ***

tags propagation: Starlark rules part

Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see [doc](https://docs.google.com/document/d/1X2GtuuNT6UqYYOK5lJWQEdPjAgsbdB3nFjjmjso-XHo/edit#heading=h.5mcn15i0e1ch) and bazelbuild#7766 for details), set o...

***

RELNOTES: None.
PiperOrigin-RevId: 256561364
@arlyon
Copy link

arlyon commented Jul 23, 2019

@ishikhman is this change applied or is this currently still in progress? It's not clear if it has been rolled back or not.

@ishikhman
Copy link
Contributor

ishikhman commented Jul 23, 2019

@arlyon it will be included into our next release, 0.29, which will happen in the beginning of August.
The change is already in the HEAD though, feel free ti try it out!
Just add a flag --incompatible_allow_tags_propagation to your build.

Note1: flag will be flipped to be true by default in September's release.
Note2: at the moment the change affects only Starlark rules, change for the native rules is coming.
See #8830 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
4 participants