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

use dedicated exec group for link actions #3711

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sluongng
Copy link
Contributor

When remote execution is used, a user may want to use different
exec properties for different actions under the same target.

For example:

A go_test could include 3 different actions:

  • GoCompilePkg
  • GoLink
  • GoTest

If users wish to use a special exec_properties for GoLink actions,
after this PR, they could do so with:

go_test(
  ...
  exec_properties = {
    # Default
    "container-image": "docker://alpine",
    "EstimatedMemory": "500MB",

    # Only applied to GoLink exec_group
    "GoLink.container-image": "docker://ubuntu",
    "GoLink.EstimatedMemory": "2GB",
  }
  ...
)

Reference: https://bazel.build/extending/exec-groups

What type of PR is this?

Uncomment one line below and remove others.

Bug fix
Feature
Documentation
Other

What does this PR do? Why is it needed?

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@sluongng sluongng force-pushed the sluongng/link-exec-group branch 5 times, most recently from aeaba9e to 700170b Compare September 26, 2023 08:36
When remote execution is used, a user may want to use different
exec properties for different actions under the same target.

For example:

A go_test could include 3 different actions:
- GoCompilePkg
- GoLink
- GoTest

If users wish to use a special exec_properties for GoLink actions,
after this PR, they could do so with:

```python
go_test(
  ...
  exec_properties = {
    # Default
    "container-image": "docker://alpine",
    "EstimatedMemory": "500MB",

    # Only applied to GoLink exec_group
    "GoLink.container-image": "docker://ubuntu",
    "GoLink.EstimatedMemory": "2GB",
  }
  ...
)
```

Reference: https://bazel.build/extending/exec-groups
@sluongng
Copy link
Contributor Author

This seems to be a breaking change for downstream rules that consume rules_go action directly (i.e. gazelle_binary.bzl).

The solution is those rules must declare exec_groups in the rules declaration in order to consume the action 🤔

I don't really see a way around it.

@sluongng
Copy link
Contributor Author

Gona open up this PR for review early.

If needed, I could break this PR into 3 parts for easier merging.

  1. Add the execgroup definition without using it in actions.
  2. Modify gazelle to use the execgroup
  3. Modify rules_go actions to start using the execgroup.

@linzhp @fmeum let me know what you think.

@sluongng sluongng marked this pull request as ready for review September 26, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant