-
Notifications
You must be signed in to change notification settings - Fork 543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests failing on Bazel CI #67
Comments
@duggelz made PAR construction deterministic, so without probing deeper I would guess that you changed something (transitively) about how the PAR is built (the same |
How can we resolve this issue? Who would be the person to talk to? |
Bazel has apparently just switched to a new CI system: https://groups.google.com/d/msg/bazel-discuss/8spcVZMv9yE/nz-ya0tpAQAJ |
Heya, we are still seeing this error on our CI.
|
I am pretty curious what the actual difference is. I don't have access to the new CI, so I guess I'll see if I can repo it locally. |
@duggelz I have sent you an invite to your @google.com address. |
Any updates on this issue @duggelz ? Thanks! |
Friendly ping @duggelz, we're still seeing the failures in Bazel CI |
Ok, so here's my diagnosis: .par file construction is not deterministic between different Python interpreter versions. In particular, python 2.7.13+ and python 2.7.12- produce zip files with slightly different header flags, because 2.7.12 was out of spec (but the zip files still work - it's a loose spec). That seems to be what's happening here. It's not really fixable by subpar. We could relax the test to check that the contents of the individual zip members are the same. But a malicious adversary could easily handcraft a zipfile header that passes that relaxed check. And I'm actually not comfortable with this as a "security" boundary anyway, because I certainly didn't write the code as such. So, unless we have some kind of process where .par files are regenerated on a "blessed" image with a known Python interpreter version, and/or by a "blessed" release person, we should probably disable this test. And we really shouldn't let untrusted contributors submit updated .par files. |
As per this thread: https://bugs.python.org/issue29094 , it appears that Python 2.7.13, 3.5.3, 3.6.0, and 3.6.1 (only) produce these subtly broken .par files. Unfortunately, those are the versions that are installed with the widely used Debian 9 (not to mention my personal workstation!). By chance, it appears that none of the BuildKite builder machines have those "bad" versions, so we should be able to rebuild the .par files, and turn the build green. |
Regenerating the files on Ubuntu 16.04 turned the build green: #83 Going forward, it seems like we can keep this test, at the cost of a bit of annoyance for people with "broken" Python versions (basically just Debian 9). Google's internal workstations will be updating from Python 2.7.13 to 2.7.14 "soon", and thus get the fix, in the future (We also have Python 3.6! Which is now entirely supported and recommended inside Google! Use Python 3 everywhere!) |
Can #83 be merged? We'd like to cut a release for Bazel 0.13 and failing rules_python builds block that. |
Merged. |
@duggelz Not sure what's going on, but this still seems to fail (tested with Bazel 0.12.0rc3 just a few minutes ago):
|
I am seeing failures in Bazel CI for both Bazel HEAD and Bazel 0.10.1: https://ci.bazel.build/blue/organizations/jenkins/Global%2Frules_python/detail/rules_python/276/tests
I can reproduce this locally and get the same error messages.
The text was updated successfully, but these errors were encountered: