This is an archive of the discontinued LLVM Phabricator instance.

Make llvm-git (monorepo helper) compatible with both Python 2 and 3
AcceptedPublic

Authored by t.p.northover on Mar 3 2017, 3:39 PM.

Details

Summary

It seems that the subprocess functions return bytes rather than str for the stdout on Python 3. When this script then tries to combine them with normal strings, an exception is raised.

It seems like we either want to use bytes everywhere, or assume utf-8 encoding. The latter is a much smaller change, and probably reasonable since both git internals and (I assume) our repository text is assumed utf-8. Actual binary data seems to get base64 encoded so I don't think coincidentally-invalid utf-8 is an issue there.

Anyway, with these changes I can commit using both python3.5 and python2.7.

Diff Detail

Repository
rL LLVM

Event Timeline

t.p.northover created this revision.Mar 3 2017, 3:39 PM
zturner added inline comments.Mar 3 2017, 3:59 PM
llvm/utils/git-svn/git-llvm
102

This is a bit wrong since it will try to print call str(b'...') behind the scenes. So if the output is "Hello, world" we'll see b'Hello world` printed.

Can you add a decode here?

103–104

I'd add the decode above this line, that way you can strip it without the b syntax.

109

Probably will need another decode here. Alternatively, you could try hiding this inside of the eprint function so users don't have to remember to do it.

Thanks for the suggestions. I think I've incorporated all of them, and the diff seems much cleaner.

zturner accepted this revision.Mar 7 2017, 3:39 PM
This revision is now accepted and ready to land.Mar 7 2017, 3:39 PM