fix(precompiling)!: make binary-level precompile opt-in/opt-opt work#2243
fix(precompiling)!: make binary-level precompile opt-in/opt-opt work#2243rickeylev merged 5 commits intobazel-contrib:mainfrom
Conversation
9a96ff4 to
46c03b7
Compare
18bef1e to
21a167b
Compare
ac954e6 to
6c2639e
Compare
6c2639e to
dab0846
Compare
|
Ok, finally ready for review. |
|
Before I have a look at this, there is a bug report in #2178. If I try to include the |
|
If a py_library includes pyc files in srcs, then...I'm not sure. It won't be an action conflict, because the pyc is a source file, not a generated file. Both the generated pyc and source pyc will try to go to the same location in runfiles, but I don't recall offhand what'll happen. Runfiles is a bit more lenient about duplicate files. I think it'll pick the first file, where first is according to whatever the depset order happens to result in.
Assuming the pyc files are for the matching python version, then yes, I agree. I'm not entirely sure if we can safely include pyc files, though. If the wheel's pycs are using timestamp checking (the default), then...well I don't see how a wheel could possibly include timestamp-based pyc files reliablity. From our CI, we saw frequent issues due to runtime-generated pycs files, and ultimately had to ignore them. However, I never fully understood why -- presumably everything is in runfiles, with per-file symlinks, and sandboxed, so it's not clear how the runtime was "escaping" and creating files in the backing repo directory along side the original source files (maybe it was specific to the stdlib?). But all those issues went away when we made it ignore pyc source files. 🤷 |
…o fix.pyc.collection
…o fix.pyc.collection
This makes binary-level opt-in/opt-out of precompiling work as intended. Previously,
when
pyc_collection=include_pycwas set on a binary, only transitive libraries thathad explicitly enabled precompiling were being included (which was moot anyways --
libraries put their files in runfiles, so no matter what, their files were included).
The intent was that, when a binary set
pyc_collection=include_pyc, then precompiledfiles would be used for all its transitive dependencies (unless they had, at the
target-level, disabled precompiling). Conversely, if
pyc_collection=disabledwas set,the precompiled files would not be used (unless a target had, at the target level,
enabled precompiling).
To make it work as desired, the basic fix is to make it so that libraries have a place to
put the implicit pyc files (the ones automatically generated), and have the binaries
include those when requested. The net effect is a library has 4 sets of files it produces:
runfiles (e.g., when a library sets
precompile=enableddirectly).it's up to the binary if they go into the runfiles
doesn't include the implicit pyc file, it must include the source py file (otherwise
none of the library's code ends up included).
Similarly, in order to allow a binary to decide what files are used, libraries must
stop putting the py/pyc files into runfiles themselves. While this is potentially
a breaking change, I found that, within Google, there was no reliance on this behavior,
so should be safe enough. That said, I added
--add_srcs_to_runfilesto restorethe previous behavior to aid in transitioning.
BREAKING CHANGES
py_libraryno longer puts its srcs into runfiles directly.--precompile_add_to_runfiles--pyc_collectionprecompile=if_generated_sourceremovedprecompile_source_retention=omit_if_generated_sourceremovedThough 2 through 5 are technically breaking changes, I don't think precompiling
was very usable anyways, so usages of those flags/values is rare.
Fixes #2212