Morgane ”Sardem FF7” Glidic Reporter
	Description • 11 years ago

When building Firefox with GTK+3 widgets, I hit two build failures.
The first one is a wrong assignment (wrong types) in widget/gtk2/nsDeviceContextSpecG.cpp, the second one a missing #include in toolkit/components/downloads/nsDownloadManager.cpp

	Morgane ”Sardem FF7” Glidic Reporter
	Comment 1 • 11 years ago

Here are the two full build logs (11M and 3M):
http://dev.exherbo.org/~sardemff7/firefox-gtk3/failure-1.log
http://dev.exherbo.org/~sardemff7/firefox-gtk3/failure-2.log

The first log contains the full build log from hg archive to the first failure.
The second log contains the full build log from compile after manual patching for the first failure.

The used mozconfig is the default browser one, other options are passed directly to the configure script (visible in the first log).

	Morgane ”Sardem FF7” Glidic Reporter
	Comment 2 • 11 years ago

This patch fixes these two issues for me.
For nsDownloadManager.cpp, the first hunk (to fix the #include) is the only needed one, but obviously the rest is needed to have native platform support as intended.

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 3 • 11 years ago

Can you attach your .mozconfig file please? Or how do you configure and launch the build? The logs does not look familiar to me...

	Morgane ”Sardem FF7” Glidic Reporter
	Comment 4 • 11 years ago

(In reply to Martin Stránský from comment #3)
> Can you attach your .mozconfig file please? Or how do you configure and
> launch the build? The logs does not look familiar to me...

I have no such file. I use my package manager (Paludis, for Exherbo, a source-based distribution) to launch the build. The configure step (visible in the first log file) is run by calling the configure script directly with package manager’s switches as describe in the package file.
Here is the command:
./configure --prefix=/usr --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --disable-silent-rules --enable-fast-install --libdir=/usr/lib64 --enable-optimize=-O3 --disable-pedantic --disable-strip --disable-strip-libs --disable-install-strip --disable-warnings-as-errors --disable-debug --disable-debug-symbols --disable-tests --with-distribution-id=org.exherbo --disable-installer --disable-updater --with-system-bz2 --with-system-zlib --with-system-ply --enable-system-cairo --enable-system-hunspell --enable-system-ffi --enable-system-libvpx --enable-system-pixman --with-system-libevent=/usr --x-includes=/usr/include --x-libraries=/usr/lib64 --enable-chrome-format=omni --enable-default-toolkit=cairo-gtk3 --enable-gio --enable-ogg --enable-opus --enable-single-profile --enable-svg --enable-wave --enable-webm --disable-crashreporter --disable-profilesharing --disable-profilelocking --disable-gconf --disable-gnomeui --disable-gnomevfs --disable-websms-backend --enable-dbus --enable-pulseaudio --enable-raw --enable-startup-notification --enable-necko-wifi --enable-ipc --enable-application=browser --with-default-mozilla-five-home=/usr/lib64/firefox --enable-extensions=default,-gnomevfs,inspector --enable-crypto --enable-tracejit --enable-pango --enable-mathml --enable-safe-browsing --enable-storage --enable-places --enable-places_bookmarks --with-branding=browser/branding/nightly --disable-elf-hack

In the logs, lines starting with "[35;01m===[0m " are package manager’s messages.

	Karl Tomlinson (:karlt)
	Comment 5 • 11 years ago

Comment on attachment 764615 [details] [diff] [review]
GTK+3 build fixes

The changes of MOZ_WIDGET_GTK2 to MOZ_WIDGET_GTK look good, assuming the GtkRecentManager API hasn't changed too much.

>+#if ! GLIB_CHECK_VERSION(2,35,1)
>     g_type_init();
>+#endif

It should be safe to have the g_type_init() when building against any GLib,
and that may have the advantage of providing binary compatibility with libraries older than the headers.

> static void
>-print_callback(GtkPrintJob *aJob, gpointer aData, GError *aError) {
>+print_callback(GtkPrintJob *aJob, gpointer aData, const GError *aError) {
>   g_object_unref(aJob);
>   ((nsIFile*) aData)->Remove(false);
> }

Does this still work when building against GTK2?
https://developer.gnome.org/gtk2/2.24/GtkPrintJob.html#GtkPrintJobCompleteFunc
https://developer.gnome.org/gtk3/3.8/GtkPrintJob.html#GtkPrintJobCompleteFunc

	Morgane ”Sardem FF7” Glidic Reporter
	Comment 6 • 11 years ago

(In reply to Karl Tomlinson (:karlt) from comment #5)
> Comment on attachment 764615 [details] [diff] [review]
> GTK+3 build fixes
> 
> The changes of MOZ_WIDGET_GTK2 to MOZ_WIDGET_GTK look good, assuming the
> GtkRecentManager API hasn't changed too much.

Seems they just remove get_for_screen(), set_screen(), set_limit() and get_limit().


> >+#if ! GLIB_CHECK_VERSION(2,35,1)
> >     g_type_init();
> >+#endif
> 
> It should be safe to have the g_type_init() when building against any GLib,
> and that may have the advantage of providing binary compatibility with
> libraries older than the headers.

It should (new patch updated). In this case, using GLIB_VERSION_MIN_REQUIRED is probably a good idea.
https://developer.gnome.org/glib/2.32/glib-Version-Information.html#GLIB-VERSION-MIN-REQUIRED:CAPS


> > static void
> >-print_callback(GtkPrintJob *aJob, gpointer aData, GError *aError) {
> >+print_callback(GtkPrintJob *aJob, gpointer aData, const GError *aError) {
> >   g_object_unref(aJob);
> >   ((nsIFile*) aData)->Remove(false);
> > }
> 
> Does this still work when building against GTK2?
> https://developer.gnome.org/gtk2/2.24/GtkPrintJob.
> html#GtkPrintJobCompleteFunc
> https://developer.gnome.org/gtk3/3.8/GtkPrintJob.html#GtkPrintJobCompleteFunc

No, this new patch should work fine.

	Karl Tomlinson (:karlt)
	Comment 7 • 11 years ago

(In reply to Quentin "Sardem FF7" Glidic from comment #6)
> In this case, using GLIB_VERSION_MIN_REQUIRED
> is probably a good idea.
> https://developer.gnome.org/glib/2.32/glib-Version-Information.html#GLIB-
> VERSION-MIN-REQUIRED:CAPS

Sounds good, although the minimum version required is earlier than GLIB_VERSION_2_26 and there are no macros for earlier versions.  G_ENCODE_VERSION doesn't appear to be documented.  Once bug 795354 is fixed this can be set to 2.22.
Currently it should be even older.

A file in widget/gtk2/compat may be one way to set GLIB_VERSION_MIN_REQUIRED, if we can work out what to set it to.  Otherwise we'd need to either add it to every file, or to a number of the configuration variables MOZ_GTK2_CFLAGS MOZ_PANGO_CFLAGS MOZ_GTHREAD_CFLAGS MOZ_GIO_CFLAGS MOZ_GCONF_CFLAGS MOZ_DBUS_GLIB_CFLAGS

	Martin Stránský [:stransky] (ni? me) Assignee
	Comment 8 • 11 years ago

Karl, does the patch need some polishing or can we check it in?

	Karl Tomlinson (:karlt)
	Comment 9 • 11 years ago

This patch is ready.

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b1fcb81ac10

	Ryan VanderMeulen [:RyanVM]
	Comment 11 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/5b1fcb81ac10

	Morgane ”Sardem FF7” Glidic Reporter
	Comment 12 • 11 years ago

There was a new file introduce recently which more or less copies the code fixed in the previous patch. Here is a new patch to fix that. (Not yet tested, as the build did not finish yet.)

	Karl Tomlinson (:karlt)
	Comment 13 • 11 years ago

Comment on attachment 804877 [details] [diff] [review]
Other GTK+3 fixes

Looks good, thanks.

I don't know why we have all this code duplicated.

	Karl Tomlinson (:karlt)
	Comment 14 • 11 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 15 • 11 years ago

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