Python bindings: Fix editable install + Execute Python2.7 workflow tests

[Antelox]

Python bindings:

• Fixed editable mode install and some more code cleanup in setup.py
• Added missing license field in pyproject.toml file
• Refreshed README.md
• Replaced f-string formatter in tests with format method in order to be py2-compatible
• Fixed typos
• PEP8 fixes

GitHub Action:

• Install Python2.7 and run tests for re-tagged wheels on native arch runners only

[elicn]

Got a few comments on the changes made here, if that's OK:

• I see there are "tests" and "regression tests" (executed by regress.py); do we need them both? Can we spare one of them, or maybe merge them into one coherent test quite?
• Regression tests suite uses unittest while the tests use pytest (starting from this PR); do we want to keep it consistent and switch to either one of them?
• Python 2 is a relic. I don't see a point making any modifications to that piece of archeology, including petty PEP8 changes (which I personally like, everywhere else). At this point I suppose it would be better to offer it "as-is", including whatever bugs that are hiding there. Modifying the tests to support both Python 3 and 2 is a maintenance disaster I think we better avoid; Python2 support is going to be dropped sooner or later and then what? Are we going to change everything back to meet Python 3 standards..?
• As for the Python 2 support, I see that there were changes made to move away from f-strings by using format, but this can be done in a more elegant way. For example: Don't turn this:

  print(f">>> X2 = {mu.reg_read(UC_ARM64_REG_X2):x}")

  into this:

  print(">>> X2 = {reg}".format(reg=hex(mu.reg_read(UC_ARM64_REG_X2))))

  but instead, use this:

  print(">>> X2 = {:#x}".format(mu.reg_read(UC_ARM64_REG_X2)))

  That former is cumbersome and unreadable, while the latter still resembles the f-string it used to be and might be easier to change back when time comes.

• And on a final personal note, glad to see that my code did not require any improvements except of a few typos and grammar fixes (:

[Antelox]

Hi @elicn , thanks for the feedback!

Yeah so it's quite confusing having both tests and regression tests, I agree. In fact, I asked about this to @wtdcode and it has to be properly integrated into current CI in the upcoming releases once he fixes some bugs. My point here together with the previous PRs that I created was that of at least make them pytest-runnable and be executed within the CI pipeline, right after the build in order to spot bugs/issues. Indeed this helped spot few more bugs that probably were there since some times.

Regarding Python2 it's a best-effort thingy and if you check indeed, there is no actual changes beside PEP8 fixes, typos and removing of a python version check that is kinda a leftover from previous refactoring.

The changes regarding tests are very dummy I admit, it was to let them run with Python2. I don't think they harm ehehe, but I do agree I can rewrite strings formatting as you suggested. However keep into account that those prints has to be removed once these tests turn into actual pytest unittests. At the moment they are just scripts that will spot failures only if they fail to run, there is no checks/asserts towards expected values or things that make them actual tests ;). That is why i didn't sweat much about them at the moment. I don't see current change how can limit current Python 3 development, actually it just make sure the re-tagged python 2 wheels are working as suppose to do. Then once maintainers will decide to definitely drop it, it's a matter of removing few lines/files without reverting anything.

[elicn]

@Antelox, my two key points here are:

1. We should avoid having two test suites; we should consolidate one of them into the other and all tests running as part of a PR, as well as providing a convenient way to run them locally so contributors may be able to test their code before they push it. I invite you to take a look at the regression tests suite, which you can run locally by executing regress.py, or run any test individually - directly. It doesn't use prints, but a centralized logger that can be tuned for the needed logging level.
2. Any change to the Python 2 bindings at this stage is redundant, in my opinion, and that includes any change. There was not a separated binding to Python 3 just until recently, so it made some sense to maintain the old and messy one. Now that we do have one, we should let the Python 2 binding die peacefully.

[wtdcode]

@Antelox @elicn

Thanks for your efforts and input. I hope to clarify a bit here.

Any change to the Python 2 bindings at this stage is redundant, in my opinion, and that includes any change. There was not a separated binding to Python 3 just until recently, so it made some sense to maintain the old and messy one. Now that we do have one, we should let the Python 2 binding die peacefully.

I agree with this point and we will remove python2 once pypi stops accepting the py2 wheels, which I assume should happen in the near future (?). The goal of this PR is not to "maintain the old and messy" python2 binding but to ensure our "best efforts", which is:

• We firstly provide wheels, re-tagged from python3 built wheels.
• If that fails, we will fallback to source distribution.

Current workflow changes just ensure that we don't upload broken wheels to pypi (not really maintenance, even not fixing failing tests) and this probably is the last change to py2 before removing py2 from current bindings/python.

Furthermore, I will try to separate the whole py2 relic to a standalone folder bindings/python2 once this is done so that we can remove all the maintenance burden of py2.

We should avoid having two test suites;

That's true and I will merge two tests together once I fix issues found by both of them, or we will always have falling CI.

[Antelox]

Yes I totally agree about Python2 next steps. Since it's a best-effort thingy, it can stay with no further changes then once pypi index starts to reject the upload of py2 wheels then we can totally remove it. However here there is no actual change in favor of py2 beside few strings formatter things in those tests scripts (which as already said they are not actual unittests but just scripts that can be executed to test it doesn't crash). Most of the changes you see are because of PyCharm analyzer while I executed to spot typos and had PEP8 fixed. Then removed the check about python version since it was pointless after the split @wtdcode did in the previous version and you also pointed this out in another PR (https://github.com/unicorn-engine/unicorn/pull/2039#discussion_r1816246597)

I fully agree about having a single folder with tests and if you guys don't like or don't want to use any external testing interfaces, I'm fine to use the native unittest interface offered by python.

My point here was just to be sure the re-tagged wheel produced for py2 it's not broken and since I had those scripts in python bindings folder handy I used them for this kind of test as well, and had to change string formatter to make py2 happy. In any case there is no need to revert any changes to make py3 happy, once we decide to remove py2, it's a matter of removing few lines from workflow and the unicorn_py2.py file.

If you all agree I would go ahead with this PR, then I will work on a new one to make workflow use the tests from regress folder and use just unittest interface without pytest (even though I don't see the problem with pytest, it's just a tool that helps write more customized tests but probably not really needed here). I guess the scripts in python bindings can be moved into regress and perhaps removed if they suppose to test the same things already present there. Indeed I took a look to the regress.py script and beside the logger setup, the rest should not be really needed since you can use the discover feature:

python -m unittest discover -v -s tests/regress/ -p *.py

A next-next step would be review all those regress and add asserts where missing.

[gerph]

I'm probably the only user of the Python 2 Unicorn left now - I have an operating system that is implemented in Python 2, with Unicorn as the back end. Converting a whole OS from Python 2 to Python 3 is something of a challenge - largely in the handling of strings in Python 2 needing to be converted to bytes in Python 3, with conversions for the encoding rarely done - consequently it's not something I'm rushing to. So I appreciate that Python 2 keeps working for Unicorn.

So I appreciate any and all work to make it possible to still run Unicorn with Python 2.

[elicn]

@gerph, you are just delaying the inevitable, buddy (:

[gerph]

@gerph, you are just delaying the inevitable, buddy (:

Whilst that's true, every day is delaying the inevitability of death, so... let's not play down the decision to continue down the paths we choose to take before we get there :-)

The fact of the matter is that replacing all the many byte operations within the OS with bytes specific operations, ensuring that everything works the same way, etc, etc, is a non-trivial amount of work. Work that doesn't need to be done - the system runs as it is, and is still as functional as it needs to be. There is an opportunity cost in moving to Python 3, which I have decided is something I don't wish to take. Experiments a year or so back showed that many operations were slower with Python 3 than with Python 2, and so speed isn't of benefit. The support for other packages might help, but then the packages support that the system needs is relatively stable, and can be updated easily. The deployment environment is either local or cloud based, so doesn't force an upgrade.

More specifically, the support for Python 2 in Unicorn 2 has been ropey or broken in the last few releases - each official release has required special patching to make Unicorn 2 work in Python 2 - and haven't been great enough to force an update to Unicorn 2. So the additional facilities offered by Unicorn 2 are not currently a driving factor to upgrade to Python 3, either.

I understand the situation with Python 2, and - as I said - I appreciate the work being done to keep Python 2 supported. My reinforcement that the work is appreciated, and that there are users who continue to use Python 2, was the intent of my comment.