[RFC] Restore py2 compatibility by using 2 files

[wtdcode]

This should bring back Python2 compatibility.

Q: Does this affect the current Python3 code using Unicorn2?

In theory, no.

Q: Does this affect the previous Python2 code using Unicorn1?

In theory, no. But I would be glad if someone could check it.

Q: Why not keep it compatible in 1 file?

It's a tradeoff. We of course desire to reduce code duplication but the fact is that:

• Writing Py2-Py3 compatible code is painful and especially error-prone. I once maintained such code and finally gave up.
• Really few users are stuck at Python2. The proof is that no one reports Py2 breaks such a long time.
• Python3 offers many more features which makes developing and maintaining easy. Splitting into 2 files drops the burden of Py2 and makes it easy for new contributors.

Q: So what's the status of Py2 compatibility after this PR?

We will keep the bindings Py2 compatible with all features Unicorn1 offers. For new features, we are glad to review and accept pull requests.

[gerph]

Apologies for not reporting the Python 2 breakage previously - I really just assumed that you wouldn't care and honestly I don't blame you as it's my problem to keep things working. However, I would be happy to have Unicorn 2 support...

Having checked the behaviour of the patch in #1822 I shall try this branch. I'm going to describe what I did to test this - I suspect that this might be a little longwinded, but hopefully it'll indicate where I've made mistakes if I have assumed too much :-) ... When I've got it working (or broken), I will summarise my findings and suggestions...

Testing

git fetch origin
git checkout restore-py2
cd ../build
cmake .. -DCMAKE_BUILD_TYPE=Release -DUNICORN_ARCH=arm
cd ../bindings/python/
make bdist

This gets to the upload section which prints...

We need to know who you are, so please choose either:
 1. use your existing login,
 2. register as a new user,
 3. have the server generate a new password for you (and email it to you), or
 4. quit
Your selection [default 1]: 

At which point I press ctrl-c as I don't want to upload. It would be nice if there was a way to build by not upload.

The wheel is present in dist/unicorn-2.0.2-py2.py3-none-macosx_10_14_x86_64.whl and can be installed with pip:

pip install dist/unicorn-2.0.2-py2.py3-none-macosx_10_14_x86_64.whl

DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.
Processing ./dist/unicorn-2.0.2-py2.py3-none-macosx_10_14_x86_64.whl
Installing collected packages: unicorn
  Found existing installation: unicorn 1.0.2.cjf4
    Uninstalling unicorn-1.0.2.cjf4:
      Successfully uninstalled unicorn-1.0.2.cjf4
Successfully installed unicorn-2.0.2

I can run my version check and see that it's actually installed properly and should be good to go...

charles@laputa ~/projects/RO/pyromaniac (add-tests-for-sprite-creation-with-mode-words-selectors)> pyrodev --version full
Pyromaniac 0.46 (04 Apr 2023)
Host platform: Darwin/x86_64
System emulation: Unicorn 2.0
Python version: 2.7.16 (default, Jun 19 2019, 07:40:37) 

And I can launch the desktop - so that's working well :-)

Findings

This means that it's working as well as it does with Unicorn 1. I'm happy with this change :-)

[gerph]

I had already added support for the Unicorn 2 features for selection of the CPU type. In order to do this I provide a configuration option that uses the names in the arm_const.py file which start with UC_CPU_ARM_*.

My code was:

# Mapping from the model name to the model constant to pass to Unicorn
# We must exclude the CORTEX_M processors as they do not support ARM (and we must use ARM
# for anything to work, right now).
cpu_models = {name[11:]: getattr(unicorn.arm_const, name)
                for name in dir(unicorn.arm_const)
                    if name.startswith('UC_CPU_ARM_') and not name.endswith('_MAX') and not 'CORTEX_M' in name}

And this had worked fine... however, it now offers an extra CPU model called ENDING. This is because the arm_const.py file contains:

UC_CPU_ARM_PXA270C5 = 32
UC_CPU_ARM_MAX = 33
UC_CPU_ARM_ENDING = 34

I'm pretty sure that this isn't related to the changes for Python 2, but I mention it because the support I had, doesn't allow for this. For now I shall exclude a CPU called ENDING but others might find similar.

In any case, with this code change made, I can now select different CPU models with my configuration interface using the Unicorn 2 system. This makes me happy :-)

[wtdcode]

@gerph Thanks for your thorough testing! For the ENDING problems I believe it's another issue and we added it long time ago?

[gerph]

@gerph Thanks for your thorough testing! For the ENDING problems I believe it's another issue and we added it long time ago?

Sounds likely. Seems a bit odd having both a MAX and ENDING, but I've worked around that in my CPU model selection code anyhow.

[wtdcode]

MAX is the cpu model with all features enabled while ENDING is the ending of the enum list. That’s a bit different.

…

________________________________ From: Charles Ferguson ***@***.***> Sent: Saturday, April 29, 2023 1:51:46 AM To: unicorn-engine/unicorn ***@***.***> Cc: lazymio ***@***.***>; Author ***@***.***> Subject: Re: [unicorn-engine/unicorn] [RFC] Restore py2 compatibility by using 2 files (PR #1823) @gerph<https://github.com/gerph> Thanks for your thorough testing! For the ENDING problems I believe it's another issue and we added it long time ago? Sounds likely. Seems a bit odd having both a MAX and ENDING, but I've worked around that in my CPU model selection code anyhow. ― Reply to this email directly, view it on GitHub<#1823 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHJULO44KUMDMBC6A7OHPULXDRJZFANCNFSM6AAAAAAWYZOILY>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[gerph]

Ah! It is a real CPU model that can be selected?! I hadn't realised that. Ok, I will remove it from my list of excluded constants Thank you :-)

[wtdcode]

Ah! It is a real CPU model that can be selected?! I hadn't realised that. Ok, I will remove it from my list of excluded constants Thank you :-)

Correct, maybe we should doc this somewhere... The naming is converted from QEMU directly so users without playing with QEMU too much are not familiar with these names.

[wtdcode]

Rework a bit to merge back #1629