Closed Bug 1012447 Opened 11 years ago Closed 11 years ago

Allow building webapprt against gtk3

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Platform:
All
OpenBSD
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla32

People

(Reporter: gaston, Assigned: gaston)

Details

Attachments

(2 files, 2 obsolete files)

add plumbing to make it build with toolkit=cairo-gtk3 (
2.41 KB, patch
gaston
: review+
Details | Diff | Splinter Review
rename webapprt/gtk2 to webapprt/gtk
1.51 KB, patch
myk
: review+
Details | Diff | Splinter Review
turns out it's not that hard, patch incoming in a few. webapprt-stub with gtk2 tested working fine on OpenBSD, version with gtk3 to be tested in the coming days.
While testing gtk3 port, i noticed webapprt-stub wasnt built when using gtk3 (differences in installed files) but there's no reason it shouldnt, so lets give this a try. I've kept the same style as surrounding code for configure.in but maybe it should be better to use MOZ_ENABLE_GTK - as for building 'webapprt/gtk2' with gtk3, i'm pretty sure the 'gtk2' directory name was debated when imported and that someone raised the matter of gtk3, but oh well - turns out adding MOZ_CFLAGS/LIBS_GTK3 works just fine for building. Tested working fine with fx 30.0b5, patch forward-ported to m-i. Open to feedback/comments from webapprt hackers of course :) Testing TBD.
Assignee: nobody → landry
Attachment #8424498 - Flags: feedback?(hub)
Comment on attachment 8424498 [details] [diff] [review] add plumbing to make it build with toolkit=cairo-gtk3 Bouncing to Martin Stransky. He did most of the Gtk3 work. He probably has more insight about this. Thanks for helping with this.
Attachment #8424498 - Flags: feedback?(hub) → feedback?(stransky)
Comment on attachment 8424498 [details] [diff] [review] add plumbing to make it build with toolkit=cairo-gtk3 Review of attachment 8424498 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +6277,5 @@ > MOZ_ARG_DISABLE_BOOL(webapp-runtime, > [ --disable-webapp-runtime Disable Web App Runtime], > MOZ_WEBAPP_RUNTIME=, > MOZ_WEBAPP_RUNTIME=1) > +if test "$MOZ_WIDGET_TOOLKIT" != "windows" -a "$MOZ_WIDGET_TOOLKIT" != "cocoa" -a "$MOZ_WIDGET_TOOLKIT" != "gtk2" -a "$MOZ_WIDGET_TOOLKIT" != "gtk3"; then Should probably just check MOZ_WIDGET_GTK. ::: webapprt/gtk2/Makefile.in @@ +4,5 @@ > > LIBS = \ > $(XPCOM_STANDALONE_GLUE_LDOPTS) \ > $(MOZ_GTK2_LIBS) \ > + $(MOZ_GTK3_LIBS) \ gtk3 builds are going to have gtk2 and gtk3 flags set. Please don't do this.
Attachment #8424498 - Flags: feedback?(stransky) → review-
Component: Webapp Runtime → Build Config
Hardware: x86 → All
(In reply to Mike Hommey [:glandium] from comment #3) > Comment on attachment 8424498 [details] [diff] [review] > add plumbing to make it build with toolkit=cairo-gtk3 > > Review of attachment 8424498 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: configure.in > @@ +6277,5 @@ > > MOZ_ARG_DISABLE_BOOL(webapp-runtime, > > [ --disable-webapp-runtime Disable Web App Runtime], > > MOZ_WEBAPP_RUNTIME=, > > MOZ_WEBAPP_RUNTIME=1) > > +if test "$MOZ_WIDGET_TOOLKIT" != "windows" -a "$MOZ_WIDGET_TOOLKIT" != "cocoa" -a "$MOZ_WIDGET_TOOLKIT" != "gtk2" -a "$MOZ_WIDGET_TOOLKIT" != "gtk3"; then > > Should probably just check MOZ_WIDGET_GTK. Agreed. > ::: webapprt/gtk2/Makefile.in > @@ +4,5 @@ > > > > LIBS = \ > > $(XPCOM_STANDALONE_GLUE_LDOPTS) \ > > $(MOZ_GTK2_LIBS) \ > > + $(MOZ_GTK3_LIBS) \ > > gtk3 builds are going to have gtk2 and gtk3 flags set. Please don't do this. I've checked, and afaict (at least on beta) when building with gtk3 the gtk2 flags are empty: /usr/obj/ports/firefox-30.0beta5-gtk3/build-amd64-gtk3/config.status: (''' MOZ_GTK3_CFLAGS ''', r''' -I/usr/local/include/gtk-3.0/unix-print -I/usr/local/include/gtk-3.0 -I/usr/local/include/at-spi2-atk/2.0 -I/usr/local/include/gio-unix-2.0/ -I/usr/X11R6/include -I/usr/local/include/cairo -I/usr/local/include/pango-1.0 -I/usr/local/include/harfbuzz -I/usr/local/include/atk-1.0 -I/usr/X11R6/include/pixman-1 -I/usr/X11R6/include/freetype2 -I/usr/local/include/libpng16 -I/usr/local/include/gdk-pixbuf-2.0 -pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/glib-2.0/include -I/usr/local/include '''), /usr/obj/ports/firefox-30.0beta5-gtk3/build-amd64-gtk3/config.status: (''' MOZ_GTK3_LIBS ''', r''' -L/usr/local/lib -L/usr/X11R6/lib -Wl,-rpath-link,/usr/X11R6/lib -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpangoft2-1.0 -lpango-1.0 -lm -lfreetype -lz -lfontconfig -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lintl '''), /usr/obj/ports/firefox-30.0beta5-gtk3/build-amd64-gtk3/config.status: (''' MOZ_GTK2_CFLAGS ''', r''' '''), /usr/obj/ports/firefox-30.0beta5-gtk3/build-amd64-gtk3/config.status: (''' MOZ_GTK2_LIBS ''', r''' '''), Otherwise, what should be done ? add MOZ_GTKx_CFLAGS|LIBS depending on MOZ_WIDGET_GTK value ? $(MOZ_GTK$(MOZ_WIDGET_GTK)_CFLAGS) & _LIBS ?
That said, you should probably just use TK_CFLAGS/TK_LIBS.
Will post a new patch with those comments adressed, but i just wanted to confirm that webapprt-stub actually works when linked against gtk3 - i've just done some tests with a webapp installed in my profile on 30.0b5 with gtk3, and the webapp starts fine.
Attachment #8424498 - Attachment is obsolete: true
Attachment #8425014 - Flags: review?(mh+mozilla)
Comment on attachment 8425014 [details] [diff] [review] add plumbing to make it build with toolkit=cairo-gtk3 Review of attachment 8425014 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the configure logic fixed. ::: configure.in @@ +6277,5 @@ > MOZ_ARG_DISABLE_BOOL(webapp-runtime, > [ --disable-webapp-runtime Disable Web App Runtime], > MOZ_WEBAPP_RUNTIME=, > MOZ_WEBAPP_RUNTIME=1) > +if test "$MOZ_WIDGET_TOOLKIT" != "windows" -a "$MOZ_WIDGET_TOOLKIT" != "cocoa" -a -n "$MOZ_WIDGET_GTK" ; then ITYM -a -z "$MOZ_WIDGET_GTK" ::: webapprt/moz.build @@ +12,1 @@ > DIRS += ['gtk2'] Should probably rename the directory.
Attachment #8425014 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #9) > Comment on attachment 8425014 [details] [diff] [review] > add plumbing to make it build with toolkit=cairo-gtk3 > > Review of attachment 8425014 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me with the configure logic fixed. > > ::: configure.in > @@ +6277,5 @@ > > MOZ_ARG_DISABLE_BOOL(webapp-runtime, > > [ --disable-webapp-runtime Disable Web App Runtime], > > MOZ_WEBAPP_RUNTIME=, > > MOZ_WEBAPP_RUNTIME=1) > > +if test "$MOZ_WIDGET_TOOLKIT" != "windows" -a "$MOZ_WIDGET_TOOLKIT" != "cocoa" -a -n "$MOZ_WIDGET_GTK" ; then > > ITYM -a -z "$MOZ_WIDGET_GTK" Yeah, after a test build i realized this wasnt properly setting MOZ_WEBAPP_RUNTIME. > ::: webapprt/moz.build > @@ +12,1 @@ > > DIRS += ['gtk2'] > > Should probably rename the directory. hg rename gtk2 gtk + changing webapprt/moz.build should be enough, or there might be more dragons lurking in there ?
It should be enough.
myk, any opinion/approval/disapproval/r+ on renaming webapprt/gtk2 to webapprt/gtk ? should i attach a patch for this too ?
(In reply to Landry Breuil (:gaston) from comment #12) > myk, any opinion/approval/disapproval/r+ on renaming webapprt/gtk2 to > webapprt/gtk ? That sounds like a fine idea! I grant r+ on it. > should i attach a patch for this too ? Yes, please!
With comments addressed, carrying forward r+..
Attachment #8425014 - Attachment is obsolete: true
Attachment #8425727 - Flags: review+
Oh the irony.. i was the one who wanted to get this dir renamed from webapprt/linux in bug #761516... :)
Attachment #8425729 - Flags: review?(myk)
Comment on attachment 8425729 [details] [diff] [review] rename webapprt/gtk2 to webapprt/gtk (In reply to Landry Breuil (:gaston) from comment #15) > Created attachment 8425729 [details] [diff] [review] > rename webapprt/gtk2 to webapprt/gtk > > Oh the irony.. i was the one who wanted to get this dir renamed from > webapprt/linux in bug #761516... :) May your code last long enough for you to regret it! ;-)
Attachment #8425729 - Flags: review?(myk) → review+
and just to be on the safe side, before landing it... https://tbpl.mozilla.org/?tree=Try&rev=f6e154188689
aaaaand burning right away. The underlying problem is a Python syntax error on line 11: +elif CONFIG['MOZ_ENABLE_GTK']: Heh, that'll teach me.. forgot to remove that leading + and try saved me :) https://tbpl.mozilla.org/?tree=Try&rev=5746e5951f5c
https://hg.mozilla.org/mozilla-central/rev/07318b6e3f1b https://hg.mozilla.org/mozilla-central/rev/8e1ec45ff14d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 32 → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: