Open keith opened 6 days ago
Just recapping what I said in slack. This particular issue is really annoying and frustrating. I ran into this within Google and spent about a week trying to find workarounds, but eventually gave up. Instead, I updated a few hundred targets to have them use the exec-group for exec properties instead of using general keys. e.g. using the key cpp_link.mem
instead of simply mem
. What I found was exec props were being set to affect specific actions and affecting every action was unnecessary.
This is due to two targets with the same .py
source file, but different exec_properties.
Unfortunately, there isn't much we can do in the rules because of a confluence of things.
First: the generated file name has to be based on the source file, not the target, otherwise the pyc file won't end up in a location the interpreter can find it.
The basic thing we have to do is ensure that the py_precompile
exec group is always getting the same exec properties. Ideally, both targets are set to "use the defaults for the py_precompile exec group", e.g. the equivalent of not setting any exec properties. However, having both set the same py_precompile.XXX
keys/values is also acceptable. The important thing is both targets have the same values for the py_precompile
exec group.
Unfortunately, we can't override this when ctx.actions.run()
is called -- this is because the exec_properties set on the target have precedence over exec properties set in action; this is WAI (the point of setting a target-level exec_properties is to override what the rules do)
This means we have to set the exec_properties during the loading phase, before the rule is invoked. Unfortunately, this has several issues and caveats, but the two big ones are:
(1) means, when given {"foo": "bar"}
, we can't transform it to an equivalent dictionary that means "for the py_precompile exec group, act like the foo key was unset" (e.g. {"foo": "bar", "py_precompile.foo": "SPECIAL_VALUE_TO_INDICATE_UNSET"}
.
An alternative would be to always set the py_precompile keys to some default value, e.g. merge in {"py_precompile.foo": "default"}
. But (2) prevents this: the key-values are arbitrary, and we don't know what the default is for an arbitrary key.
Another complication is exec_properties is configurable, i.e. it might be select(), so we can't reliably inspect it during the loading phase (this complicates (1) above). The |
operator can be used to merge dicts into a select, which gives some limited options.
gtg now. A bit of a half-baked wild idea would be: perform compiling in a separate target. e.g. generate a target that takes srcs as input and outputs py/pyc files. We can then control the exec props of that specific target.
🐞 bug report
Affected Rule
With this BUILD file, where 2 python rules include the same file, building both of these targets results in:
Is this a regression?
I bisected to https://github.com/bazelbuild/rules_python/commit/eb2225c31a20a7ee361054b088bcef8cd9434b74, we hit it when updating from 0.36.0 to 0.40.0
🔬 Minimal Reproduction
Build in this repo: repro.zip