Using transitions (?) to express that some targets are platform independent.
1,033 views
Skip to first unread message
Konstantin
Jun 19, 2021, 4:25:33 PM6/19/21
to bazel-discuss
We have a large project where most of the targets has to be built for each target platform separately. At the same time we have substantial number of the “codegen” targets, which invoke Python to produce additional source files. Result of that code generation does not depend on the target platform – generated sources are always the same on all platforms or configuration flavors. It is inefficient to run codegens for each platform when we can share the results for all platforms.
I am looking how to express that in Starlark and my gut feeling is that configuration transitions should be involved, although it is not obvious how exactly.
My first try was to add cfg = “exec” to express that Python targets should be run with executor configuration. Unfortunately to my surprise cfg = “exec” can be specified on the attributes of the rule, but CANNOT on the cfg attribute of the rule itself! We have thousands of rules which depend on the Python rules and it is not feasible to modify all of them (add cfg = “exec“ to the attribute) to indicate configuration change. Also in some cases Python rules are part of the lists where other rules are normal, so we cannot even specify cfg = “exec” on the consuming rule attribute as it would affect other irrelevant rules too.
My next attempt was to implement a simple custom transition like this:
def _winr64_impl(settings, attr):
current_platform = settings["//command_line_option:platforms"]
if current_platform == [Label("@tab_toolchains//bazel/platforms:x86_windows")]:
new_platform = "@tab_toolchains//bazel/platforms:x64_windows"
else:
new_platform = current_platform
return {
"//command_line_option:compilation_mode": "opt",
"//command_line_option:platforms": new_platform,
}
winr64_transition = transition(
implementation = _winr64_impl,
inputs = [
"//command_line_option:platforms",
"//command_line_option:compilation_mode",
],
outputs = [
"//command_line_option:compilation_mode",
"//command_line_option:platforms",
],
)
The idea behind it is to always use opt flavor regardless of what’s requested and also when the build platform is x86_windows it is replaced with x64_windows, so we get only one set of generated sources for all four combinations of Windows builds.
It does work, but I still don’t understand how to share the results of the platform independent targets with the other platforms, such as Linux and Mac.
Any advice?
Thank you!
Konstantin
Alex Humesky
Jul 13, 2021, 7:37:08 PM7/13/21
to Konstantin, bazel-discuss
I think using the exec "self transition" is probably the easiest way to accomplish this. The trick is to use "cfg = config.exec()" (rather than the string "exec"). This is rather new, and there appears to be some issues to iron out, because bazel crashes when I tried it myself:
defs.bzl:
def _impl(ctx):
return []
my_rule = rule(
implementation = _impl,
cfg = config.exec(),
)
BUILD:
load(":defs.bzl", "my_rule")
my_rule(name = "foo")java.lang.ClassCastException: class com.google.devtools.build.lib.packages.Rule cannot be cast to class com.google.devtools.build.lib.packages.AttributeTransitionData (com.google.devtools.build.lib.packages.Rule and com.google.devtools.build.lib.packages.AttributeTransitionData are in unnamed module of loader 'app')
In the meantime, we can look at workarounds.
> We have thousands of rules which depend on the Python rules and it is not feasible to modify all of them (add cfg = “exec“ to the attribute)
I'm a little curious about this, because "cfg = "exec"" would go on rule definitions themselves, as opposed to instances of rules (i.e. targets). It's not surprising to have thousands of rule instances / targets, but I would be a little surprised if you had thousands of rule definitions (i.e. "my_rule = rule(implementation = _impl, attrs = .....)")
> Also in some cases Python rules are part of the lists where other rules are normal, so we cannot even specify cfg = “exec” on the consuming rule attribute as it would affect other irrelevant rules too.
Yes, that would make "cfg = "exec"" on the consuming rule's attribute tricky.
It might be possible to create an intermediate rule that does the transition between the codegen targets and the consuming targets, as a workaround.
Something like
def _codegen_impl(ctx):
# codegen
_codegen_rule = rule(
implementation = _codegen_impl,
attrs = {
"codegen_tool": attr.label(default = "//tools:codegen", cfg = "exec")
}
)
def _exec_impl(ctx):
# forward the right providers
_exec_transition_rule = rule(
implementation = _exec_impl,
attrs = {
"dep": attr.label(mandatory = True, cfg = "exec"),
},
)
def codegen(name, **kwargs):
_codegen_rule(name = name + "_codegen", ...)
_exec_transition_rule(name = name, dep = name + "_codegen")
# codegen
_codegen_rule = rule(
implementation = _codegen_impl,
attrs = {
"codegen_tool": attr.label(default = "//tools:codegen", cfg = "exec")
}
)
def _exec_impl(ctx):
# forward the right providers
_exec_transition_rule = rule(
implementation = _exec_impl,
attrs = {
"dep": attr.label(mandatory = True, cfg = "exec"),
},
)
def codegen(name, **kwargs):
_codegen_rule(name = name + "_codegen", ...)
_exec_transition_rule(name = name, dep = name + "_codegen")
python_binary(
name = "codegen_tool",
srcs = [...],
)
genrule(
name = "codegen",
exec_tools = [":codegen_tool"],
srcs = [...],
outs = ["codegen_out"],
cmd = "$(location :codegen_tool) ....",
)
genrule(
name = "codegen_exec_config",
exec_tools = [":codegen_out"],
cmd = "cp $< $@",
)
name = "codegen_tool",
srcs = [...],
)
genrule(
name = "codegen",
exec_tools = [":codegen_tool"],
srcs = [...],
outs = ["codegen_out"],
cmd = "$(location :codegen_tool) ....",
)
genrule(
name = "codegen_exec_config",
exec_tools = [":codegen_out"],
cmd = "cp $< $@",
)
--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/e1630f16-3c21-4b1b-a756-ffb5ea8860dbn%40googlegroups.com.
John Cater
Jul 14, 2021, 7:39:43 AM7/14/21
to Alex Humesky, Konstantin, bazel-discuss
A number of points:
Alex, the issue you see is down to the way I wrote the TransitionFactory. It is parameterized by the type of data the factory receives, which can be either attribute data (for an attribute transition), or the rule itself (for a rule transition). The execution transition only applies to attributes, and so you cannot pass "config.exec()" to a rule's `cfg` parameter.
I'm not sure it makes sense to try and use the exec transition as an incoming transition on a rule. When it is applied, we don't yet know the execution platform that the rule will use, we only know the target platform. The execution platform is determined after incoming transitions are applied, and I don't see how this makes sense, so I don't think it's worth fixing the types to make this work. We should probably look into adding more useful error messaging, however.
Konstantin: I agree with you and Alex that an incoming rule-level transition will do what you want, to express the idea of a "platform-independent" rule. It sounds like currently you implement this via a python script. Is that a custom rule, or wrapped in a genrule? It's unclear how you are currently calling this script.
I would define a new platform in your repository, with either no constraints or a small number. Then you can add an incoming custom transition to your custom codegen rule, setting the target platform to always be this static platform. That way, no matter what target depends on the codegen, it will be the same one in the same target platform.
To get full cachability, you may need to set other common flags in your transition, if there are differences between your builds beyond just the target platform.
John C.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAAvaYBn3h_d5YM4P%2BLQwqg-s0E1qHv1Z%3DDo%3Do0HY-v2rXOgsFA%40mail.gmail.com.
Konstantin
Jul 14, 2021, 9:34:27 PM7/14/21
to bazel-discuss
Alex, I'm curious where you got that
"cfg = config.exec()" construct? I don't remember seeing it anywhere and wonder what other values could be there.
John, I like your solution with the "empty" platform a lot. For Python we use custom rules which use our custom Python toolchain. So here is what I did:
platform(
name = "any",
)
def _any_impl(settings, attr):
return {
"//command_line_option:platforms": ["@tab_toolchains//bazel/platforms:any"],
"//command_line_option:compilation_mode": "opt",
}
any_transition = transition(
implementation = _any_impl,
inputs = [],
outputs = [
"//command_line_option:platforms",
"//command_line_option:compilation_mode",
],
)
merge_codegens_rule = rule(
implementation = _merge_codegens_rule_impl,
attrs = {
"srcs": attr.label_list(providers = [TabCcIncludesInfo]),
"_script": attr.label(allow_single_file = True, default = "merge_codegens.py"),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
cfg = any_transition,
toolchains = ["@bazel_tools//tools/python:toolchain_type"],
)
And it works nicely, so specifically for Python rules the problem is solved.
But we have another type of rules (QT library Moc codegen) which does not execute Python, but rather use an executable specific for exec platform.
Here is the example:
moc_codegen_rule = rule(
implementation = _moc_codegen_rule_impl,
attrs = {
"input_header": attr.label(allow_single_file = True, mandatory = True),
"cpp_output": attr.output(mandatory = True),
"_moc_binary": attr.label(
cfg = "exec",
executable = True,
allow_single_file = True,
default = "//thirdparty:Qt5_Moc",
),
"deps": attr.label_list(default = [], providers = [CcInfo]),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
cfg = any_transition,
)
"//thirdparty:Qt5_Moc" is the alias which does select() to pick the correct executable.
This rule causes error: No matching toolchains found for types @bazel_tools//tools/cpp:toolchain_type.
I am not exactly sure why. This rule produces .cpp file through the "cpp_output" attribute and the file is then compiled by one of cc_ rules and those rules cannot resolve the toolchain as they see "any" platform instead of the target one.
AFAIU dependency chain is cc_library -> blah.cpp -> moc_codegen_rule -> _moc_binary
So in my understanding "any_transition" should change the platform for moc_codegen_rule and I don't understand why it affects .cpp toolchain which is higher in the dependency chain.
John, could you please clarify it for us?
Thank you!
Konstantin
Jul 14, 2021, 9:57:53 PM7/14/21
to bazel-discuss
After some thinking I believe my previous understanding of the problem was not correct. moc_codegen_rule has "deps" and those accidentally inherit "any" platform from the rule inbound transition.
To mitigate this problem I have changed this line:
"deps": attr.label_list(default = [], providers = [CcInfo], cfg = "exec"),
Surprisingly when I tried cfg = "target" it did not work, but cfg = "exec" does work.
For a more generic case I would like to understand: transition can query current configuration in order to decide how to change it. What if I want to use inbound transition to override some option and then use outbound transition to set it back to what it was. Inbound transition can query the current state of the option, but where can it store it, so that outbound transition has access to it and can restore it?
John Cater
Jul 15, 2021, 7:17:45 AM7/15/21
to Konstantin, bazel-discuss
You cannot, in general, have a "stack" or configurations and then push or pop them, which seems to be what you want. The "target" transition is more properly named in native code, where it is called "NoTransition" and works by not changing anything.
One option is that you could create a build setting to "store" previous configuration flags, and then have a second transition which uses that to reset them, but this is a little hacky. If I understand your issue, you have the "merge_codegens_rule", which depends on the "moc_codegen_rule", and it turns out that the "moc_codegen_rule" does have a real dependency on the target platform (since it needs a C++ toolchain). Because of this, your "merge_codegens_rule" also has a real dependency on the target platform: if the target platform changes, then different code will be generated from the moc_codegen_rule, and so merge_codegen_rule will have different outputs.
Instead of trying to trick Bazel into re-using the same configured target (by manipulating the configuration), have you considered setting up an action cache instead? The action cache has no dependency on the configuration, only on the precise inputs and command lines. If you actually generate the same code for different platforms, you will repeat the analysis work in Bazel, but then have a successful cache hit from the action cache and so not need to wait for re-execution.
https://docs.bazel.build/versions/main/remote-caching.html has some good docs on how a remote action cache would work and pointers on setting one up, if you are interested.
John C
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/46d97796-79a1-4f56-91c1-31c898816816n%40googlegroups.com.
Konstantin
Jul 15, 2021, 10:10:07 AM7/15/21
to bazel-discuss
Let me clarify the use case: there is no direct connection between
"merge_codegens_rule" which is pure Python and works fine and
"moc_codegen_rule" which invokes custom executable.
--
it turns out that the "moc_codegen_rule" does have a real dependency on the target platform (since it needs a C++ toolchain).
"moc_codegen_rule" appears to have formal dependency on C++ toolchain, but it is not real. The tool that "moc_codegen_rule" invokes reads the given C++ header file and does some textual transformations to produce its output. To do that the tool also needs to be able to find all the headers transitively included from the primary header given to it. In a typical for Bazel way additional headers are provided in a form of header only cc_library, and this is what causes confusion. Formally cc_library depends on C++ toolchain and therefore on the target platform, but in case where it is "header only library" it does not produce any actions and merely serves as the file container with some extra metadata, like a filegroup. As such it does not have a real dependency on any toolchain.
For that reason I need outgoing transition for the attribute taking header only cc_library to set the platform to any with the C++ toolchain registered, does not even matter which one.
And about action cache - it is absolutely our plan. Otherwise why would we need cacheability of the platform independent actions across different platforms.
So action cache is definitely needed here, but it does not replace the need for the transition sandwich.
John Cater
Jul 15, 2021, 11:25:01 AM7/15/21
to Konstantin, bazel-discuss
With action caching: it doesn't matter if the targets are re-analyzed with different configurations, as long as the actual actions are the same. That being the case, I think it will be more profitable for you to examine what is causing your actions to be different, rather than trying to force your configurations to be different. "bazel aquery" is the right tool for examining actions generated by different builds.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/98604362-2b12-4649-9fea-731bf602bf13n%40googlegroups.com.
Alex Humesky
Jul 15, 2021, 5:00:18 PM7/15/21
to Konstantin, bazel-discuss
On Wed, Jul 14, 2021 at 9:34 PM Konstantin <kon...@ermank.com> wrote:
Alex, I'm curious where you got that "cfg = config.exec()" construct? I don't remember seeing it anywhere and wonder what other values could be there.
So I started with "cfg = "exec"" in the bzl file, and got the same error as you:
Error in rule: `cfg` must be set to a transition object initialized by the transition() function.
then I searched for that error in the codebase, which lead me to here:
Looking at the surrounding code, it looked this this branch was what was needed:
So I looked for things that implemented TransitionFactory, and found ExecutionTransitionFactory:
Then I looked through the call hierarchy of the constructor, which lead me to here:
and the starlark annotation here:
and these are the implementation for "config.exec()". The javadoc for that class:
lead me to believe that this should be usable with rule(cfg = ...), but it seems that's not the case! Sorry for the confusion.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/068074cc-a175-4566-baf0-8aab07b92f19n%40googlegroups.com.
Reply all
Reply to author
Forward
0 new messages