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@xxxxxxxxxx> > Date: Thu Mar 5 15:28:22 2015 +0100 > > Mark unused public API methods/code as deprecated > > Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > > 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel