[Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

[Christophe Fergeau]

On Thu, Aug 11, 2016 at 04:28:58PM +0200, Francois Gouget wrote:
> 
> The following patch broke compilation of xf86-video-qxl:
> 
> commit b41220b1441b8adea6db9a98e9da1b43a8f426bb
> Author: Christophe Fergeau <cfergeau at redhat.com>
> Date:   Thu Mar 5 15:28:22 2015 +0100
> 
>     Mark unused public API methods/code as deprecated
>     
>     Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> 
> The reason is it introduces a #include <glib.h> in spice-server.h which 
> is a *public* header! This means any application that needs to include 
> spice-server.h, like xf86-video-qxl, must now check for GLib even if 
> they don't use it, like xf86-video-qxl.
> 
> This does not make sense and thus the glib.h include must be removed. 

As danpb pointed out, main reason for that is that we forgot to add
glib-2.0 as a dependency in the .pc file, if this had been done, this
change would probably have gone mostly unnoticed.

> However it was added to get G_GNUC_DEPRECATED. Fortunately there is 
> SPICE_GNUC_DEPRECATED, or maybe that's SPICE_DEPRECATED? It gets a bit 
> confusing from here:
> 
> First the macros to tag deprecated functions:
> * spice uses G_GNUC_DEPRECATED directly in spice-server.h as seen above.
> 



> * spice-gtk defines SPICE_DEPRECATED in spice-util.h as a wrapper around 
>   G_GNUC_DEPRECATED, or an empty macro if SPICE_NO_DEPRECATED is 
>   defined.
> 
> * spice-gtk also has a SPICE_GNUC_DEPRECATED_FOR() macro which is not 
>   defined anywhere else and a corresponding wrapper, 
>   SPICE_DEPRECATED_FOR(), which is a no-op if SPICE_NO_DEPRECATED is 
>   defined.

Imo this SPICE_NO_DEPRECATED is only meant to be used internally by
spice-gtk even if this is not really documented. I would not consider it
breakage if this was removed, or changed in incompatible ways.
And it turns out it's actually useless as -Wno-deprecated-declarations
is used during compilation (which also disables the
warnings from GLIB_VERSION_MIN_REQUIRED/GLIB_VERSION_MAX_REQUIRED :(
I'd tend to change that so that -Wno-deprecated-declarations is only
used for spicy, and make selective use of
G_GNUC_BEGIN_IGNORE_DEPRECATIONS / G_GNUC_END_IGNORE_DEPRECATIONS when
needed. Seems to be working with quick and dirty local changes.

SPICE_NO_DEPRECATED is then not really needed.

> 
> * spice-protocol defines SPICE_GNUC_DEPRECATED in spice/macro.h but 
>   uses the gcc __attribute__ macro directly instead of using 
>   G_GNUC_DEPRECATED.

Yup, mostly because spice-protocol does not depend on glib (and does not
use anything related to glib for that matter).


> Second the macros to 'enable/disable' deprecated APIs:
> * In spice-gtk defining SPICE_NO_DEPRECATED disables the warnings.

See above, my thinking is that this is mainly meant for internal use.

> 
> * In spice-protocol you must define SPICE_DEPRECATED to get access to 
>   the deprecated vd_agent.h VD_AGENT_CLIPBOARD_MAX_SIZE_DEFAULT and 
>   VD_AGENT_CLIPBOARD_MAX_SIZE_ENV macros.
> 
> Of course spice-gtk's source files use both spice-util.h and vd_agent.h, 
> meaning they necessarily get the spice-protocol deprecated macros due to 
> the SPICE_DEPRECATED naming conflict.

Ah, not a big deal, but not nice :(

> So here is the proposed solution:
> 
> * spice-protocol's SPICE_DEPRECATED is in a public header so keep it as 
>   is. Extend it to mean the user wants to use deprecated APIs. This 
>   includes:
>   - Defining deprecated APIs and macros (as before).
>   - Disabling warnings about the use of deprecated APIs so it takes 
>     over spice-gtk's SPICE_NO_DEPRECATED role.
> 
> * Disable spice-protocol's SPICE_GNUC_DEPRECATED warnings when 
>   SPICE_DEPRECATED is defined. 

Hm, I can see some merit with having a way to disable deprecation
warnings per-module rather than globally through
-Wno-deprecated-declarations. How big of a mess would a more
explicit/similar to glib name cause? SPICE_DISABLE_DEPRECATION_WARNINGS

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160901/35549525/attachment-0001.sig>