Closed
Bug 259033
Opened 20 years ago
Closed 20 years ago
Qt build patch
Categories
(Core Graveyard :: Ports: Qt, defect)
Core Graveyard
Ports: Qt
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: zack, Assigned: timeless)
References
Details
Attachments
(1 file, 3 obsolete files)
11.46 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux ppc; en-US; rv:1.7) Gecko/20040823 Firefox/0.9.3 Build Identifier: Mozilla/5.0 (X11; U; Linux ppc; en-US; rv:1.7) Gecko/20040823 Firefox/0.9.3 In this bug I'll attach the build system patch that incorporates the changes needed to build the Qt port. Reproducible: Always Steps to Reproduce: 1. 2. 3.
Reporter | ||
Comment 1•20 years ago
|
||
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•20 years ago
|
||
Comment on attachment 158684 [details] [diff] [review] Qt Mozilla build patch # Add explicit X11 dependency when building against X11 toolkits -ifneq (,$(filter gtk gtk2 xlib,$(MOZ_WIDGET_TOOLKIT))) +ifneq (,$(filter gtk gtk2 qt xlib,$(MOZ_WIDGET_TOOLKIT))) LIBS += $(XLDFLAGS) $(XLIBS) we suck... there's this nice MOZ_X11 subst in configure.in, although autoconf.mk.in doesn't use it... mozilla patches don't need to contain configure diffs, it is autogenerated upon checkin note to self: widget patch is in Bug 259036. Are the gfx changes available anywhere?
Attachment #158684 -
Flags: review?(bsmedberg)
Comment 3•20 years ago
|
||
Comment on attachment 158684 [details] [diff] [review] Qt Mozilla build patch >Index: configure.in >+ dnl The following warning breaks Qt >+ if test ! "$QTDIR"; then >+ _WARNINGS_CXXFLAGS="${_WARNINGS_CXXFLAGS}" >+ fi What's this about? Did you forget something? >+qt) >+ MOZ_ENABLE_QT=1 >+ MOZ_ENABLE_XREMOTE=1 >+ TK_CFLAGS='$(MOZ_QT_CFLAGS)' >+ TK_LIBS='$(MOZ_QT_LDFLAGS)' >+ AC_DEFINE(MOZ_WIDGET_QT) Whitespace nit: follow existing conventions (use spaces here, not tabs) >+dnl AM_PATH_QT($QT_VERSION,, >+dnl AC_MSG_ERROR(Test for QT failed.)) What's up? We don't generally check in code which is commented-out. >-if test "$MOZ_ENABLE_GTK" \ >+if test "$MOZ_ENABLE_GTK" || test "$MOZ_ENABLE_QT" \ > || test "$MOZ_ENABLE_XLIB" \ > || test "$MOZ_ENABLE_GTK2" grr, please be consistent >Index: embedding/browser/Makefile.in >-DIRS=webBrowser build chrome >+DIRS=webBrowser build chrome qt Do we build this dir even if qt is not the toolkit in use? >Index: modules/plugin/base/src/nsPluginHostImpl.cpp >+#ifdef MOZ_WIDGET_QT >+#undef CursorShape /*X.h defines it as 0, >+ qnamespace.h makes an enum type by that name >+ */ >+#endif I would prefer "#ifdef CursorShape instead of #ifdef MOZ_WIDGET_QT >Index: widget/src/xpwidgets/Makefile.in > uconv \ > unicharutil \ >+ docshell view intl \ > $(NULL) follow existing convention, one dir per line, with consistent tab-tab-space-space indenting Fix those nits and attach a new patch. I also want jst to review the changes to the plugin code.
Attachment #158684 -
Flags: superreview?(jst)
Attachment #158684 -
Flags: review?(bsmedberg)
Attachment #158684 -
Flags: review+
Comment 4•20 years ago
|
||
Comment on attachment 158684 [details] [diff] [review] Qt Mozilla build patch sr=jst with bsmedbergs nits fixed.
Attachment #158684 -
Flags: superreview?(jst) → superreview+
Reporter | ||
Comment 5•20 years ago
|
||
Actually this baby is essentially what you used to have in the tree for the old Qt port. I assumed that one was already reviewed and ok. All yours truly did was boost the Qt requirement and add the embedding dir (btw, yeah, that one needs to be in ifdefs as well). I'll update it after work and will send an update tomorrow morning. And to answer the question the full set of patches for the Qt port is in: 259035 (GFX), 259036 (widget), 259037 (webshell) and 259038 (embedding). I guess it'd be nice to have a top level bug depending on those to follow the general progress of the merge but without internet connection at home I can't really do it at the moment.
Reporter | ||
Comment 6•20 years ago
|
||
(In reply to comment #3) > (From update of attachment 158684 [details] [diff] [review]) > >Index: configure.in > > >+ dnl The following warning breaks Qt > >+ if test ! "$QTDIR"; then > >+ _WARNINGS_CXXFLAGS="${_WARNINGS_CXXFLAGS}" > >+ fi > > What's this about? Did you forget something? It was in the original patch. I removed it. > >+qt) > >+ MOZ_ENABLE_QT=1 > >+ MOZ_ENABLE_XREMOTE=1 > >+ TK_CFLAGS='$(MOZ_QT_CFLAGS)' > >+ TK_LIBS='$(MOZ_QT_LDFLAGS)' > >+ AC_DEFINE(MOZ_WIDGET_QT) > > Whitespace nit: follow existing conventions (use spaces here, not tabs) That's also something that has been orginally in your tree. BTW, you don't really have a convention there, xlib, photon, cairo-xlib use tabs - others spaces. I switched Qt to spaces, but it would make sense to make it all consistent if the spaces are a convention. > >+dnl AM_PATH_QT($QT_VERSION,, > >+dnl AC_MSG_ERROR(Test for QT failed.)) > > What's up? We don't generally check in code which is commented-out. Also something that was orginally in your tree. Fixed that. > >-if test "$MOZ_ENABLE_GTK" \ > >+if test "$MOZ_ENABLE_GTK" || test "$MOZ_ENABLE_QT" \ > > || test "$MOZ_ENABLE_XLIB" \ > > || test "$MOZ_ENABLE_GTK2" > > grr, please be consistent Also not me. Fixed :) > >Index: embedding/browser/Makefile.in > > >-DIRS=webBrowser build chrome > >+DIRS=webBrowser build chrome qt > > Do we build this dir even if qt is not the toolkit in use? Yap, that was wrong. I just added it for us and forgot to remove it. > >Index: modules/plugin/base/src/nsPluginHostImpl.cpp > > >+#ifdef MOZ_WIDGET_QT > >+#undef CursorShape /*X.h defines it as 0, > >+ qnamespace.h makes an enum type by that name > >+ */ > >+#endif > > I would prefer "#ifdef CursorShape instead of #ifdef MOZ_WIDGET_QT Makes sense. Moved it. > >Index: widget/src/xpwidgets/Makefile.in > > > uconv \ > > unicharutil \ > >+ docshell view intl \ > > $(NULL) > > follow existing convention, one dir per line, with consistent > tab-tab-space-space indenting That wasn't really a problem. This part was just plain wrong. I did it because we had some problems regenerating the Makefile because of some funky things I did and since I didn't feel like messing with it at the time I just made the whole thing link against it. Fixed it. Thanks for taking a look at it.
Reporter | ||
Comment 7•20 years ago
|
||
Reporter | ||
Updated•20 years ago
|
Attachment #158684 -
Attachment is obsolete: true
Comment 8•20 years ago
|
||
hm... aclocal: configure.in: 3343: macro `AM_PATH_QT' not found in library
Reporter | ||
Comment 9•20 years ago
|
||
(In reply to comment #8) > aclocal: configure.in: 3343: macro `AM_PATH_QT' not found in library Oh, yeah, the patch should of course not have the : + AM_PATH_QT($QT_VERSION,, + AC_MSG_ERROR(Test for Qt failed.)) lines. Sorry about that.
Comment 10•20 years ago
|
||
in allmakefiles.sh: widget/src/qt that should be widget/src/qt/Makefile otherwise you'll have a file called qt in the objdir, which causes the build to fail...
Reporter | ||
Comment 11•20 years ago
|
||
Fixes the allmakefiles issue plus updates to cleanly apply with HEAD
Reporter | ||
Updated•20 years ago
|
Attachment #158877 -
Attachment is obsolete: true
Comment 12•20 years ago
|
||
Comment on attachment 159279 [details] [diff] [review] Qt build patch + MOZ_ENABLE_XREMOTE=1 so does qt actually support xremote? because if it doesn't, this will do bad things, the user-visible one being that bookmarks aren't loaded in browser windows (well, due to sucky error handling, but still)
Reporter | ||
Comment 13•20 years ago
|
||
No, it doesn't I was entertaining the thought of supporting it and didn't think that adding something in will cause such problems.
Attachment #159279 -
Attachment is obsolete: true
Reporter | ||
Comment 14•20 years ago
|
||
I just updated gfx, widget and embedding patches. Could someone please either let me know whether there's anything wrong with them or commit them?
Comment 15•20 years ago
|
||
so, it seems that currently embedding/browser/qt is never built. shouldn't it be built, similar to embedding/browser/gtk, by mozilla/Makefile.in? (maybe it shouldn't, I don't know :) )
Reporter | ||
Comment 16•20 years ago
|
||
I disabled it for a bit. Now it's actually working fine so we can enable it by default. I'm not 100% committed to this api yet so I don't want to have people jumping on it yet. I can enable it though if that's easier.
Reporter | ||
Comment 17•20 years ago
|
||
Qt Mozilla is now in CVS.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 18•20 years ago
|
||
I just tried to build a Qt Seamonkey and it seems the file qlist.h is missing. Or am I doing something wrong? I used the following .mozconfig: ac_add_options --enable-freetype2 ac_add_options --enable-crypto ac_add_options --enable-extensions=wallet,irc,help,inspector,typeaheadfind,xml-rpc,xmlextras,venkman,cookie,content-packs,transformiix,universalchardet,webservices,spellcheck,gnomevfs,negotiateauth,sroaming ac_add_options --enable-svg ac_add_options --enable-svg-renderer-libart ac_add_options --disable-installer ac_add_options --enable-debug ac_add_options --disable-optimize mk_add_options MOZ_INTERNAL_LIBART_LGPL=1 export MOZ_INTERNAL_LIBART_LGPL=1 ac_add_options --enable-default-toolkit=qt ac_add_options --with-qt ac_add_options --with-qtdir=/usr/share/qt3 ac_add_options --enable-chrome-format=symlink
Reporter | ||
Comment 19•20 years ago
|
||
qlist.h is a deprecated header which I simply forgot to removed and apperently some distros stopped including it. Either way I just fixed it.
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•