1114898 - Compile warning in toolkit/components/jsdownloads/src/DownloadPlatform.cpp for g_type_init

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Zhenbo Li]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141201111728

Steps to reproduce:

./mach build toolkit/components/jsdownloads/src


Actual results:

 0:00.20 /usr/bin/gmake -C /home/lizhenbo/src/mozilla-central/obj-x86_64-unknown-linux-gnu -j8 -s backend.RecursiveMakeBackend
 0:00.32 /usr/bin/gmake -C toolkit/components/jsdownloads/src -j8 -s
 0:00.35 DownloadPlatform.o
 0:04.97 /home/lizhenbo/src/mozilla-central/toolkit/components/jsdownloads/src/DownloadPlatform.cpp: In static member function 'static DownloadPlatform* DownloadPlatform::GetDownloadPlatform()':
 0:04.97 Warning: -Wdeprecated-declarations in /home/lizhenbo/src/mozilla-central/toolkit/components/jsdownloads/src/DownloadPlatform.cpp: 'void g_type_init()' is deprecated (declared at /usr/include/glib-2.0/gobject/gtype.h:679)
 0:04.97 /home/lizhenbo/src/mozilla-central/toolkit/components/jsdownloads/src/DownloadPlatform.cpp:52:3: warning: 'void g_type_init()' is deprecated (declared at /usr/include/glib-2.0/gobject/gtype.h:679) [-Wdeprecated-declarations]
 0:04.97    g_type_init();
 0:04.97    ^
 0:04.97 Warning: -Wdeprecated-declarations in /home/lizhenbo/src/mozilla-central/toolkit/components/jsdownloads/src/DownloadPlatform.cpp: 'void g_type_init()' is deprecated (declared at /usr/include/glib-2.0/gobject/gtype.h:679)
 0:04.97 /home/lizhenbo/src/mozilla-central/toolkit/components/jsdownloads/src/DownloadPlatform.cpp:52:15: warning: 'void g_type_init()' is deprecated (declared at /usr/include/glib-2.0/gobject/gtype.h:679) [-Wdeprecated-declarations]
 0:04.97    g_type_init();
 0:04.97                ^



Expected results:

No compile warnings

Comment 1

[Marco Bonardo [:mak]]

looks like there's more the need to call g_type_init() at all.

Comment 2

[Zhenbo Li]

Attachment: check_version.patch

"g_type_init has been deprecated since version 2.36"

So I've written a patch with GLIB_CHECK_VERSION().
But I don't know I should check (2,36,0) or (2,35,0)

Reference: https://developer.gnome.org/gobject/unstable/gobject-Type-Information.html#g-type-init

See also: https://www.libreoffice.org/bugzilla/show_bug.cgi?id=62886

Comment 3

[Karl Tomlinson (:karlt)]

The GTK+2 port of Gecko is currently targeting GLib versions 2.20 and higher
(based on GIO_VERSION in configure.in).
Usually we aim to have the same code compiled by developers (usually with
newer GLib) as on build machines (with 2.22), so usually we try to avoid
compile-time version checks (unless checking different ports for
non-backward-compatible libraries like GTK+).

I suspect the sustainable way to have these deprecated warnings available is
to set GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED.  The
GLIB_VERSION_* macros start at 2_26, but I think we can still set these
macros to "(2 << 16 | 20 < 8)".

Comment 4

[Zhenbo Li]

(In reply to Karl Tomlinson (:karlt) from comment #3)
> The GTK+2 port of Gecko is currently targeting GLib versions 2.20 and higher
> (based on GIO_VERSION in configure.in).
> Usually we aim to have the same code compiled by developers (usually with
> newer GLib) as on build machines (with 2.22), so usually we try to avoid
> compile-time version checks (unless checking different ports for
> non-backward-compatible libraries like GTK+).
> 
> I suspect the sustainable way to have these deprecated warnings available is
> to set GLIB_VERSION_MIN_REQUIRED and GLIB_VERSION_MAX_ALLOWED.  The
> GLIB_VERSION_* macros start at 2_26, but I think we can still set these
> macros to "(2 << 16 | 20 < 8)".

I edited configure.in, but such a change will make mozilla fail to be built on some computers. Is it acceptable?

diff -r 7b33ee7fd162 configure.in
--- a/configure.in      Sat Dec 20 12:18:28 2014 -0800
+++ b/configure.in      Wed Dec 24 23:57:34 2014 +0800
@@ -61,6 +61,7 @@
 PERL_VERSION=5.006
 CAIRO_VERSION=1.10
 PANGO_VERSION=1.22.0
+GLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_36
 GTK2_VERSION=2.18.0
 GTK3_VERSION=3.0.0
 WINDRES_VERSION=2.14.90

Comment 5

[Zhenbo Li]

$ hg grep g_type_init
toolkit/components/downloads/nsDownloadManager.cpp:218080:    g_type_init();
toolkit/system/gnome/nsGnomeModule.cpp:215781:  g_type_init();
toolkit/components/jsdownloads/src/DownloadPlatform.cpp:211407:  g_type_init();
gfx/thebes/gfxQtPlatform.cpp:167722:    g_type_init();
toolkit/xre/nsNativeAppSupportQt.cpp:106774:  g_type_init();
toolkit/system/gnome/nsGConfService.cpp:69384:  g_type_init();
gfx/thebes/src/gfxQtPlatform.cpp:46955:    g_type_init();
gfx/thebes/src/gfxPangoFonts.cpp:19286:    g_type_init();

All the g_type_init() in codebase.
If we edit configure.in, then all of them should be removed.

Comment 6

[Mike Hommey [:glandium]]

(In reply to litimetal from comment #5)
> $ hg grep g_type_init

hg grep is not grep/git grep.

$ git grep g_type_init
toolkit/components/downloads/nsDownloadManager.cpp:    g_type_init();
toolkit/components/jsdownloads/src/DownloadPlatform.cpp:  g_type_init();
toolkit/system/gnome/nsGnomeModule.cpp:  g_type_init();

That said, I don't think deprecation warnings are interested to spend time fixing.

Comment 7

[Mike Hommey [:glandium]]

Especially when the deprecated function does something in the supported versions of glib. BTW, we went through hoops in bug 793634 so that building with a recent gtk/glib still works with gtk 2.18/lib 2.22. I'm not sure how well that actually works with recent gtk/glib, but "fixing" the deprecation warnings would definitely break that feature.

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to litimetal from comment #4)
> +GLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_36

I meant setting this to indicate the current target version 2.20 (or maybe 2.22, not sure) to indicate that g_type_init should not cause a deprecated warning.  G_ENCODE_VERSION(2,20) can be used instead of hard-coded bitshifts I assume.

The value in setting GLIB_VERSION_MAX_ALLOWED is that developers will get warnings if trying to use functions newer than 2.26 at least.  (functions added between 2.20 and 2.26 are not supported, and so would not warn.)

I assume AC_DEFINE turns the configure variable into a preprocessor macro.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: set GLIB_VERSION_MIN_REQUIRED/MAX_ALLOWED to support symbol version warnings

Also bumping GLIB_VERSION to 2.22 and GTK3_VERSION 3.4 in line with Mozilla
build environment.

GLIB_VERSION_MAX_ALLOWED for GTK3 is based on the min requirement for GTK 3.4.
I don't know exactly what build machines use.
GLIB_VERSION_MIN_REQUIRED is still set to the minimum of 2_26 even for GTK3 to
suppress warnings from common code using glib symbols that may be deprecated.

Comment 11

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba61225ba91c

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ba61225ba91c