End of 2016 update on PyKF5 bindings

Wed Jan 4 17:14:40 UTC 2017

Hi Steve,

I'm all rebased onto KDE master. There are 4 pending reviews as follows:

https://git.reviewboard.kde.org/r/129759/
https://git.reviewboard.kde.org/r/129760/ which depends on '759
https://git.reviewboard.kde.org/r/129763/
https://git.reviewboard.kde.org/r/129765/

The first three are fixes to the processing of C++ syntax. The 4th one
reverts an API change which lays the ground for reintroducing the tools I
need to go further. Those changes are not yet posted for review pending
getting the currently queued changes in, but can be seen in clean logical
chunks at
https://github.com/ShaheedHaque/extra-cmake-modules/commits/KDE_restore_rules_engine
if you are interested. Roughly:

   1. The first of the changes there adds back the databases to allow full
   control over the generated code (and are used by the last commits in the
   branch) and my development/regression testing tools.
   2. The second adds extensive diagnostics in the form of source comments
   to make it easier to diagnose what changes were made by rules.
   3. Changes the way rules databases are initialised (and, new, appended
   to) to make it easier to support the workflow you developed by greatly
   reducing the boilerplate needed, and better supporting #2. There is
   backwards compatibility in there for the way you are presently working too,
   so I can probably help migrate your code after it lands to take advantage
   of this.

Changes #4-#7 take things to the point where KAuth, KCoreAddons, KCodecs,
KCompletion and KConfigCore run cleanly through the SIP compilation
process. I stole freely from your efforts to get there, but these are
subsets of what you did i.e. just enough to act as regression tests upto
the SIP compilation stage. The code for the tools part of #2 and #4-#7 all
live in a subdirectory.

On 3 January 2017 at 23:08, Shaheed Haque <srhaque at theiet.org> wrote:

> FWIW, I was able produce a test case for the CursorKind.ENUM_CONSTANT_DECL
> change. I'll submit a patch with the test in due course.
>
> On 3 January 2017 at 12:52, Shaheed Haque <srhaque at theiet.org> wrote:
>
>> On 3 January 2017 at 10:18, Shaheed Haque <srhaque at theiet.org> wrote:
>>
>>> Hi Steve,
>>>
>>> On 2 January 2017 at 23:15, Stephen Kelly <steveire at gmail.com> wrote:
>>>
>>>> Shaheed Haque wrote:
>>>>
>>>> >> Can you extract self-contained commits and propose them for that
>>>> repo?
>>>> >
>>>> > What is the review procedure for extra-cmake-modules?
>>>>
>>>> I think there is a reviewboard or something for it.
>>>>
>>>> Posting a link to commits anywhere (eg a branch on github) works for me
>>>> anyway too as I find reviewboard unusable for multi-commit branches.
>>>>
>>>
>>> I was mostly wanting to check whether reviewboard or phabricator was in
>>> use, but github is even better. Thanks!
>>>
>>>
>>>> > There is also one
>>>> > change which needs to be made in a synchronised fashion between
>>>> > extra-cmake-modules and kguiaddons, any suggestions on how to handle
>>>> that
>>>> > are welcome.
>>>>
>>>> Sure, if you post it, we can think about how to do it. I only pushed the
>>>> kguiaddons bindings today. I am curious because the patch was very
>>>> small and
>>>> I wonder what could possibly need to be done in concert.
>>>>
>>>
>>> I misspoke, it wasn't in kguiadddons, but basically the ModuleCodeDb
>>> dict was documented as being keyed by the filename, but I implemented it
>>> wrongly, and you used a key as per the implementation, not the docs :-). We
>>> can deal with the conflict, by temporarily duplicating the dict entry,
>>> ahead of the core logic changing, and then removing the original, but it
>>> all just needs to be done in order, perhaps with a couple of days between
>>> each change to avoid tripping folk up.
>>>
>>>
>>>> >> You seem to have changed some things in your clone such as using
>>>> >>
>>>> >>  enum.kind == CursorKind.ENUM_CONSTANT_DECL
>>>> >>
>>>> >> Please extend the unit test in tests/GenerateSipBindings for that and
>>>> >> other similar items.
>>>> >>
>>>> >
>>>> > I'm always looking for opportunities to add them but in most cases,
>>>> the
>>>> > fixes modify the text output for a given source in another framework
>>>> > component. It is not obvious how to create a simple self-contained
>>>> test
>>>> > case.
>>>>
>>>> Well, what does that change do? You have a comment saying "Skip
>>>> visibility
>>>> attributes and the like.". What does that mean? You encountered an issue
>>>> when parsing an enum with a visibility attribute? Then open
>>>>
>>>>  tests/GenerateSipBindings/cpplib.h
>>>>
>>>> put an enum with an attribute somewhere, run the tests, watch them fail,
>>>> make your fix, run the tests and watch them pass.
>>>>
>>>> Isn't this extremely obvious? What am I missing? Do I misunderstand your
>>>> difficulty?
>>>>
>>>
>>>> > For example, in the case you noticed, the point of the change only
>>>> > becomes apparent when libclang decides to work in a particular
>>>> way...and I
>>>> > have no idea how in general to provoke those cases.
>>>>
>>>> Start with whatever provokes it with whatever header is being
>>>> processed, and
>>>> then simplify. Again, I must be missing something because this is really
>>>> obvious, and I'm certain you know how to do this. I must be missing
>>>> something.
>>>>
>>>
>>> Please let me set some context.
>>>
>>> Remember my present focus is on making sure the tools run, and produce
>>> output that the SIP compiler does not barf on across huge portions of the
>>> KDE codebase. Specifically, as I have hinted before, I need to be
>>> reasonably sure that I can handle the breadth without changes to the rule
>>> format as my primary goal. I perceive that a failure to get that right will
>>> result in a repeat of the failure to smoothly migrate PyKDE4 to KDE5 (which
>>> is why we are even here!).
>>>
>>> Thus, my regression test environment generates 2280 non-duplicate .SIP
>>> files across 133 components. Of this set, and building on the work you did
>>> on github, I can currently run 5 of those components cleanly through the
>>> SIP compiler. So, I'm not even concerned at this point in whether the
>>> resulting bindings work (of course, I use/verify the unit tests you already
>>> wrote for the various components on github, though I've not yet checked the
>>> status of those on KDE master).
>>>
>>> My *main* regression tests therefore include carefully diff'ing the 2280
>>> files from run to run, gradually picking off errors as I can to get to the
>>> point of a clean SIP compiler run across the 133 components in turn. (BTW,
>>> that is also the reason for the continued existence of
>>> sip_buik_generator.py and sip_compiler.py as necessary tooling to support
>>> this development and testing). It was this test process that found this bug
>>> when an assert was triggered.
>>>
>>> Now, if I could easily see what in the source code of the enum causes
>>> libclang to emit an attribute, I would have considered writing the test,
>>> but based on the ROI of the effort involved in doing what you suggest
>>> (which is, I agree, theoretically the right thing to do) just did not meet
>>> the bar of a simple-to-develop test, given the scope of what else I need to
>>> get done. That is, not firing on the assert seemed, and frankly seems, good
>>> enough to me in this case.
>>>
>>> > My focus beyond the PRs I posted has been to make the system easier to
>>>> use
>>>> > and diagnose, especially for your workflow.
>>>>
>>>> Great, I think I saw some of that stuff with follow-up 'oops - fix
>>>> something' commits. If you can present that work in commits which are
>>>> already 'fixed' I'd love to see them.
>>>>
>>>
>>> Absolutely, that is a given.
>>>
>>> >> > If we can get the PRs merged, and work out item 4 above,
>>>> >> > what else might be needed to getting this into production?
>>>> >>
>>>> >> There are still packaging related issues, but I don't know how to
>>>> solve
>>>> >> them
>>>> >> and I would look to others to provide guidance there.
>>>> >>
>>>> >
>>>> > Likewise, assuming the rebase does not fix it, I'm sure I'll need help
>>>> > with item 4.
>>>>
>>>> I'm sure I can help with that soon enough.
>>>>
>>>
>>> Excellent. Having had a quick look at the code in the KDE repos, I see
>>> that there are quite some changes, notably to rules_engine.py which removes
>>> some of the test facilities I need to revive the sip_builk_generator.py
>>> test workflow. I'll see if I can package those changes with collateral that
>>> will ease any concerns with the testing that is being done.
>>>
>>> Thanks, Shaheed
>>>
>>>
>>>> Thanks,
>>>>
>>>> Steve.
>>>>
>>>
>>>
>> OK, step 1. After a few "how does reviewboard work again?" moments, I've
>> posted:
>>
>> https://git.reviewboard.kde.org/r/129759/
>> https://git.reviewboard.kde.org/r/129760/ which depends on '759
>>
>> Thanks
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-bindings/attachments/20170104/a93e52a4/attachment.html>