Not-to-be-merged review of Python bindings generator
Needs ReviewPublic

Authored by shaheed on Sep 8 2017, 2:17 PM.

Details

Reviewers
lbeltrame
Summary

As per https://marc.info/?l=kde-core-devel&m=150464598710128&w=2, this is a snapshot of the current state of the SIP-based Python binding generator intended for KF5, KDE more generally, and beyond.

Beyond what I mentioned in the post, the key differences compared to what is in ECM might be summarised as follows:

  • The core logic has several sets of fragile heuristics replaced by more solid logic based on Clang structures. (The remaining heuristics cannot, as far as I can see from the internals of Clang, be done away with).
  • There is an approach to portability built atop CMake, but intended to support operation as a standalone utility, as opposed to being integrated into ECM.
Test Plan

n/a

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
shaheed created this revision.Sep 8 2017, 2:17 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptSep 8 2017, 2:17 PM
Restricted Application added subscribers: Build System, Frameworks. · View Herald Transcript
shaheed edited the summary of this revision. (Show Details)Sep 8 2017, 2:18 PM
shaheed retitled this revision from Not-to-be-merged review fo Python bindings generator to Not-to-be-merged review of Python bindings generator.
shaheed edited the summary of this revision. (Show Details)
lbeltrame edited edge metadata.Sep 8 2017, 2:29 PM

A small number of reviews (this thing is huge). One question: would it be possible to have the bindings per-framework, rather than a single, long list? This is also what made PyKDE4 unwieldy. IOW, each Framework should ship their (optional) bindings.
We can put the tooling in ECM so that everything is in place for all the Frameworks. (This is how the current approach works):

find-modules/module_generation/CMakeLists.txt
49

Don't use find_package(FOO REQUIRED). Use find_package(FOO) then set_package_properties(FOO TYPE REQUIRED.... There are many examples in KDE git you can use. You will need to include FeatureSummary for this to work properly.

Why do I say this? Because now CMake will fail *immediately* at the first error. Otherwise, it will collect and print all missing packages after configuration. It is much easier on the packagers.

53

Any specific reason for this SIP version?

find-modules/module_generation/FindClang.cmake
52

This won't work on all distros. Can you either:

a. Take inspiration from KDevelop's FindClang (which works already nicely)
b. Use upstream Clang module to find it and its version?

The same comment goes for FindLibClang.

find-modules/module_generation/HOWTO
12

Before distro-specific instructions, put a list of all the dependencies with their upstream names. This ensures that packagers of non Ubuntu/Debian distros can also look them up.

My comments here are phrased as if this SIP-based approach was the solution eventually adopted (cppyy might be different). With that said...

This...

(this thing is huge).

and this...

One question: would it be possible to have the bindings per-framework, rather than a single, long list? This is also what made PyKDE4 unwieldy. IOW, each Framework should ship their (optional) bindings.

are related. Though the diff is huge, much of that is due to the 98 files in the PyKF5 directory (as opposed to the 10 that actually contain the crown jewels, plus the 5 template-related files). Let me explain...

Those 98 files are effectively two things:

  1. The basis of the regression test suite for the tooling. In this sense, these rules (more or less) do just enough work to get beyond the fail-on-first-error semantics of the SIP compiler, and so are needed to allow enough of the syntax of the KF5 suite in the long list to be processed for me to feel comfortable that the tooling is "complete".
  2. The starting point for the real per-framework rules that would be used in real life. In real life, I would expect the bindings to be packaged per-framework via ECM as you say, with each framework owner starting from the set of rules here.

We can put the tooling in ECM so that everything is in place for all the Frameworks. (This is how the current approach works):

The tooling in ECM would likely also host the shared rules (including the 5 files worth of template support) much as the current ECM does.

shaheed added inline comments.Sep 8 2017, 5:26 PM
find-modules/module_generation/CMakeLists.txt
49

Good point. I even helped update the Techbase docs for this, but then forgot to implement it :-).

53

Yes, this contains needed patches. Though it turns out that even this will not be sufficient because I submitted further patches for more issues. Unfortunately, upstream decided to implement some of those patches using Python3 only and I'm still stuck on Python2 because of the CLang dependency.

CLang Python3 support was shipped yesterday.

When it is available, a newer version of SIP that contains the support for scoped enums as per https://www.riverbankcomputing.com/pipermail/pyqt/2017-August/039494.html will need to be integrated (the core logic here may or may not need to change slightly).

find-modules/module_generation/FindClang.cmake
52

I'm well aware of the issue. Unfortunately, upstream CLang was not available to me, and the KDevelop one is not sufficiently complete to do what I need.

Hence this would need addressing in due course.

find-modules/module_generation/HOWTO
12

Yup. I'd likely need a bit of help at that point, but I would hope that using KDE's CI system would be a good starting point.

Another issue that would need to be addressed is the fact that the newer versions of SIP will only support Python3 as noted above.

My suggestion would be to focus any review efforts in this order:

  1. files in the top level directory.
  2. possibly files the ./templates subdirectory.
  3. __init__.py and CMakelists.txt in the ./PyKF5 subdirectory. Ignore all other files therein.

I hate to add a ping without any useful review, but I'm quite interested in this effort as I have a pykde4-based application which I would really like to get ported to the modern frameworks. Current distro packages of pykde4 are increasingly broken, and coverage of the existing bindings isn't sufficient to get the stuff I'm using (which isn't much, really) running.

Is there anything that someone like me can do to help with this?

I hate to add a ping without any useful review, but I'm quite interested in this effort as I have a pykde4-based application which I would really like to get ported to the modern frameworks. Current distro packages of pykde4 are increasingly broken, and coverage of the existing bindings isn't sufficient to get the stuff I'm using (which isn't much, really) running.

Is there anything that someone like me can do to help with this?

Well, I only really got into this because I wanted to revive the Python support I added to Kate!

In all seriousness, I strongly doubt that a SIP-based solution for PyKF5 can ever be sustained without a dedicated maintainer, and my starting premise for my work was this was not a viable prospect. As hinted on the mailing list I've now spent the last 2 days with cppyy, and once I have made a bit more progress will post an update on that.

Thanks for the update; I'll follow the bindings list and work on getting cppyy into Fedora. Hopefully that will be relatively uncomplicated.