[Spice-devel] [client 00/10] Deprecation warning cleanup

[Victor Toso]

Hi,

On Tue, Oct 18, 2016 at 07:46:34PM +0200, Francois Gouget wrote:
>
> This is another take on trying to clean up the deprecation warning
> situation in Spice-Gtk.

Many thanks :)

>
> Let me know if it goes in the right direction.
>
>
> With this patchset deprecation warnings are enabled again and there are
> no remaining warnings, except those in src/controller since they happen
> in generated code and I don't know how to modify it so it does not use
> the deprecated GSimpleAsyncResult class (and I don't really want to do
> the conversion to GTask either). Of course those could also be avoided
> by keeping the -Wno-deprecated-declarations in src/controller but then
> nobody will fix that code.
>
> Francois Gouget (10):
>   build-sys: Enable deprecation warnings instead of ignoring them entirely
>
> First the Spice-Gtk situation is different from the Spice server one.
>
> In Spice server the only deprecated APIs we use are Spice's own. So new
> uses will only result from changes in Spice's code. Thus it's ok to have
> them produce an error to prevent their use from spreading.
>
> In Spice-Gtk the deprecated APIs are almost all GTK APIs. Those can be
> deprecated at any time independently of Spice-Gtk's code. Thus we don't
> want deprecation warnings to break the build.
>
> But so far they have be completely disabled, thus ensuring no one
> will ever fix them. So the first patch turns them back to regular
> warnings with -Wno-error.

I agree

>
>
>   gtk: Avoid gtk_vbox_new(); it is deprecated
>   gtk: Avoid gtk_hbutton_box_new(); it is deprecated
>   gtk: GtkTable is deprecated so use GtkGrid instead
>
> These patches just fix usage of some deprecated APIs. Fixes that would
> likely have happened not long after GTK+ 2.0 support was dropped if
> deprecation warnings had not been disabled. Those should clearly be
> applied even if we don't enable deprecation warnings.
>
>
>   gtk: Ignore GLib's too-new warnings where we explicitly check its version
>
> GLib also issues warnings for APIs that are more recent than
> GLIB_VERSION_MIN_REQUIRED. But these happen in suitable #if sections and
> thus should be ignored.
>
>   build-sys: Remove SPICE_NO_DEPRECATED
>   build-sys: Use spice-protocol's deprecation macros
>
> This one may be somewhat questionable since it spice-protocol's macros
> don't use GLib's helpers and thus don't support Clang and Visual C++.
> This means we may locally disable these warnings with
> G_GNUC_BEGIN_IGNORE_DEPRECATIONS when they would not have in fact been
> issued at all.
> * It's not like this issue would cause compilation errors or
>   warnings (the opposite) so maybe it should just be ignored.
> * Or maybe it means Spice-Gtk should keep using its own deprecation
>   macros, but it would be a bit of a shame that spice-protocol's macros
>   cannot even be reused by all Spice's codebase.
> * Or maybe  spice-protocol should use GLib's macros if available,
>   even if it is a bit distasteful.

I recall that Christophe commented about the spice macros but I failed
to find it.

>
>   session: Disable the spice_audio_new() deprecation warning
>
> I donsider this to be a "false positive", although maybe the code should
> be rewritten to avoid it.

Yes, I think we can remove for public header nowadays (i replied earlier
to that one)

>   RFC: gtk: Temporarily ignore the keyboard/mouse grabbing deprecation warnings
>   RFC: spicy: Temporarily ignore deprecation warnings
>
> These two really just silence warnings that should be fixed. So if
> someone fixes these warnings they will not be needed. Otherwise I think
> it's still better than globally disabling the deprecation warnings
> because these patches are local and add nice FIXME warnings in the
> relevant places.

I think we should try to fix this two and not ignore the warnings. We
are early in 0.33.

>
>
>  m4/spice-compile-warnings.m4 |  2 +-
>  src/Makefile.am              |  1 -
>  src/controller/Makefile.am   |  1 -
>  src/spice-channel.h          |  5 +++--
>  src/spice-gtk-session.c      |  5 +++++
>  src/spice-session.c          |  2 ++
>  src/spice-util.h             | 14 +++-----------
>  src/spice-widget-egl.c       |  3 +++
>  src/spice-widget.c           | 30 ++++++++++++++++++++++++++++++
>  src/spicy-connect.c          | 22 +++++++++++-----------
>  src/spicy.c                  |  3 +++
>  src/usb-device-manager.h     |  2 +-
>  12 files changed, 62 insertions(+), 28 deletions(-)

Thanks again!
  toso
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161019/7b6525ec/attachment-0001.sig>