bazel-lib icon indicating copy to clipboard operation
bazel-lib copied to clipboard

Add toolchain param to affected actions

Open kotlaja opened this issue 1 year ago • 8 comments

This is part of migrations of Automatic Exec Groups (AEGs) where a toolchain parameter should be set inside actions which use executable or tools from a toolchain.

AEGs are under an incompatible flag right now, but will be flipped soon in Bazel 7. I've added this change since I was manually fixing rules_nodejs, which depends on bazel-lib. I can't fully test this change since I need at least Bazel 7, but Bazel@Head raises non related errors to my change. That's why there might be more related updates once bazel-lib supports Bazel 7.

Still, as I already mentioned, adding a toolchain parameter is masked by the incompatible flag, so these changes don't do anything right now. But it can help you with your migration in the near future when AEGs are enabled by default and you are forced to update actions which use toolchains.

Also note that the toolchain parameter is available from Bazel 6, but the full logic (and the incompatible flag) will be available from Bazel 7.

Please check official documentation for Automatic Exec Groups for additional info.


Type of change

  • New feature or functionality (change which adds functionality)

Test plan

  • Manual testing; please provide instructions so we can reproduce:
  • Use (at least) Bazel 7 and add --incompatible_auto_exec_groups flag.

kotlaja avatar Nov 21 '23 13:11 kotlaja

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Nov 21 '23 13:11 CLAassistant

Hi @alexeagle, Nevena here from Bazel team in Munich! :) Can you please take a look at this PR?

kotlaja avatar Nov 24 '23 13:11 kotlaja

Looks like this needs a bazel run //docs:update for docs tests.

kormide avatar Nov 24 '23 21:11 kormide

@alexeagle Do you think we want this backported to the 1.x branch as well?

kormide avatar Nov 24 '23 21:11 kormide

We can say that bazel-lib 1.x doesn't support Bazel 7.0 so I don't think we need to backport this. If someone complains I can revisit.

alexeagle avatar Nov 26 '23 20:11 alexeagle

@kotlaja I tried to update the docs for you, but you didn't check the "allow maintainers to add commits" box

% git commit -a -m "bazel run docs:update"
[kotlaja/main f452708] bazel run docs:update
 1 file changed, 2 insertions(+), 1 deletion(-)
alexeagle@MacBook-Pro-8 bazel-lib % git push
Enumerating objects: 7, done.
Counting objects: 100% (7/7), done.
Delta compression using up to 8 threads
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 493 bytes | 44.00 KiB/s, done.
Total 4 (delta 3), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (3/3), completed with 3 local objects.
To github.com:kotlaja/bazel-lib.git
 ! [remote rejected] kotlaja/main -> kotlaja/main (permission denied)
error: failed to push some refs to 'github.com:kotlaja/bazel-lib.git'

alexeagle avatar Nov 26 '23 20:11 alexeagle

Sorry, I was on a vacation. Thanks for the help here!

Hm, allow maintainers to add commits was already checked, not sure why it didn't work. I've run the command now, please take another look.

kotlaja avatar Nov 29 '23 10:11 kotlaja

@kotlaja i meant to speak with somebody from bazel team about this. does adding this parameter make the action incompatible with older versions of bazel where this attribute isn't recognized or does it get simply ignored?

thesayyn avatar Dec 08 '23 20:12 thesayyn