477155 - build failure and crash on X11.

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ja; rv:1.8.0.4) Gecko/20060508 Firefox/3.0
Build Identifier: 

build failure and crash on x11.
On X11 environment, libxul is still linked so it causes crash at start up time.
And link path for test app should be "-L../x11/.libs" instead of "-L../x11".

Reproducible: Always

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix link path and remove libxul dependency.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 360823 [details] [diff] [review]
Fix link path and remove libxul dependency.

Anton, can you take a look at this patch?

Comment 3

[Anton Rogaynis (antonr)]

valid and patch fixes the problem, but

1. don't think '-L../x11/.libs' needed at all.
Since you have to 'make install' in gtk/x11 before compiling test, it will install .pc file and libs.

2. Would be nice to have more build cleaning here:
Don't think MOZILLA_JS_(CFLAGS and LIBS) needed in 
gtk/x11/configure.ac and
gtk/x11/Makefile.am

3. Also there is no need to have MOZ_(CFLAGS and LIBS) in gtk/tests/Makefile.x11

Comment 4

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #3)

> 1. don't think '-L../x11/.libs' needed at all.
> Since you have to 'make install' in gtk/x11 before compiling test, it will
> install .pc file and libs.

I think test program should be built before the library is installed. But yes, '-L../x11/.libs' is ugly.
It should be fixed.

> 2. Would be nice to have more build cleaning here:
> Don't think MOZILLA_JS_(CFLAGS and LIBS) needed in 
> gtk/x11/configure.ac and
> gtk/x11/Makefile.am

Sure.

> 3. Also there is no need to have MOZ_(CFLAGS and LIBS) in
> gtk/tests/Makefile.x11

Also MOZ_DLL_SUFFIX is not needed at all?

And after some thought, win32 target should be built by cross compiling.
I will post a new patch to incorporate win32 target and test program in autotools staff.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: autotools incorporation patch

Test program, x11 and win32 target are incorporated in autotools staff.

Also move autogen.sh, configure.ac, libmozwebview.pc.in into gtk/. 
Also remove libxul and mozjs dependencies.
Also remove MOZ_DLL_SUFFIX.

Cross compiling for win32 fails yet, I will work for as soon as possible.

Comment 6

[Anton Rogaynis (antonr)]

> 
> I think test program should be built before the library is installed. But yes,
> '-L../x11/.libs' is ugly.
> It should be fixed.

That's normal if you do 'make install'.
In case someone doing it's own test app (not under embedding folder) it also requires library installed.

> > 3. Also there is no need to have MOZ_(CFLAGS and LIBS) in
> > gtk/tests/Makefile.x11
> 
> Also MOZ_DLL_SUFFIX is not needed at all?
It's needed if you're building a library for OS10 for example. 

> 
> And after some thought, win32 target should be built by cross compiling.
> I will post a new patch to incorporate win32 target and test program in
> autotools staff.

I think this should be a separate bug, since it offers new feature and not concerning this bug.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #6)
> > 
> > I think test program should be built before the library is installed. But yes,
> > '-L../x11/.libs' is ugly.
> > It should be fixed.
> 
> That's normal if you do 'make install'.
> In case someone doing it's own test app (not under embedding folder) it also
> requires library installed.

Yes, I know. But in this case, the test app is inside embedding tree. The app should be built without installation the library.

> > > 3. Also there is no need to have MOZ_(CFLAGS and LIBS) in
> > > gtk/tests/Makefile.x11
> > 
> > Also MOZ_DLL_SUFFIX is not needed at all?
> It's needed if you're building a library for OS10 for example. 

Ah, I see. 
 
> > And after some thought, win32 target should be built by cross compiling.
> > I will post a new patch to incorporate win32 target and test program in
> > autotools staff.
> 
> I think this should be a separate bug, since it offers new feature and not
> concerning this bug.

OK. I will open a new bug entry for it.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: autotools incorporation patch v2

Also move autogen.sh, configure.ac, libmozwebview.pc.in into gtk/. 
Also remove libxul and mozjs dependencies.

Comment 9

[Anton Rogaynis (antonr)]

(In reply to comment #7)
> (In reply to comment #6)
> > > 
> > > I think test program should be built before the library is installed. But yes,
> > > '-L../x11/.libs' is ugly.
> > > It should be fixed.
> > 
> > That's normal if you do 'make install'.
> > In case someone doing it's own test app (not under embedding folder) it also
> > requires library installed.
> 
> Yes, I know. But in this case, the test app is inside embedding tree. The app
> should be built without installation the library.

If an application is inside embedding tree or outside it does not matter if you do 'make install'. 

> > > And after some thought, win32 target should be built by cross compiling.
> > > I will post a new patch to incorporate win32 target and test program in
> > > autotools staff.
> > 
> > I think this should be a separate bug, since it offers new feature and not
> > concerning this bug.
> 
> OK. I will open a new bug entry for it.

Thanks.

(In reply to comment #8)
> Created an attachment (id=361681) [details]
> autotools incorporation patch v2
> 
> Also move autogen.sh, configure.ac, libmozwebview.pc.in into gtk/. 
> Also remove libxul and mozjs dependencies.

May be I wasn't clear enough here. 'Incorporation patch' offers (at least starting to offer) a new way of building which requires making decision for which platform and toolkit you're going to build your embedding. See, there might be a case when you have X11 environment for win32.
That's why currently embedding has separate building places. If you have one building environment (like Incorporation patch proposing now for x11 and later for win32) then user should be asked for what is expected to build. 
That why I suggested to make a new bug.

For this bug is ok to have suggestions which I proposed in comment #3

Comment 10

[Jerry Talton]

When I apply the second revision of the patch linked above and run the new autogen.sh in gtk/, I get:

+ libtoolize --automake
+ aclocal
+ autoconf
+ autoheader
+ automake --add-missing --foreign
configure.ac: installing `./install-sh'
configure.ac: installing `./missing'
tests/Makefile.am: installing `./depcomp'
configure.ac:55: installing `./config.guess'
configure.ac:55: installing `./config.sub'
configure.ac:78: required file `common/Makefile.in' not found
configure.ac:78: required file `x11/Makefile.in' not found

Are the Makefile.in files missing, or did I screw something up?

Comment 11

[Anton Rogaynis (antonr)]

Actually first patch is fine. Just some minor modifications required (comment #3).

Comment 12

[Anton Rogaynis (antonr)]

Attachment: simple fix

mark incorporation patch v2 as obsolete because it's incomplete.

Comment 13

[Anton Rogaynis (antonr)]

Attachment: little better simple fix

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Hiroyuki - can you verify that attachment 363863 [details] [diff] [review] works for you too?

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/incubator/embedding/rev/cecafde2dd75

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Ooops! I am sorry I did not aware the progress of this bug since I forgot to add me to CC list.
I will try the latest trunk soon.