search

dropbox / dbx_build_tools

Dropbox's Bazel rules and tools
Other
208 stars 36 forks source link

Fix python3 stdlib-zip #46

Closed corypaik closed 2 years ago

corypaik commented 2 years ago

The current implementation of @org_python_cpython_38//:stdlib-zip produces different python byte-code on each compilation. As you can imagine, this also means that each rebuild results in a different hash so all tests are invalidated. I initially tracked this down because it's one of the only actions with different outputs and the same inputs in my build. You can reproduce this by building it twice and inspecting the python byte-code with diffoscope:

# Build 2 copies
bazel build @org_python_cpython_38//:stdlib-zip
cp bazel-bin/external/org_python_cpython_38/lib/python38.zip .
bazel clean 
bazel build @org_python_cpython_38//:stdlib-zip
# Inspect with diffoscope
diffoscope bazel-bin/external/org_python_cpython_38/lib/python38.zip  python38.zip

I believe this is due to the ordering of sets in the compiled byte-code, but I am not certain. Either way, defining a set PYTHONHASHSEED fixes the issue (and can be verified using the same steps above).

jhance commented 2 years ago

Thanks, will merge it tomorrow.

Note that you could also avoid this issue by using our prebuilt mechanism which is much more battle tested (we do not use org_python_cpython_38 internally outside of building the prebuilt interpreter, see build_tools/cpython/build_cpython.sh.

jhance commented 2 years ago

Actually, it has conflicts. I did a export so you can rebase.

corypaik commented 2 years ago

Note that you could also avoid this issue by using our prebuilt mechanism which is much more battle tested (we do not use org_python_cpython_38 internally outside of building the prebuilt interpreter, see build_tools/cpython/build_cpython.sh.

Thanks I'll have to take a look sometime! I've been using a modified version of org_python_cpython_38 that works with the standard python toolchain for a while but just thought someone else might run into something similar and find it useful :slightly_smiling_face:.

jhance commented 2 years ago

(It is merged internally and will be present on the next export)