Closed Bug 884708 Opened 7 years ago Closed 6 years ago

Port GTK2 to GTK3 - build failures

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: sardemff7+mozilla, Assigned: stransky)

References

Details

Attachments

(2 files, 1 obsolete file)

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
Blocks: gtk3
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).
Attached patch GTK+3 build fixes (obsolete) — Splinter Review
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.
Assignee: nobody → stransky
Can you attach your .mozconfig file please? Or how do you configure and launch the build? The logs does not look familiar to me...
Flags: needinfo?(sardemff7+mozilla)
(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.
Flags: needinfo?(sardemff7+mozilla)
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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.
Attachment #764615 - Attachment is obsolete: true
(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
Karl, does the patch need some polishing or can we check it in?
Flags: needinfo?(karlt)
This patch is ready.
Flags: needinfo?(karlt)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5b1fcb81ac10
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
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.)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #804877 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/df11d218b990
Status: REOPENED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
Blocks: 1034064
No longer blocks: 1034064
You need to log in before you can comment on or make changes to this bug.