[bazel] Update tblgen rules to support path-mapping #158354
Conversation
Path mapping is a feature of bazel that allows actions to be deduplicated across bazel transitions if they are otherwise identical. This is helpful if you build a binary in N transitions, where the settings differences are irrelevant to this action. In our case we build multiple python native extensions transitioning on the python version they target, and before this change would run each of these actions once per python version even though the outputs would be identical. This is a no-op unless `--experimental_output_paths=strip` is passed. The changes here are just enough to make bazel automatically remap the paths, which is done by how you use the args object. The core change is that instead of carrying around paths that have `ctx.bin_dir` hardcoded in the strings. This is done by mapping them with the output file object's root path when adding them to the args. As a side effect this drops the genfiles_dir, but that has been the same as bin_dir for a long time so hopefully that's a no-op for folks.
|
we have quite a few internal instances of these rules so i'm pretty confident in this change, but it would be great if some other folks could try in their projects if you have your own tblgen rules |
| ctx.attr.deps, | ||
| ) | ||
|
|
||
| test_args = [ctx.executable.tblgen.short_path] |
There was a problem hiding this comment.
Sidenode: I don't immediately see how, but surely this can be expressed via ctx.actions.args(). Not relevant to this PR though.
There was a problem hiding this comment.
yea shouldn't be a big deal either since that would only affect things if this test itself was under a transition
It is, but the old behavior can be restored w/ some |
I don't think that should matter for this case then, since all the files are collected from other ctx.actions.run calls, so I think it would only hit if a td input file was produced by a filegroup, which I don't see any cases of today |
|
if that ends up breaking cases we can conditionalize this behavior on |
keith
deleted the
ks/bazel-update-tblgen-rules-to-support-path-mapping
branch
Path mapping is a feature of bazel that allows actions to be
deduplicated across bazel transitions if they are otherwise identical.
This is helpful if you build a binary in N transitions, where the
settings differences are irrelevant to this action. In our case we build
multiple python native extensions transitioning on the python version
they target, and before this change would run each of these actions once
per python version even though the outputs would be identical.
This is a no-op unless
--experimental_output_paths=stripis passed.The changes here are just enough to make bazel automatically remap the
paths, which is done by how you use the args object. The core change is
that instead of carrying around paths that have
ctx.bin_dirhardcodedin the strings. This is done by mapping them with the output file
object's root path when adding them to the args.
As a side effect this drops the genfiles_dir, but that has been the same
as bin_dir for a long time so hopefully that's a no-op for folks.