-
Notifications
You must be signed in to change notification settings - Fork 30k
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
build: python3 support for configure #30047
Conversation
This is similar to #30023 |
indeed it is! but in this case the return value needs to mix with non-utf8 strings and python2 borks at that, hence the |
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.
LGTM.
Minor nit: for line in open(uvernum_h).readlines():
leaves the filehandle open so the preferred approach is:
with open(uvernum_h) as in_file:
for line in in_file:
because the with open() as
context manager will automate the filehandle close()
.
ok, have tried that approach, ptal @cclauss |
I have already approved. For those who want to dig in... |
40c91e1
to
88ecfee
Compare
PR-URL: #30047 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com>
Landed in 779d7ef |
PR-URL: #30047 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: #30047 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: #30047 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: #30047 Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com>
Discovered while messing with some infra switching things around to
python3
. On Linux, if you callconfigure
directly with apython3
(not letting it findpython
for you) and haveLC_ALL=C
set, as we do in some of our infra, apparentlyopen()
defaults to ascii. This is a problem for icu version detection because there's a©
at the top of deps/icu-small/source/common/unicode/uvernum.h which it reads to find major version number.This change fixes it and makes it work for Python 2 & 3. Although I don't really know what I'm doing with the conversion back to ascii for later compatibility so someone who knows better should check.