Deprecation Defines

[David Faure]

(moving this to kde-buildsystem)

On Tuesday 08 July 2014 10:47:21 Matthew Dawson wrote:
> On July 8, 2014 11:20:37 AM David Faure wrote:
> > > I have a patch sitting in review for kconfig.
> > 
> > What's the RR number? I can't see it in my k-f-d folder.
> 
> 118489.  It already has comments on it, with the decision that policy needs
> to be figured out before it is pushed.

Indeed. Well, let's figure it out then :-)

> I meant that if some libraries enable DEFINE_NO_DEPRECATED, and some do not,
> the compiler will complain about redefining DEFINE_NO_DEPRECATED.  I can't
> tell if that is actually legal either.

Oh, right, when including multiple *_export.h files these do conflict.
I'm surprised that we don't get tons of warnings right now about that already.
I checked, and the precompiler sees both, but says nothing, because it's set again to the
same value. I only get a warning when the value differs from the previous value.

kcmutils_export.h:37:0: warning: "DEFINE_NO_DEPRECATED" redefined [enabled by default]
 #define DEFINE_NO_DEPRECATED 1

I believe the idea was that this was a local value (local to the export header). So cmake should be patched like this:

diff --git a/Modules/exportheader.cmake.in b/Modules/exportheader.cmake.in
index 118de16..04978a9 100644
--- a/Modules/exportheader.cmake.in
+++ b/Modules/exportheader.cmake.in
@@ -37,5 +37,6 @@
 #if DEFINE_NO_DEPRECATED
 # define @NO_DEPRECATED_MACRO_NAME@
 #endif
+#undef DEFINE_NO_DEPRECATED
 
 #endif

Stephen, do you agree?

> > OK, I see two ways in which this could work:
> > 
> > 1) someone wants to compile KF5 itself without deprecated stuff, to ensure
> > their own apps don't use any deprecated methods. That's how cmake's
> > GenerateExportHeader.cmake was written. And in that case, even virtual
> > methods could be excluded. But for this we need to provide a way to pass
> > DEFINE_NO_DEPRECATED to generate_export_header (like the docu suggests,
> > with an option). ECM could define that option for all frameworks.
> > 
> > 2) someone installs a "normal" KF5 (maybe even from a distro) and wants to
> > ensure their app doesn't use any deprecated method. They will have to add
> > things like add_definitions(-DKCONFIGCORE_NO_DEPRECATED). This sounds
> > cumbersome, if one wants to set this for each and every framework they are
> > using.
> > 
> > Historically in KDE we were doing something like solution 2 with
> > KDE_NO_DEPRECATED. At some point we had something more like solution 1
> > with
> > the "bic feature reduction and no deprecated stuff" defines in kdelibs. I
> > tend to think option 2 is easier to use for app developers who don't
> > compile their own KF5; but of course they can also just watch for
> > deprecation warnings. On the other hand, solution 1 is the one cmake
> > provides, and is more geared towards "making KF5 smaller for specific use
> > cases (e.g. mobile devices)", so it's potentially more useful.
> > 
> > As it is right now, this doesn't seem usable to me. We need to choose one
> > of the two solutions and then make it work :)
> 
> I agree.  I figured the first step was to come up with a draft policy and
> post to the list for comments, and as I said I'm trying to get a new
> contributer to help (they have lots of experience writing documents, and
> have a strong technical background).  Time, as always, limited our ability
> to get it written.

I think we need to decide before we can write it out :-)

> I don't see option 1) being usable without fixing cmake to avoid the
> redefining warnings (though I imagine that is a small task).  Also, I don't
> think any general purpose distribution would enable it, as that option would
> break SC and BC anytime new deprecated features are added.  For single
> purpose devices, it's not a problem.  But single purpose devices are
> rapidly shrinking.

Not necessarily single-purpose, just different BC than desktop linux.
E.g. Meego could have decided "we ship KF-5.0 without deprecated stuff,
to make it smaller".

> Option 2 definitely has its uses, but as you say deprecation warning should
> take care of that.  Come to think of it, one problem with option 2 is that
> developers may enable that in their production build scripts, and thus with
> a new release of KF5 deprecating new behaviour, will break old
> applications.

This is why I rather see *application* developers setting it (only within their own app,
they typically don't care about others). New KF5 releases would then force them to
port away from the newly-deprecated stuff.

But yeah, the Qt solution with version-specific deprecation (QT_DEPRECATED_SINCE) is much better,
we should adopt something like that.
This way, you can ensure your code keeps compiling "without the stuff that was deprecated in 5.1",
without getting a compilation error if someone suddenly installs KF 5.2 which deprecated new stuff.
This means not using the stuff currently in cmake then, but something more evolved based
on the project's version number...

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