Check whether addons are Python 3 compatible

[mzfr]

This is a generalization of @Rechi's comment here.

I was wondering if we should add a checker that detects whether the addon code is Python 3 compatible?

A very simple way is to just run 2to3 on the code and see if it detects any issues. We could try out other tools like modernize etc. as well. Even though these tools mostly detect if any cosmetic changes are required. They serve as a good first step towards python 3 compatibility.

We could log their output as warning or even suggest changes like use print() rather than print inline as a comment on the pull request itself.

An extension to this would be to just run 2to3 on the code that has been touched in the pull request, rather than running it on the entire file.

Please let me know if this sounds reasonable and I can start working on a pull for this!

[razzeee]

I love that idea, especially as it will help us migrate :)

[Rechi]

2to3 has differnt transformations which you can all run separately.

I would add a config setting where you can add a list of transformations.
The checker should then run the fixes on by one (2to3 -w -f except -- <ADDON BASE PATH>) and check if anything has changed (git diff --quiet -- <ADDON BASE PATH>). After each check reset the changes (git checkout HEAD -- <ADDON BASE PATH>) so you can run git diff again.
The usage of git for checking if anything has changed is just a suggestion and you can do it in any other way if you want.

The following transformations should can be applied for all Kodi/XBMC versions: dict
except, filter, has_key, import, itertools, map, ne, next, print, renames, types, xrange and zip
set_literal can be only run for Krypton (v17) and higher as it is only python 2.7 (and not 2.6) compatible.

[mzfr]

While thinking about this a bit more, I looked for any other alternatives to 2to3 that we could use. Having something that acted as a linter would be nice - so the tool should just report issues rather than fix them (which is what 2to3 does.)

Sadly, there isn't anything already existing that meets our requirements.

I did find this though, which is a flake8 plugin that has some 2to3 cases.

Do you think creating something like this for our particular usecase would make sense?

[razzeee]

Hrm, maybe we can just talk to the cpython guys or do a PR? Just do an argument like dry run and report the changes instead of making them. The source for 2to3 is here https://github.com/python/cpython/blob/master/Lib/lib2to3/main.py

[Rechi]

What output would you expect from the tool?
If it's just like 'Fix print usage' or 'Fix except syntax', that can be achieved easily with the process I described above. You could even add the filenames which need those fixes.

[mzfr]

@Rechi Running 2to3 multiple times (for each type of fix) and then using git to figure things out just feels a bit too hacky for me. Sure, it'd work, but I feel we should first explore if there is a a better way to do it.

The problem is, we want to report issues and not really fix them!

Sadly, I haven't had much luck with 2to3 - it just isn't made for reporting and it can be done but will need significant changes to the codebase, which I'm not sure will be worth it.

The best direction I currently have is to create something like this: https://github.com/openstack-dev/hacking/blob/master/hacking/checks/python23.py

OpenStack Hacking is a tool very similar to what we're trying to build for addon-check - code quality tests and what not! It has been created as a flake8 plugin which contains a lot of custom checks.

One of the custom checks is to test whether the code is Python 3 compatible. We can just copy this verbatim and then add more Py2/3 checks to it (there's a lot of those in the lib2to3 codebase.)

What do you think about this approach?

/cc @razzeee

[razzeee]

Can you put a number on how many things we would need to add? Really not looking forward to reimplementing a known factor.

[Rechi]

The suggestion with running 2to3 multiple times and using git to track changes was just something that came quickly into my mind.
I've investigated further how 2to3 works and I created a PoC how to use 2to3 without abusing git.

from lib2to3 import refactor

class KodiRefactoringTool(refactor.RefactoringTool):
	def print_output(self, old, new, filename, equal):
		if not equal:
			print(filename)


for fix in ['dict', 'except', 'filter', 'has_key', 'import', 'itertools', 'map', 'ne', 'next', 'print', 'renames', 'types', 'xrange', 'zip']:
	print(fix)
	rt = KodiRefactoringTool(sorted(['lib2to3.fixes.fix_' + fix]), {}, sorted(['lib2to3.fixes.fix_' + fix]))
	rt.refactor(['repo-scripts/script.module.requests'], None, None)
	print("")

[mzfr]

@Rechi That is an amazing approach.

I extended it to also print a diff of changes required, or we can extract just the line that the error is occurring on:

import difflib

from lib2to3 import refactor


class KodiRefactoringTool(refactor.RefactoringTool):

    def print_output(self, old, new, filename, equal):

        # A fix has been made
        if not equal:
            print(filename)

            # This diff can be commented to the Pull Request itself?
            # So that users can see and fix?
            diff = difflib.unified_diff(old.splitlines(), new.splitlines())
            print("\n".join(diff))


# This will come from our config file
list_of_fixes = ['dict']

for fix in list_of_fixes:
    print(fix)

    fixer = ['lib2to3.fixes.fix_' + fix]
    rt = KodiRefactoringTool(fixer, options=None, explicit=fixer)
    rt.refactor(['/home/mzfr/dev/Demo/plugin.video.reddit_viewer/'])

    print("\n\n")

Except for the fact that this makes multiple passes over the addon code (which isn't really a problem since the code won't be big?) this approach is very good as it doesn't involve re-inventing the wheel!

We now just need to decide how we want to report the issues to the user:

1. Just print the 2to3 error that occurred (with filename and line number?)
2. Print the diff of the code (since 2to3 is already giving us that info we can pass it on to the user)
3. Skip reporting of these errors, and just fix the issues ourselves?

Here, by print I mean a comment on the user's pull request.

[razzeee]

Well code for a single addon might not be big. But code for a whole repo might be. So if we can optimize it, we should. Otherwise we will wait for the builds longer.

Well what about an --verbose flag? And defaulting to 1.

[mzfr]

I'll do some runs on the big plugin branches and report times that this approach takes.

[razzeee]

Please also do something on repo-resources, which should be fine, if you detect if it really is a py file. But just in case ;P
https://github.com/xbmc/repo-resources/tree/krypton

[mzfr]

Hey @razzeee I'm trying to understand more about how Kodi's Python plugins run. Few questions:

1. Plugins can be created in BOTH Py2 / Py3 right? (just confirming this) What is the version of Py3 that is supported? So for eg. Can I use features like fstrings?

2. Can you point me to the codebase which runs these plugins? I'm interested in how it detects whether a plugin is written in Python 2 / 3.

3. Do we have any stats on how many total plugins we have, and what percentage of them are Python 2 vs 3.

[Rechi]

1. Currently python addons have to be py2 compatible to work in Kodi. For v19 they have to be py3 compatible (https://kodi.tv/article/attention-addon-developers-migration-python-3).
2. not detected as we only support either py2 or py3 in one Kodi version
3. Hopefully currently all addons are py2 compatible. Some devs have already made their addons py3 compatible

[mzfr]

This is fixed by #100