shell toolchain implementation question

149 views
Skip to first unread message

László Csomor

unread,
Apr 26, 2018, 4:13:02 AM4/26/18
to John Cater, Marissa Staib, Greg Estren, bazel-discuss
Hey all,

I need some guidance on the shell toolchain I'm working on.

== Background ==
My goals are to let Bazel run without Bash or any shell -- unless you build rules that need it -- and to support other paths than the hardcoded "/bin/bash". The solution seems to be Toolchains, so I introduced the shell toolchain in commit 84d30975.


== Problem ==
To use a toolchain in an action, the action's owning rule class must declare a dependency on the toolchain_type rule. Therefore every rule that needs to run shell commands has to depend on the toolchain_type rule. Obvious examples are sh_* rules and genrule, and anything that creates a SpawnAction with setShellCommand.

Any Skylark rule may create a shell action with ctx.actions.run_shell, therefore, to not break existing Skylark rules by requiring an explicit toolchain dependency, every Skylark rule by default must depend on the shell toolchain.

Finally, extra actions can shadow any action of any rule class. Extra actions are shell commands, and the extra action's owner is the "target" rule, i.e. that which created the shadowed action, meaning the "target" rule's configuration selects the toolchain, therefore every rule must depend on the shell toolchain.

== Questions ==
(a) Is my problem analysis correct?

(b) Where do I declare the dependency on the toolchain_type?
Obvious choice is to add it in BaseRuleClasses.commonCoreAndSkylarkAttributes, we just need a way to retrieve Label objects (to add a dependency on the toolchain_type rule). This method has 3 call sites: in BaseRule we have a RuleDefinitionEnvironment, in SkylarkRuleClassFunctions we can use the labelCache member, but in SkylarkRepositoryModule, I have no idea how to get a Label.

(c) If every rule depends on the toolchain_type, but the toolchain_type itself is a rule, how do I avoid the cycle?


Thanks,
Laszlo

--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

László Csomor

unread,
Apr 26, 2018, 4:27:13 AM4/26/18
to John Cater, Marissa Staib, Greg Estren, bazel-discuss
Correction: the commit introducing the ShToolchain was 81ed3add.



--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado


John Cater

unread,
Apr 26, 2018, 1:35:56 PM4/26/18
to László Csomor, Marissa Staib, Greg Estren, bazel-discuss
Replies inline.

On Thu, Apr 26, 2018 at 4:27 AM László Csomor <laszlo...@google.com> wrote:
Correction: the commit introducing the ShToolchain was 81ed3add.
 
On Thu, Apr 26, 2018 at 10:12 AM László Csomor <laszlo...@google.com> wrote:
therefore every rule must depend on the shell toolchain.

== Questions ==
(a) Is my problem analysis correct?


Yes, I think that's correct. Unfortunate, but correct.
 
(b) Where do I declare the dependency on the toolchain_type?
Obvious choice is to add it in BaseRuleClasses.commonCoreAndSkylarkAttributes, we just need a way to retrieve Label objects (to add a dependency on the toolchain_type rule). This method has 3 call sites: in BaseRule we have a RuleDefinitionEnvironment, in SkylarkRuleClassFunctions we can use the labelCache member, but in SkylarkRepositoryModule, I have no idea how to get a Label.

Looks right. You don't need to retrieve the Label, just create one. See how the cc toolchain type is handled at CppRuleClasses.ccToolchainTypeAttribute (https://source.bazel.build/bazel/+/ea302f5341ac7532f7536f3bd3bc5b6ca48e2141:src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java;l=94)
 

(c) If every rule depends on the toolchain_type, but the toolchain_type itself is a rule, how do I avoid the cycle?


RuleClass.Builder has a supportsPlatforms setting (https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/packages/RuleClass.java;l=1163?q=RuleClass), which is used to signal "this rule skips toolchain resolution and never has an execution platform". It's intended for just this case.

Unfortunately, it's not currently exposed to Skylark's rule() function. It shouldn't be too hard to wire that up and set the flag in sh_toolchain.

John C
 

Greg Estren

unread,
Apr 26, 2018, 4:42:43 PM4/26/18
to John Cater, László Csomor, Marissa Staib, bazel-discuss
Is it really a bad thing if every target ends up depending on the shell toolchain? How much migration logic would that actually involve? The optimist in me would like to think this would be way more straightforward than, say, Java or C++.

László Csomor

unread,
May 17, 2018, 4:33:30 AM5/17/18
to Greg Estren, John Cater, Marissa Staib, bazel-discuss
Thanks John and Greg! Forgive my delay in answering, it took me a while to use your answers, but they were essential to unblock me.

I'm now struggling with another question: how do we auto-configure remote toolchains? A Skylark repository rule can discover its execution machine's configuration and synthesize a BUILD file with the right toolchain definitions for local actions. But can we do the same for a remote worker? Or must we register statically configured toolchains for remote actions?



--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado


John Cater

unread,
May 17, 2018, 4:39:13 AM5/17/18
to László Csomor, Greg Estren, Marissa Staib, bazel-discuss
There have been some discussions about a system where auto-config can run remotely, but nothing has actually happened there. The current only solution is that, if you are using remote execution, you need to statically configure your toolchains for those execution platforms.

László Csomor

unread,
May 17, 2018, 4:49:37 AM5/17/18
to John Cater, Greg Estren, Marissa Staib, bazel-discuss
I see, thanks for the clarification!

I have no more questions for now. :)



--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado


László Csomor

unread,
May 17, 2018, 5:18:51 AM5/17/18
to John Cater, Greg Estren, Marissa Staib, bazel-discuss
I spoke too soon, I do have one more question: is it possible to obtain a toolchain without a RuleContext? In particular, RunCommand [1] obtains the shell interpreter's path from the ShellConfiguration fragment. If the `--shell_executable` flag is empty but there's a valid toolchain, RunCommand cannot use it.




--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado


John Cater

unread,
May 17, 2018, 5:25:01 AM5/17/18
to László Csomor, Greg Estren, Marissa Staib, bazel-discuss
The toolchain resolution process only runs for a ConfiguredTarget (which means there's a RuleContext somewhere). You could possibly add hooks into RunCommand to call into ToolchainUtil for the ToolchainContext, but I'm not sure how well it would work.


Although note that the ToolchainContext this creates isn't useful until after dependencies have been resolved and re-added to the context.

It might be easier to create a fake target that RunCommand configures just to get the shell executable path from, although that's also kind of ugly.

László Csomor

unread,
May 17, 2018, 5:50:24 AM5/17/18
to John Cater, Greg Estren, Marissa Staib, bazel-discuss
Thank you for the quick answer. Alas, implementing your suggestion seems onerous.
Perhaps we should not use the shell at all in RunCommand. I don't know why we do.
For now I'll keep using the config fragment in RunCommand.



--
László Csomor | Software Engineer | laszlo...@google.com

Google Germany GmbH | Erika-Mann-Str. 33 | 80636 München | Germany
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado


John Cater

unread,
May 17, 2018, 5:52:38 AM5/17/18
to László Csomor, Greg Estren, Marissa Staib, bazel-discuss
Removing it entirely seems the best approach if possible.
Reply all
Reply to author
Forward
0 new messages