156593 - Remove the dependence to gtk1 when the default toolkit is gtk2

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[John Sun]

Now after the build configured with the option enable-default-toolkit=gtk2,
glib1/gtk1 is still neccessary for the build success. 
Should change the situation.

Comment 1

[John Sun]

Attachment: V1 -- RFC

Note:
a) Not completely remove the check scripts for old glib 1.
   It doesn't block work, but it's good enough?
   
b) For new libIDL package, the config script file is named as libIDL-config-2
   in Solaris and Linux platform, but I'm not sure if it's a normal condition
   in other platform.
c) Now there are errors in libIDL-config-2, but I'm not sure if it's a normal  
  condition in other platform.
   Although new libIDL based on glib2, the script file libIDL-config-2 use     
glib-config to inport glib's config.
   Moreover, libIDL-config-2 export -lIDL as part of its cflags, while it's lib

   name is libIDL-2 and never create link to libIDL.

  Pls refer to the next patch jsut for libIDL-config-2.

Comment 2

[John Sun]

Attachment: patch for libIDL-config-2

patch for libIDL-config-2

Comment 3

[John Sun]

see Comment #1
a) Not completely remove the check scripts for old glib 1.
Should be 
a ) Not completely avoid the execution of the check scripts for old glib/gtk one
when enable-default-toolkit=gtk2.

Comment 4

[John Sun]

Blizzard,
If it's neccessary
a) Completely avoid the execution of the check scripts for old glib/gtk one
when enable-default-toolkit=gtk2.
b) Add some scripts to futherly check gtk2 environment like those for glib1/gtk1,

Pls give your suggestion
   

Comment 5

[Idan Gazit]

Hey,

I've been working on getting a gtk2-only build (cvs trunk) for a week now and
I've finally gotten a compile to work, based off the patches above. Now, some of
my trouble involves autoconf and I know only what I have learned in the past
week (which isn't much). Some notes:

The autoconf macro patch (libIDL.m4) seems to do most of the trick.
Unfortunately,  I still had to make the changes by hand to ./configure, as
running autoconf barfed when it ran across:

MOZ_READ_MOZCONFIG(MOZ_TOPSRCDIR)

at the end of build/autoconf/altoptions.m4. Removing the offending macro
produced a ./configure that didn't work for me (specifically, it died when it
couldn't find XIE). So, I "patched" ./configure by hand with the changes that
would have happened had autoconf worked. I'll polish up the patch shortly and
post it.

So, having gotten past ./configure, the next stumbling block appeared to be just
1 file at the beginning of the build process: config/elf-dynstr-gc.c. Apparently
 that specific makefile doesn't get passed the new glib-2.0 CFLAGS and LDFLAGS
that are discovered in ./configure, MOZ_GTK2_CFLAGS and MOZ_GTK2_LIBS (by way of
pkg-config). So, the build dies noisily off the start with this specific
sourcefile. I ran ./configure once more after setting env vars CFLAGS and
LDFLAGS to what should be discovered in ./configure (e.g. export
CFLAGS=`pkg-config --cflags gtk+-2.0`, etc). I'm posting this comment from my
build. (Yay!)

There's probably somewhere in ./configure that needs to pass the gtk2 cflags and
libs to /config's makefile, but I have yet to figure it all out (still slogging
away). Forgive me, but I'm new to autoconf.

Conclusions:

1. The above patch does the job, but perhaps I am not using autoconf correctly
so I had to do it by hand.

2. Something needs to pass the gtk-2 cflags and libs to /config's makefile. If I
work it out I'll post a patch.

Any constructive criticism is welcome, just trying to donate some effort here :)

Idan

Comment 6

[John Sun]

Idan,
As for your conclusion.
1) Which platform are you working on? On solaris 9 I can't make the configure
while I can do it smoothly on Redhat72.
   I'll upload a patch for configure later.
2) I guess you miss the option enable-default-toolkit=gtk2, right?

Thanks for testing my patch, I'll enhance it later.

Comment 7

[Idan Gazit]

John,

1) I'm building on a compiled-from-scratch linux, 2.4.18, gcc 2.95.

2) The three different options I used to enable gtk2 were:

--enable-default-toolkit=gtk2
--enable-toolkit-gtk2
--with-gtk2

Apparently these three activate different sets of #ifdefs that contribute to 
the gtk2 build. Maybe not all are needed, but it doesn't hurt to have them 
there ;)

I'll have my ./configure patch up by tonight, hopefully. I want to figure out 
and integrate the CFLAGS/LDFLAGS fix before I post, but if I can't figure it 
out by tonight I'll just post the ./configure patch and post an update some 
time later.

So far so good with my moz build. :)

Idan

Comment 8

[John Sun]

Attachment: v2

Add removing glib1 check when default toolkit is gtk2.

seeking r=?

Seawood, could you review it or give a comment? thanks!

Comment 9

[John Sun]

Attachment: configure I made (gz format)

Idan,
Perhaps you can use that configure when having trouble to make it.

BTW, you can just use the option 
--enable-default-toolkit=gtk2
to enable gtk2 build.

Any suggestions are welcomed!

Comment 10

[Idan Gazit]

Attachment: Removes gtk1 deps when configuring for gtk2

Okeydokey,

Sorry for the delay, I had meatspace obligations to attend to.

This patch is basically a workaround for those of us who can't get a working
autoconf (like me :) ). It basically does what John Sun's autoconf libIDL.m4
patch does, with some minor differences. Some notes:

1) This patch, in addition to including if-else blocks so as not to break gtk1
builds, also takes out gmodule. There might be a way to build with gmodule from
glib2 but I haven't worked it out yet, I probably need to add a conditional
block there with `pkg-config --cflags gmodule-2.0` or the like, but for now it
is looking for glib-config (which is obviously not there in glib2-only
systems).

2) [Minor] I added in echos of ./configure's decision not to check for glib1
when MOZ_ENABLE_GTK2 = 1 to config.log as well as to the console.

3) There seemed to be a problem with /config/elf-dynstr-gc.c not getting passed
the gtk2 cflags/libs even though it is dependent on them. My hacked solution
was as follows: upon getting the gtk2 cflags/libs from pkg-config, I copied the
same cflags/libs into $GLIB_CFLAGS and $GLIB_LIBS. This is probably not the
right solution; chances are that /config/Makefile.in needs to be smart enough
that when a gtk2 build is being configured then it takes MOZ_GTK2_CFLAGS and
MOZ_GTK2_LIBS instead of GLIB_CFLAGS and GLIB_LIBS. However, my solution makes
it work for now.

Let me know how it works for you. Obviously the right solution is doing it via
autoconf, but it seems to be broken for now.

Idan

Comment 11

[John Sun]

Idan,
The patch you uploaded can't be pathed to current configure, there is conflicts.
Since configure is from configure.in and other .m4 files, a little change in other
files maybe cause a big change in configure. You'd better directly upload configure.

I saw your patch, perhaps changes made manually are not complete, so cause issue
3 you talked about.  As for 1, although configure will give warning, i think,
gtk+-2.0 flags and libs cover gmodule's, then no actual effect on build.

If you have time, you can try the configure I made in gz format.
http://bugzilla.mozilla.org/attachment.cgi?id=92882&action=view

Good luck!

Comment 12

[hacker formerly known as [e-mail]]

Idan, your configure problems appear to stem from using autoconf 2.5x which we
do not support (see bug 104642).  If you want to hack on configure.in properly,
you'll need to install autoconf 2.1[23].

Comment 13

[hacker formerly known as [e-mail]]

Comment on attachment 92881 [details] [diff] [review]
v2

I'd rather just import libIDL-2.m4 rather than hack up our copy to handle
libIDL-2.  With the exception of the .m4 which implements the moz specific
options, those .m4 files should be left as pristine as possible and not contain
any moz-isms.

Comment 14

[Idan Gazit]

Chris: I am indeed running autoconf 2.5. Thanks for the pointer.

John: Strange that you're having trouble with the patch. I just cvs updated
(five minutes ago) and the patch still applies just fine. *shrug* I have yet to
build yet but I'm not even sure ./configure was updated since my past pull...
Note that I'm applying the patch to the stock ./configure that gets pulled from
trunk, so I don't know how it will deal with your autoconf generated
./configures; I surmise that they look different (especially as it sounds like
you're building on solaris and I on x86-gnu-linux).

I'm back in the saddle this week and I'll get to checking for the gmodule
problem, probably shouldn't be too difficult. I'll see about downgrading myself
to autoconf 2.1 -- I wasn't aware that there's such a problem with autoconf 2.5.

As for wading into writing .m4 files -- I'm over my head in that territory, at
least until I teach myself the basics of how autoconf works (I get the real
rudiments right now from messing around with it, but not much more).

The patch I have up basically does exactly what John's .m4 patch would do -- any
call in an .m4 file to AM_PATH_GLIB generates a whole block of checking code
that attempts to find glib 1 and associated sundries. I took a look inside
glib.m4 and basically, all code generated by the macro I placed a giant if
around and tested for $MOZ_ENABLE_GTK2 first (exactly like John's patch). Up top
in configure, where it tests for gtk2 I added two lines which set GLIB_CFLAGS
and GLIB_LIBS to their gtk2 equivalents in order to fix the elf-dynstr-gc.c
problem. 

The /config/elf-dynstr-gc.c problem doesn't reside in my configure patch -- all
of the checks that generate the gtk2 cflags and libs generate them just fine.
This really is a /config/Makefile.in problem or thereabouts. I don't know what
takes the variables generated in ./configure and then goes off to generate
makefiles, but that something needs to know about gtk2 and doesn't right now,
basically it invariably looks only for gtk1 $GLIB_CFLAGS and $GLIB_LIBS -- so my
ugly solution works, but it isn't the right one. A pointer in the direction of
this "something" would help me out -- I haven't been able to figure it out on my
own but I'm sure that if I was handed the target file it I could come up with a
patch.

The remaining bits in my patch basically add in John's conditional checks for
libIDL-config-2.

I'll try out your configure tomorrow and see how it goes, but for now my
configure is working and building flawlessly (knock wood).

I'll upload my full configure following this comment, check it out and tell me
if anything tickles your fancy.

--Idan

Comment 15

[Idan Gazit]

Attachment: patched configure (gzip)

This is my configure, the result of patching the cvs trunk configure with above
patch 93018 ("Removes gtk1 deps...")

For your look-over pleasure,

Idan

Comment 16

[John Sun]

Idan,
Sorry for my mistake. Perhaps updating configure cause the error -- old version
merged with new one. After delete configure then update it. It's ok.

I have tested you configure. It's ok for config. I'll make a build for testing
it. But I still feel strange about thoser isues related with gmodule and
elf-dynstr-gc.c, since I tested my configure on solaris and linux without
glib1/gtk1, it's ok.

Comment 17

[John Sun]

Idan,
I went over the gmodule related. 
Do you use --enable-ctl option to make ctl?
I tried it as well, but encounter error( the error msg is in the end)
It seem need to do porting on thai-x.c. 
Is your subject ctl now?


"./../pangoLite/pango-types.h", line 79: warning: typedef redeclared: gunichar
"./../pangoLite/pango-types.h", line 80: warning: typedef redeclared: gunichar2
"thai-x.c", line 108: identifier redeclared: g_utf8_skip
        current : array[256] of char
        previous: const pointer to const char :
"/opt/gnome-2.0/include/glib-2.0/glib/gunicode.h", line 164
"thai-x.c", line 118: warning: macro redefined: g_utf8_next_char
cc: acomp failed for thai-x.c
make[1]: *** [thai-x.o] Error 2
make[1]: Leaving directory
`/export/home/jsun/mozilla-build/mozilla-trunk/mozilla/extensions/ctl/src/thaiShaper'
make: *** [all] Error 2

Comment 18

[John Sun]

Attachment: proposed patch for thai-x.c

Idan,
Is your subject the file related? Pls take a look at the patch, and give some
comment.

Comment 19

[John Sun]

Attachment: v3 - all in one

According Seawood's suggestion, and
added dealing with gmodule

Seawood, 
pls review the patch.

Comment 20

[John Sun]

Attachment: v4 -- all in one

Add dealing with cflags to comiple cofig/elf-dynstr-gc.c

Comment 21

[John Sun]

Attachment: configure.gz

configure.gz

Comment 22

[hacker formerly known as [e-mail]]

Comment on attachment 95840 [details] [diff] [review]
v4 -- all in one

Looks good but I don't have gtk2 installed to test.  r=cls

Comment 23

[DJ]

Oops.  Some additional information about dependencies here:

attachment 95840 [details] [diff] [review] has a minor flaw with the libIDL-2.m4 it creates (using fresh
tarball from 1.1 release).  Using libIDL-config-2 appears deprecated (or
flawed).  I'm using libIDL 0.8.0 and the libIDL-config-2 outputs
-I/usr/include/glib-1.2 and -I/usr/lib/glib/include after compile for '--cflags'
and -lglib for '--libs', both of which are from the old glib1.x releases which
it should not be depending on.

I'm guessing the folks are using pkgconfig as default which is what configure
should be doing when checking for libIDL-2 nowadays.  I don't know much on
making a patch but I do know that libIDL-2 needs to be migrated to using
pkgconfig instead.  Comments anyone?

Comment 24

[John Sun]

DJ,
This is libIDL-config-2's flaw. Patch it with
http://bugzilla.mozilla.org/attachment.cgi?id=91932&action=view

Comment 25

[John Sun]

Attachment: refresh of v4

Refresh v4 patch. Mainly for ctl moved from extensions to intl

Comment 26

[Bolian Yin]

checked in trunk.

Comment 27

[Christian :Biesinger (don't email me, ping me on IRC)]

this broke the SunOS tinderbox, please fix it.
(error message is
/builds/tinderbox/SeaMonkey/SunOS_5.7_Depend/mozilla/configure: test: argument
expected)

Comment 28

[Thomas Leineweber]

I think, the line

      if test $MOZ_ENABLE_GTK2 = 1; then
in the patch for configure.in is the culprit. It is not defined for my
SunOS build and the test command fails with the message given by Christian

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

I suppose this'd work:
      if ! test -z "$MOZ_ENABLE_GTK2"; then

Comment 30

[Thomas Leineweber]

the fix given by Christian seems to be not the right one. There is another
error on my SunOS system. I'd suggest

   if test $MOZ_ENABLE_GTK2 ; then

as used in some other parts of the script. (Patch will be attached)

Comment 31

[Thomas Leineweber]

Attachment: Proposed patch to resolve error with the test command on SunOS

Comment 32

[Thomas Leineweber]

Attachment: Patch for configure.in (not configure)

the patch should be for configure.in, not configure (where is the brown paper
bag?)

Comment 33

[Thomas Leineweber]

Attachment: alternative solution for configure.in

This is a alternative patch suggested by Dave Baker in Bug 165432

Comment 34

[Bolian Yin]

Attachment: patch to fix tinderbox broken on SunOS

this patch has been checked in the trunk about 12 hours ago to fix the broken
tinderbox. Thanks for everyone.

Comment 35

[John Sun]

Thanks all of you very much, especially Bolian an Thomas.

Comment 36

[DJ]

Attachment: configure.in patch aftermath

Patch for configure.in (after all other patches were applied) to use
PKG_CHECK_MODULES instead.  This removes the need of adding another m4 file and
having to include it.  Because the tarball to libIDL2 depends on pkgconfig, I
figure it's better to use this instead.

One more thing, is it me or does attachment 97120 [details] [diff] [review] tried to patch files that're
no longer in that directory?  Still using stock 1.1 release tarball at the
moment.

Comment 37

[Bolian Yin]

DJ, the "ctl" directory has been moved from mozilla/extentions into mozilla/intl.

Comment 38

[hacker formerly known as [e-mail]]

Comment on attachment 97286 [details] [diff] [review]
configure.in patch aftermath

libIDL may use pkgconfig but we do not.  Where does the PKG_CHECK_MODULES macro
come from?  I suspect it comes from a .m4 provided by the pkgconfig package.

Comment 39

[Christopher Blizzard (:blizzard)]

build/autoconf/pkg.m4

It's been there for a long time and is used by the gtk2 build config.

Comment 40

[hacker formerly known as [e-mail]]

Comment on attachment 97286 [details] [diff] [review]
configure.in patch aftermath

Oh, hey, so there is.  Forgot all about that.

Comment 41

[Kyle Yuan]

CCing gtk2 group, removing atf group

Comment 42

[John Sun]

(For attachment 97286 [details] [diff] [review])
But somtimes the libIDL may be compiled and intalled as "make install",
so it's not added into pkg-config database. I'm not sure there must be a gnome2 
environment when building mozilla with gtk2. Moreover, libIDL's path can be
assigned by configure options.

Comment 43

[Christopher Blizzard (:blizzard)]

If you have gtk2, you have pkgconfig since there are no *-config files for gtk2.
 This has nothing to do with gnome2.

Comment 44

[DJ]

Attachment: aclocal.m4 patch to unpatch the other patch

(unpatches aclocal.m4 changes from attachment 97120 [details] [diff] [review], makes
build/autoconf/libIDL-2.m4 an idling file so it needs to be removed)

re: comment #42

Suppose for instance that an individual has a GTK2 environment.  Eventually
this'll be the case on GNOME2 computers.  But that's beside the point.	The
GTK2 suite requires pkg-config.  libIDL 0.6.x relies on old glib1.x if I am not
mistaken.  And the libIDL 0.8.0 release, which I dub as libIDL2, depends on
glib2 and thus depends on pkg-config.  In a likely case, an application using
GTK2 and depending on libIDL2 will use pkg-config to check for both of these
libraries.

I believe the patch for using PKG_CHECK_MODULES macro instead is more
effective, since it is assumed that, with GTK2 set as default toolkit, one is
likely to have libIDL2 installed.  And this particular bug, after all, aims in
removing dependence to gtk1 (which in effect removes deps on glib1 as well)
when using gtk2 for the toolkit.  As I mentioned in when I submitted the
previous patch, this removes the need of having another m4 file included in
aclocal.m4.  So I am requesting that these two patches be reviewed for
checkins.

Comment 45

[Jay Yan]

current trunk status:

mozilla/xpcom/typelib/xpidl can not build successfully.

I believe it is becasue of libIDL-config-2, please see the output of
libIDL-config-2 in my machine:


bash-2.05$ libIDL-config-2 --version
0.8.0
bash-2.05$ libIDL-config-2 --cflags
/usr/gnome/bin/libIDL-config-2: glib-config: not found
-I/usr/gnome/include/libIDL-2.0

Comment 46

[John Sun]

pls refer to comment #1 and patch
http://bugzilla.mozilla.org/attachment.cgi?id=91932&action=view

Comment 47

[Jay Yan]

Yes, I believe firmly that is the bug of libIDL0.8.0, so I logged a bug in
gnome.org just now:

http://bugzilla.gnome.org/show_bug.cgi?id=93469

Thanks John for the good patches.

Comment 48

[Christopher Blizzard (:blizzard)]

This slipped through because no one uses libIDL-config-2.  Everyone uses pkgconfig.

Comment 49

[hacker formerly known as [e-mail]]

Comment on attachment 97286 [details] [diff] [review]
configure.in patch aftermath

r=cls

Comment 50

[Christopher Blizzard (:blizzard)]

Checked in.  Thanks!

Comment 51

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 169635 has been marked as a duplicate of this bug. ***