Differential D14912

Bindings: Make generator forward compatible with Python 3
ClosedPublic
Actions

Authored by bruns on Aug 17 2018, 10:36 PM.

Details

Reviewers
arojas
Group Reviewers
Frameworks
Commits
R240:b6e4e645dac6: Bindings: Make generator forward compatible with Python 3
Summary

iteritems is no longer an available method for dict in Python 3. Using
dict.items() is functionally identical, although it creates some overhead
for Python 2.7 (creation of a temporary list). As this is only called
when tracing, this is not a big issue. For Python 3, there is no
overhead (dict.items() returns an iterator).

Test Plan

Run python3 sip_generator.py ...
Run python2 sip_generator.py ...
Both generate the same code

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.
bruns created this revision.Aug 17 2018, 10:36 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptAug 17 2018, 10:36 PM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Aug 17 2018, 10:36 PM
arojas added a subscriber: arojas.Aug 18 2018, 7:34 AM

This adds a new build time dependency to all frameworks which support bindings. Any chance to simply port it to python 3 instead?

bruns added a comment.Aug 18 2018, 1:15 PM
In D14912#311063, @arojas wrote:

This adds a new build time dependency to all frameworks which support bindings. Any chance to simply port it to python 3 instead?

Dropping python2 means dropping support for any distro not providing python3 clang bindings. AFAICS e.g. Fedora only has python2 bindings.

I consider future a very minor addition, compared to everything else needed for generating the bindings. And there still is the option to not generate the bindings at all (which is a quite popular choice).

bruns added a comment.Aug 18 2018, 2:53 PM

Another option is to include a wrapper function directly in the code, instead of importing it

bruns added a comment.Aug 22 2018, 8:20 PM

Third option: just use original.items(). This is slightly more expensive in python 2 (it generates a temporary list<k,v> instead of passing an iterator), but is functionally identical. I don't think the overhead is an issue here - it is only called when tracing. For python 3, future.utils.viewitems(dict) is identical to dict.items().

bruns updated this revision to Diff 40256.Aug 22 2018, 8:28 PM
bruns edited the summary of this revision. (Show Details)
bruns removed a subscriber: arojas.

Use dict.items() for Python 2.7 and Python 3

bruns added a subscriber: arojas.Aug 25 2018, 2:17 AM
arojas added a comment.Aug 25 2018, 7:49 AM

Python3 still can't be used after this change: python2 is explicitely required at FindPythonModuleGeneration.cmake:177 (and all subsequent calls to sip_generator.py are done with ${GPB_PYTHON2_COMMAND}). This can be adjusted in another review though.

bruns added a comment.Aug 25 2018, 2:46 PM
In D14912#315093, @arojas wrote:

Python3 still can't be used after this change: python2 is explicitely required at FindPythonModuleGeneration.cmake:177 (and all subsequent calls to sip_generator.py are done with ${GPB_PYTHON2_COMMAND}). This can be adjusted in another review though.

Already there - D14914. Also check D14915, D15068, D15070

arojas added a comment.Aug 26 2018, 5:34 PM

With this + D14914 + D14915 I can successfully compile bindings without having any python2 package installed.

bruns added a comment.Aug 26 2018, 6:53 PM
In D14912#315733, @arojas wrote:

With this + D14914 + D14915 I can successfully compile bindings without having any python2 package installed.

Thats on Arch?

arojas added a comment.Aug 26 2018, 6:59 PM
In D14912#315763, @bruns wrote:

Thats on Arch?

Yes

bruns added a comment.Aug 31 2018, 1:07 PM

So, tested on Arch, openSUSE TW and Leap 15.0 ...
@arojas - can you accept, also for the two other reviews?

arojas accepted this revision.Aug 31 2018, 1:08 PM
This revision is now accepted and ready to land.Aug 31 2018, 1:08 PM
Closed by commit R240:b6e4e645dac6: Bindings: Make generator forward compatible with Python 3 (authored by bruns). · Explain WhyAug 31 2018, 9:41 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mfind-modules/rules_engine.py (4 lines)

Diff 40793

View Options

find-modules/rules_engine.py

Loading...