Suggestion: When doing the refactoring, drop support for Python 2 and older IDA

[Harding-Stardust]

Having all the code for Python 2 and older IDA makes the code way longer and harder to read.
Is the extra time and effort really worth it?

[arizvisa]

How do you mean "way longer"? Are you referring to the encoding of strings to bytes for communicating with IDAPython? The majority of the py2-compatibility exists entirely within the internal.utils module (other than a stray check for using the correct method).

Personally, though, I'm not actively maintaining support for py2. A lot of that code has existed before py3 became a thing and hence it still works the same way. However, do you have a specific example that you're referencing?

[Harding-Stardust]

So for Python 2 I was thinking that if you dropped that, then you could be using type hints and f-strings which are both very nice upgrades in newer Python.

I also was seeing lots and lots of if idaapi.version < X.X that made the code longer, just an example I found when I was browsing aroung in the code:

def changes(cls):
        '''Return the number of changes within the database.'''
        if idaapi.__version__ < 7.0:
            raise E.UnsupportedVersion(u"{:s}.changes() : This function is only supported on versions of IDA 7.0 and newer.".format('.'.join([__name__, cls.__name__])))
        elif idaapi.__version__ < 7.2:
            return cls.info.database_change_count
        return idaapi.inf_get_database_change_count()

Which if you dropped old IDA support would be just

return idaapi.inf_get_database_change_count()

[arizvisa]

Actually, that idaapi.__version__ is specifically patched in by the plugin for that capability. In all the cases where it's performance-sensitive, I take care to use partial evaluation so that the callable being used does not execute with those version checks. However, a lot of that versioning isn't something that was explicitly added as a feature but rather it has just organically grown that way over the years.

This version checking is not something I plan on actively removing, but also not something that I plan on continuing to add unless explicitly requested (like some of the first issues that were created in this repo). The reason being is that due to IDA's licensing scheme, there are some versions of the disassembler out there that cannot be run headless and in parallel. With regards to older versions more lenient licensing schemes, however, this is more practical. Personally, prior to beginning any sort of reversing project, I utilize gnu parallel to build all my databases with an older version before distinguishing what is actually worth looking at.

Also, having support for compatibility allows you to run the same code for a newer version on an older version of the database and still expect the same results. Originally this plugin was oriented around the exchange of information between the disassembler and any random Python interpreter. This way you can collaborate and exchange types or information (artifacts) with other users and more importantly debuggers (which can be considered other users anyways).

So, it is not a guarantee that everybody will have the same license as you, nor the same version. But, with this plugin it doesn't matter as you can serialize your state and bring it to any version of the disassembler that enables IDAPython.

[arizvisa]

Converting this to a discussion.