-
Notifications
You must be signed in to change notification settings - Fork 5.1k
bazel: adapt cc_wraper.py to python3 #7519
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
Conversation
Description: In Arch and Clear linux /usr/bin/python points to python3 causing build failures due to type mismatch in os.write(): string is used where bytestring is expected. Explicitly convert string to bytestring. Risk Level: low Testing: unit tests Release Notes: N/A Documentation: N/A Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
@envoyproxy/maintainers as an aside, given the number of things that had trouble I think we should be more careful tweaking our python library in future |
And thanks for picking up this fix, @rojkov :-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to force the file to be Python 3 compatible? To @alyssawilk point, I think that as we roll over files to Python 3, we need to ensure we don't regress, and machine checking is the way to go.
/retest |
🔨 rebuilding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rojkov LGTM, but can you address my above comment?
Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@linux.intel.com>
@htuch I've updated the file's shebang. Or you want to enforce compatibility with both pythons? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks; I'm hoping this doesn't break folks, I think it's reasonable to assume that most of the world has python3 these days, given python2 is deprecated.
This change breaks compiling on RHEL7. I do have python3 but in a non-standard place. After updating the shebang to point to python3 the compilation fails in spectacular ways. Will be interesting to see if this breaks those on Centos. /cc @htuch @rojkov Edit - upon digging further I think that #7329 is responsible for the RHEL7 issues, not this PR /cc @lizan |
@moderation would switching to |
Description:
In Arch and Clear Linux
/usr/bin/python
points topython3
causing build failures due to type mismatch inos.write()
: string is used where bytestring is expected.Explicitly convert string to bytestring.
Risk Level: low
Testing: unit tests
Release Notes: N/A
Documentation: N/A