Please reconsider planned build breakage on new KF deprecations

[Friedrich W. H. Kossebau]

Am Freitag, 20. August 2021, 07:32:18 CEST schrieb Laurent Montel:
> On vendredi 20 août 2021 00:26:36 CEST Friedrich W. H. Kossebau wrote:
> > Am Mittwoch, 18. August 2021, 18:01:36 CEST schrieb Laurent Montel:
> > > On mercredi 18 août 2021 16:27:29 CEST Friedrich W. H. Kossebau wrote:
> > > > Hi,
> > > > 
> > > > the recent deprecations in KF master around KPluginLoader broke some
> > > > PIM
> > > > builds for me, due to KF_DISABLE_DEPRECATED_BEFORE_AND_AT being set to
> > > > a
> > > > number only matching the upcoming KF versions, thus immediately
> > > > changing
> > > > things when new deprecations come up.
> > > 
> > > For me the problem is that we mark as deprecated before porting code.
> > > Volker methods was better, porting all code to new code (when it’s
> > > possible) and after mark them as deprecated.
> > > For the moment we mark method as deprecated and nobody port them.
> > 
> > "Nobody ports them" is not what I see happening across the places.
> 
> Ah ? I don’t see it for kdegames/kdeutils/kdeaccessibility/kdeadmin .

It might happen when one does not look ;) Some there (for kdegames almost all 
even, I would know having done related commits ;) ) have got some brush over 
now and then recently.
In the case of those who indeed are forgotten and left behind, I wonder though 
if that not also shows that those are no longer of interest to people, both 
users and developers. E.g. kteatime might have had its purpose, now everyone 
has smartphones (even most feature phones have timers), so no-one might fancy 
a desktop app for that use case. Etc. Possibly better to drop those apps and 
focus resources on those which still have their user pool and also chance to 
grow there.

> > And more, why only the focus on Qt5 and KF5? Why not on Akonadi & PIM
> > libraries as well? How comes there is still
> > Akonadi::AgentBase::ObserverV4?
> > ;)
> 
> Because this api will not removed if we don’t port to new api.
> It’s not the case for kf5 deprecated methods in kf6.

Okay, makes sense.
Though as newcomer please everyone get the feedback that this leaves a bad 
impression, and would be the next thing I put on my list after restoring all 
unit tests. As adding new code to a landscape full of dprecated stuff is not 
attractive :) You surely know, but I feel it would be good to get these pain 
point poked once more.

> > > So using KF_DISABLE_DEPRECATED_BEFORE_AND_AT is good for it to avoiding
> > > to
> > > wait x months before porting them. (as for sure I will need to port it)
> > 
> > My assessment is different here. It is bad, because:
> > 
> > As said before it has chance to turn people with little time away as they
> > have to spent time on breakages when not wanting, surely does for me, I
> > could have spent time yesterday but did not because things are broken.
> > 
> > It makes PIM look like a place where things are broken and broken again,
> > worsening the reputation, making it less attractive for others (who wants
> > to work on things that seem to break easily). Just look at the build
> > breakages on build.kde.org triggered now. And worse, which sadly are not
> > only restricted on PIM applications, but also affects other applications
> > due to the Dependency builds breaking on the respective PIM parts.
> > 
> > Do a full rebuild of all PIM repos right now. I just did, and only build a
> > subset, but these here just all failed to me (and there would be more
> > which
> > I had hacked up to build before though):
> > * akonadi-contacts
> > * akonadi-search
> > * kidentitymanagement
> > * pimcommon
> > * calendarsupport
> > * incidenceeditor
> > * libksieve
> > * kmail
> > * kmail-account-wizard
> 
> It’s just because some part of https://invent.kde.org/frameworks/kservice/-/
> merge_requests/53/ was not commited yet.
> It’s the problem a patch was commited without making sure that all build
> correctly

Ah, I see now (and commented on the respective MR), sorry for falsely 
attributing to the KF_DISABLE_DEPRECATED_BEFORE_AND_AT in this very case then.
There is now already
    https://invent.kde.org/frameworks/kservice/-/merge_requests/57
to sort that out soonish.

> > So could we find a way where things only fail building for you locally on
> > new deprecations, while not breaking the build for everyone else, starting
> > with CI and then individual people?
> 
> Easy
> 
> option(BUILD_WITH_DEPRECATED_METHOD “Build with deprecated method” OFF)
> 
> If (NOT BUILD_WITH_DEPRECATED_METHOD)
>  Add_definitions(-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x055600)
> endif()

Not that easy sadly, as the visibility guard has effects on overloads and 
implicit conversions due to different API being visible to the compiler with 
different values on that control variable. So things might build with 
KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0xXYYZZ because a method overload or 
constructor is hidden, but will fail once overloads or constructors are 
visible due to a lower number in the control variable.

Also having to add this explicitly to all repos would be a bit annoying for 
all, more code and more duplication.

I am having an idea how we could perhaps add something new to 
KDECompilerSettings instead. Which would then also allow to have this build-
break-on-new-deprecated-API in more places by default (even if just one person 
so far is known to fancy it, but that person does lots of important work, so 
that deserves explicit support IMHO :) ).

At the same time might also allow to get away from having to use those hard to 
read/use hex numbers and instead being able to switch to human-readable 
version numbers.

So any 
    add_definitions(-DQT_DISABLE_DEPRECATED_BEFORE=0x050f02)
    add_definitions(-DKF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x055500)
could be replaced by
    ecm_disable_deprecated_api(
        Qt 5.15.2
        KF 5.85.0
    )
(with additional support for the KF module overwrites), and there would be 
KDECompilerSettings provided CMake options to overrule those numbers with 
6.0.0, to express "disable any until next major), consistent across any repo 
usig KDECompilerSettings and the respective macro.
In many cases the arguments could be even set via variables from the min Qt/KF 
requirement version variables usually around, to automate things some more.

I will see to sketch up some prototype code around that idea for discussion 
and considerations this upcoming WE.

Cheers
Friedrich