Differential D15091

Compile python bindings with the same sip flags used by PyQt
ClosedPublic
Actions

Authored by arojas on Aug 26 2018, 7:00 PM.

Details

Reviewers
bruns
Group Reviewers
Frameworks
Commits
R240:05bd97492704: Compile python bindings with the same sip flags used by PyQt
Summary

PyQt>=5.11 is compiled with the 'PyQt5.sip' sipname by default. This causes a sipname mismatch with KF5 bindings and prevents them from loading:

----> 1 from PyKF5.KCoreAddons import KJob
RuntimeError: the PyQt5.QtCore module failed to register with the sip module

To fix this, we compile KF5 bindings using the same sip flags (name and tags) that PyQt was compiled with. This ensures that we always use the correct sip name. We also stop setting the sip tags in FindPythonModuleGeneration.cmake to avoid duplication, and remove a (seemingly) unnecessary check for the Qt version that was forcing PyQt to be rebuilt for every patch Qt update.

Test Plan

Compiled kcoreaddons against PyQt 5.10.1 (with default 'sip' sipname), PyKF5.KCoreAddons is correctly imported
Compiled kcoreaddons against PyQt 5.11.2 (with 'PyQt5.sip' sipname), PyKF5.KCoreAddons is correctly imported
Without this patch, they fail to load with PyQt 5.11.2

Diff Detail

Repository
R240 Extra CMake Modules
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
arojas created this revision.Aug 26 2018, 7:00 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptAug 26 2018, 7:00 PM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
arojas requested review of this revision.Aug 26 2018, 7:00 PM
bruns added inline comments.Aug 26 2018, 7:16 PM
find-modules/run-sip.py
11

Is there a gurantee -n and its arg are space separated?

otherwise:

import re
m = re.search('-n\s*(\S+)', PYQT_CONFIGURATION["sip_flags"])
if m:
  sipArgs = ['-n', m.group(1)] + sipArgs
arojas updated this revision to Diff 40479.Aug 26 2018, 8:04 PM

Use a regex to account for the case where -n an its arg are not space-separated

find-modules/run-sip.py
11

I find it highly unlikely that they aren't, but it's certainly possible

arojas marked an inline comment as done.Aug 26 2018, 8:05 PM
rdieter added a subscriber: rdieter.Aug 27 2018, 3:13 PM

Why are you simply not using all PYQT_CONFIGURATION["sip_flags"]?

arojas added a comment.Aug 27 2018, 9:54 PM
In D15091#316150, @rdieter wrote:

Why are you simply not using all PYQT_CONFIGURATION["sip_flags"]?

Not sure if we really want to do this. For instance, bindings would get tagged with the Qt version that PyQt was compiled with, not with the one KF5 is being compiled with (not sure if that would cause any actual issue)

rdieter added a comment.Aug 27 2018, 9:56 PM

I was asking mostly because that's what pyqt upstream strongly suggested (to use all sip_flags) when I asked them.

arojas added a comment.Aug 28 2018, 8:39 AM
In D15091#316243, @rdieter wrote:

I was asking mostly because that's what pyqt upstream strongly suggested (to use all sip_flags) when I asked them.

Oh, if upstream recommends it then I guess it should cause no issues. That would allow dropping quite some code from FindPythonModuleGeneration.cmake. And, as a bonus, if would fix the annoying issue that KF5 bindings require PyQt5 to be recompiled (and patched) for every new patch version of Qt. Is there an upstream thread/post for reference?

arojas planned changes to this revision.Aug 28 2018, 1:55 PM
arojas updated this revision to Diff 40570.Aug 28 2018, 4:58 PM
arojas retitled this revision from Compile python bindings with the same sipname used by PyQt to Compile python bindings with the same sip flags used by PyQt.
arojas edited the summary of this revision. (Show Details)
bruns added a comment.Sep 20 2018, 12:32 PM

Can you rebase, please?
And upload the patch with arc diff, to have the full context available.

bruns requested changes to this revision.Sep 20 2018, 12:32 PM
This revision now requires changes to proceed.Sep 20 2018, 12:32 PM
bruns accepted this revision.Sep 30 2018, 8:42 PM

Confirmed to work with PyQt 5.10.1 and Qt 5.11.1

This revision is now accepted and ready to land.Sep 30 2018, 8:42 PM
Closed by commit R240:05bd97492704: Compile python bindings with the same sip flags used by PyQt (authored by arojas). · Explain WhySep 30 2018, 8:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 42625

View Options

find-modules/FindPythonModuleGeneration.cmake

Loading...
View Options

find-modules/run-sip.py

Loading...