Skip to content

Migrate Automatic Exec Groups by adding a toolchain parameter to the affected actions#10514

Merged
ejona86 merged 3 commits intogrpc:masterfrom
kotlaja:aegs
Sep 6, 2023
Merged

Migrate Automatic Exec Groups by adding a toolchain parameter to the affected actions#10514
ejona86 merged 3 commits intogrpc:masterfrom
kotlaja:aegs

Conversation

@kotlaja
Copy link
Contributor

@kotlaja kotlaja commented Aug 23, 2023 •
edited by sergiitk
Loading

This is a step forward for the full migration of Automatic Exec Groups (AEGs). This change will be effective once AEGs are enabled.

Goal of this migration:

  • Each toolchain type should have its own execution platform. Before this change, the execution platform was selected on a rule base, not on a toolchain type base.
  • Each action which uses a tool coming from a toolchain should specify the toolchain type inside ctx.action.{run, run_shell} toolchain attribute.

I've added None since the executable (protoc) seems not to be from a real toolchain. Moreover, the rule itself doesn't register any toolchains.

Ref cl/559113276.

@kotlaja
Copy link
Contributor Author

kotlaja commented Aug 23, 2023

I can see that this PR request fails with no toolchain attribute error, which is probably since this repo uses Bazel version lower than 6.0, right? toolchain param is added from Bazel 6.0.

@ejona86
Copy link
Member

ejona86 commented Aug 23, 2023

Yeah, we support the two most recent versions. So we'll drop Bazel 5.x in January.

@kotlaja
Copy link
Contributor Author

kotlaja commented Sep 4, 2023

Okay, that means we should not add toolchain param until Bazel 5.x support is dropped. I've disabled Automatic Exec Groups in the new commit and added TODOs. Can you please take a look?

@sergiitk sergiitk added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Sep 5, 2023
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Sep 5, 2023
@ejona86 ejona86 requested a review from sergiitk September 5, 2023 23:52
@ejona86 ejona86 merged commit 9ab35b0 into grpc:master Sep 6, 2023
DNVindhya pushed a commit to DNVindhya/grpc-java that referenced this pull request Oct 5, 2023
@kotlaja @DNVindhya
java_grpc_library.bzl: Disable Automatic Exec Groups inside grpc libr…
ed99e58 
…aries (grpc#10514)
@kotlaja
Copy link
Contributor Author

kotlaja commented Oct 20, 2023

@ejona86 @sergiitk
Hi one more time! :)

I'm interested in when this change will be inside a new release since it's needed for my further migration of Automatic Exec Groups. Do you maybe know when can this happen approximately?
I see that v1.58.0 release doesn't have these changes, so I need to wait for the next release in order to use it in other projects to not be blocked by the error before this PR. Example of usage is intellij: link.

@ejona86
Copy link
Member

ejona86 commented Oct 20, 2023

@kotlaja, v1.59.0 was scheduled for this past Tuesday. It was delayed because of some conversations with protobuf. It looks like it was released yesterday. Release notes and notification are still in the works, but the release is available.

@ejona86 ejona86 mentioned this pull request Jan 10, 2024
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sergiitk sergiitk sergiitk approved these changes

@ejona86 ejona86 ejona86 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kotlaja @ejona86 @sergiitk @grpc-kokoro