Optional Python 2 Support for Krita
ClosedPublic

Authored by jhoolmans on May 12 2018, 4:34 PM.

Details

Summary

Adding Python 2 support is a nice to have feature for the VFX industry. They are slowly moving towards Python 3 for their libraries (vfxplatform.com) but until then it's good to support compiling Krita with Python 2.

Python plug-ins now respond to the X-Python-2-Compatible option. Whenever they are not compatible, they won't be loaded by the Plugin Manager.

A couple of plugins have been converted to support both versions and are marked compatible for 2. This means it is compatible with 3 by default.

Tested on Linux (Fedora 26)

Test Plan

Dependencies:

packages:

  • Python 2 devel
  • PyQt5 for Python 2
  • Sip for Python 2

Some extra dependencies to run supplied python plugins.

pip:

  • trollius (python 2 port of asyncio)
  • pathlib (not included by default in python 2)

Compile and try:

  1. Compile Krita with -DENABLE_PYTHON_2=ON
  2. Run Tools->Scripting->Scripter (if enabled)
  3. Verify Python version:
import sys
print(sys.version_info)
  1. Try out some krita API commands
  2. Notice missing python plugins in the plugin manager such as the Comic project manager.

Verify Python 3 still works

  1. Compile with -DENABLE_PYTHON_2=OFF (this is OFF by default)
  2. See steps from Python 2 above but notice more plugins in the plugin manager

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
jhoolmans requested review of this revision.May 12 2018, 4:34 PM
jhoolmans created this revision.
rempt added a subscriber: rempt.May 15 2018, 8:36 AM
rempt added inline comments.
cmake/modules/FindPythonLibrary.cmake
31

This shouldn't be 3.6 -- 3.0 should be fine here. Otherwise building with Python on a reasonably recent distribution with 3.4.6 fails:

boud@linux-u0cn:~/dev/b-krita> make -j4 install

  • Using CMake version: 3.5.2
  • Krita version: 4.1.0-pre-alpha
  • Release build: TRUE
  • Found PythonInterp: /usr/bin/python3 (found suitable version "3.4.6", minimum required is "3.0")

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:148 (message):

Could NOT find PythonInterp: Found unsuitable version "3.4.6", but required                                    
is at least "3.6" (found /usr/bin/python3)

Call Stack (most recent call first):

/usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:386 (_FPHSA_FAILURE_MESSAGE)                      
/usr/share/cmake/Modules/FindPythonInterp.cmake:163 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)                        
cmake/modules/FindPythonLibrary.cmake:31 (find_package)                                                        
CMakeLists.txt:246 (find_package)
  • Configuring incomplete, errors occurred!

See also "/home/boud/dev/b-krita/CMakeFiles/CMakeOutput.log".
See also "/home/boud/dev/b-krita/CMakeFiles/CMakeError.log".

rempt accepted this revision.May 15 2018, 9:10 AM

I've tested it and it works, and apart from the remark I already posted, I don't see any problems. Please get a developer account and push it :-)

This revision is now accepted and ready to land.May 15 2018, 9:10 AM
alvinhochun requested changes to this revision.May 15 2018, 9:11 AM
alvinhochun added a subscriber: alvinhochun.

I would suggest just not allow Python 2.7 to be used with Windows/MinGW because the whole Python setup for the Windows build is pretty much tied to the embedded Python 3.6 package.
Do note that the Python setup for the Windows build is pretty specific and even requires a certain version range of Python to be installed on the build host to build properly.
Unless you also modify ext_python, ext_sip, ext_pyqt, the Python plugin code and the packaging scripts to be able to produce a package with Python 2.7 embedded into it, it's basically not usable.

And AFAIK there isn't an official embeddable Windows release for Python 2.7. I would not suggest allowing a user install of Python because a lot of Windows users can't be trusted to set things up properly and will often lead to a broken setup and an angry user.

3rdparty/CMakeLists.txt
147โ€“152

I would suggest to just not allow the flag, perhaps make it a fatal error.

CMakeLists.txt
222โ€“227

Same as above...

plugins/extensions/pykrita/plugin/utilities.cpp
418

Please don't change this part... I'm 90% sure this will break something with the Windows build. The path passed to Py_SetPath is specifically crafted to handle the embedded Python used with the Windows build.

This revision now requires changes to proceed.May 15 2018, 9:11 AM
rempt added a comment.May 15 2018, 9:16 AM

That's right -- but we don't need Python 2 support on Windows. The only need for python2 is with the vfx platform people. So we could just ifdef that.

I agree, I haven't tested this with Windows and assumed this would be a solution since Py_SetPath isn't available in Python 2. Only the PySys_SetPath.
We do have a big chunk of machines on Windows (sadly), that would have to fall back to Photoshop, But that's fine for now.

I'll make the changes tonight and send an updated diff.

Maybe you can find a way to make it work on Windows using a user-specified Python 2.7 install and self-built SIP/PyQt, but it must use separate code paths that is switched by an ifdef.

jhoolmans updated this revision to Diff 34219.May 15 2018, 6:45 PM
jhoolmans edited the summary of this revision. (Show Details)

For now I've disabled the option for Windows.
This needs some more investigation.

alvinhochun accepted this revision.May 15 2018, 6:56 PM

Looks good to me now. I haven't tested it but I believe the remaining changes shouldn't specifically affect the Windows build.

This revision is now accepted and ready to land.May 15 2018, 6:56 PM
This revision was automatically updated to reflect the committed changes.