Page MenuHomePhabricator

[opt viewer] Python compat - decode/encode string
ClosedPublic

Authored by ychen on Jan 19 2020, 5:57 PM.

Details

Summary

Use io.open instead of codecs.open according to here
https://stackoverflow.com/questions/10971033/backporting-python-3-openencoding-utf-8-to-python-2

Add u prefix to string literal to make them utf-8 in python2.

Diff Detail

Event Timeline

ychen created this revision.Jan 19 2020, 5:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2020, 5:57 PM

#!/usr/bin/env python -> python3 will be simpler, but I know @thakis could be unhappy.

Unit tests: pass. 62002 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

@MaskRay Yeah, that would be ideal. I'm also in favor of supporting python3 only. The change for open to add encoding parameter is still needed though since the default value is platform-dependent.

I'm also in favor of a loose consensus such as « as long as we can keep py2/3 compat, keep it. »

RHEL7 still ships Python2 by default, but it has reached « End of Full Support ». (see https://en.wikipedia.org/wiki/Red_Hat_Enterprise_Linux#Product_life_cycle)
Latest OSX version also explicitly obsoletes Python2 (see https://developer.apple.com/documentation/macos_release_notes/macos_catalina_10_15_release_notes#3318257)

So I'd advocate to keep the compatibility until end of 2020? That's probably a subject that should be discussed on the mlist though.

@ychen I summarized the situation in http://lists.llvm.org/pipermail/llvm-dev/2020-January/138730.html , but according to the documentation, https://llvm.org/docs/GettingStarted.html#id7 we still support python2.7 ...

ychen added a comment.Jan 29 2020, 7:36 AM

@ychen I summarized the situation in http://lists.llvm.org/pipermail/llvm-dev/2020-January/138730.html , but according to the documentation, https://llvm.org/docs/GettingStarted.html#id7 we still support python2.7 ...

@serge-sans-paille io.open is supported since 2.6 https://docs.python.org/2/library/io.html#module-interface. I did test that the patch works for both 2.7 and >3. I saw some failures with the Unicode problem without this change.

@serge-sans-paille io.open is supported since 2.6 https://docs.python.org/2/library/io.html#module-interface. I did test that the patch works for both 2.7 and >3. I saw some failures with the Unicode problem without this change.

Good lord - you're absolutely right!

The discussion diverged because of the python3 shebang, but the actual diff is portable. LGTM then :-)

This revision is now accepted and ready to land.Jan 29 2020, 12:18 PM
This revision was automatically updated to reflect the committed changes.