KDE Frameworks 5.29.0

Sat Dec 10 14:47:56 UTC 2016

On samedi 10 décembre 2016 09:52:06 CET Martin Graesslin wrote:
> On Thursday, December 8, 2016 9:32:13 AM CET David Faure wrote:
> > On mercredi 7 décembre 2016 21:06:11 CET Kevin Funk wrote:
> > > On Wednesday, 7 December 2016 20:10:40 CET Albert Astals Cid wrote:
> > > > El dimecres, 7 de desembre de 2016, a les 10:08:18 CET, David Faure va
> > > > 
> > > > escriure:
> > > > > On lundi 5 décembre 2016 18:40:46 CET Martin Gräßlin wrote:
> > > > > > Am 2016-12-05 09:20, schrieb David Faure:
> > > > > > > On dimanche 4 décembre 2016 23:42:44 CET šumski wrote:
> > > > > > >> On nedjelja, 4. prosinca 2016. 00:37:52 CET David Faure wrote:
> > > > > > >> > Dear packagers,
> > > > > > >> > 
> > > > > > >> > KDE Frameworks 5.29.0 has been uploaded to the usual place.
> > > > > > >> > 
> > > > > > >> > New framework: prison
> > > > > > >> > 
> > > > > > >> > Public release next Saturday.
> > > > > > >> > 
> > > > > > >> > Thanks for the packaging work!
> > > > > > >> 
> > > > > > >> kconfig (r129382) breaks compilation of kdevplatform:
> > > > > > >> http://paste.opensuse.org/82016854
> > > > > > > 
> > > > > > > Indeed (but it's not the change from RR 129382, it's commit
> > > > > > > cd4e650
> > > > > > > from
> > > > > > > https://phabricator.kde.org/D3386
> > > > > > > 
> > > > > > > Seems to come from Inherits=BaseClass while BaseClass doesn't
> > > > > > > use
> > > > > > > arg="true".
> > > > > > > 
> > > > > > > Here's a testcase for the kconfig unittests. Martin, can you
> > > > > > > take
> > > > > > > a
> > > > > > > look?
> > > > > > 
> > > > > > The earliest I can have a look is probably on Friday, I'm sorry.
> > > > > > 
> > > > > > My suggestion is to revert my two commits and I'll redo for next
> > > > > > frameworks.
> > > > > 
> > > > > OK, done. New git tag and tarball:
> > > > > 
> > > > > kconfig v5.29.0-rc2
> > > > > 47f7e954a58ba5538d055e2f75e483cade48ee8a
> > > > > d6c12e0908de1b91529de15e75a52c9974685c91b423d5b5abeb06f261d0fa47
> > > > > sources/kconfig-5.29.0.tar.xz
> > > > 
> > > > Acoording to kfunk the thing that broke kdevplatform wasn't really
> > > > kconfigs
> > > > fault but a side effect of kdevplatform code not being very good.
> > > 
> > > Heya,
> > > 
> > > the patch restoring the kdevplatform build with KF5 5.29:
> > >   https://cgit.kde.org/kdevplatform.git/commit/?
> > > 
> > > id=e84645d1694bdad7f179cd41babce723fe07aa63
> > > 
> > > The code in kdevplatform is a bit special, it's probably the only place
> > > in
> > > whole KDE which broke due to the recent changes in kconfig. I don't see
> > > an
> > > easy migration path, even if you introduce said change in a later
> > > kconfig
> > > release.
> > > 
> > > I don't mind if you leave kconfig as-is. But that's probably something
> > > for
> > > dfaure to decide.
> > 
> > Well, the change to kdevplatform isn't released yet, so kconfig 5.29-rc1
> > would break compilation of the current kdevplatform releases.
> > 
> > Also, the fact that I'm able to write a kconfig unittest that doesn't
> > compile tells me that something isn't right with these kconfig changes ---
> > unless it can be proven that what I'm doing in that new test is not
> > meaningful and is (now) forbidden, in which case it should at least be
> > documented. This is certainly worth another month of careful thinking
> > rather than rushing this into 5.29 now that it proved to be not 100%
> > perfect.
> 
> I investigated and can prove now that the test is not meaningful: it doesn't
> compile on master either. See https://paste.kde.org/po6oahg5p

Well, that only means my testcase is not good (sorry about that).
KDevelop *did* compile, so clearly my testcase failed to emulate exactly what 
happened in kdevelop.

> The problem is the "Inherits" - it doesn't really specify the conditions.
> All we have in the documentation is "Class the generated class inherits
> from. This class must inherit  KConfigSkeleton."

I see.

> But inheriting from KConfigSkeleton is not enough as the test case and the
> kdevelop example shows. It must have the same ctors as KConfigSkeleton
> available for the inheriting class. 

This makes sense. But adding a new constructor to KConfigSkeleton cannot 
mean that suddenly the requirement on base classes used by Inherits changes to 
include a new constructor - that would make it a source incompatible change.

So the requirement has to be more precise than "do whatever KConfigSkeleton 
does", which is not a fixed requirement (it can change over time).

Instead the requirement has to be "you need a constructor that takes a 
QString" or whatever the requirement actually is.

> That's the problem with the autotest and
> the problem with kdevelop's case. There the ctor existed, but was private
> instead of public.

Yes, because it was not needed before. That's what I mean by this change being 
source incompatible.

> Given that I think my change can go in, but we also should specify more
> clearly the Inherits requirements.

It's still a SIC. Which is only acceptable if what KDevelop was doing made no 
sense at all (e.g. broken behaviour at runtime). But from your description, it 
sounds to me like it did make sense, it's just that the change suddenly raises 
the requirements for base classes...

Maybe InheritsV2 is needed (or the same idea with a better name).
I.e. explicitly activating the new feature by saying "yes, my base class obeys 
version 2 of the requirement, go ahead".

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5