Skip to content

Added python3 support#62

Merged
gaasedelen merged 3 commits into
gaasedelen:developfrom
xarkes:py3
Mar 18, 2019
Merged

Added python3 support#62
gaasedelen merged 3 commits into
gaasedelen:developfrom
xarkes:py3

Conversation

@xarkes

@xarkes xarkes commented Mar 16, 2019

Copy link
Copy Markdown
Contributor

This pull requests adds python3 support while keeping python2 compatibility.
Please note that it works for my needs for adding lighthouse to https://github.com/radareorg/cutter however I did not really try it on other disassembler with python2, so I suggest to heavily test it before :)

@xarkes xarkes changed the base branch from master to develop March 16, 2019 19:45
@gaasedelen

Copy link
Copy Markdown
Owner

Sorry for the merge conflicts on the dev branch. I started to re-architect some stuff, and there is more coming 😅. I'll try to merge your stuff in before my commit storm hits. I should have some time to test it tonight and tomorrow.

I am also kind of surprised. This is fewer changes than I thought Py3 compatibility was going to take. It was also on the roadmap for the few binja users that choose to use a Python 3 interpreter (#62)

@gaasedelen

Copy link
Copy Markdown
Owner

I had a chance to look at this today and will not accept it outright.

My chief complaint is the use of six. Lighthouse does not require any external dependencies at this time, and I would prefer to keep it that way for user convenience.

Considering how few items we need from six, it is best we just create our own shim :-). I went ahead and did this. It appears to be working in my IDA/Python 2.7 env, but I don't have Binja on Py 3.x (or your cutter changes).

Care to test??

@gaasedelen

gaasedelen commented Mar 18, 2019

Copy link
Copy Markdown
Owner

I switched my Binja env over to 3.6 and got it set up with PyQt5. Needed a few more fixups, but seems to work.

Good enough for dev ¯\_(ツ)_/¯

@gaasedelen gaasedelen merged commit 7cab1c1 into gaasedelen:develop Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants