348533 - Build break due to missing glue

Status: RESOLVED FIXED

(Toolkit Graveyard :: XULRunner, defect)

Description

[Andy Willis (abwillis)]

E:\cvs\work\mozilla\xulobj\xpcom\stub\nsOS2VACLegacy.o(nsOS2VACLegacy.o) : error
 LNK2029: "nsCOMPtr_base::~nsCOMPtr_base()" : unresolved external
E:\cvs\work\mozilla\xulobj\xpcom\stub\nsOS2VACLegacy.o(nsOS2VACLegacy.o) : error
 LNK2029: "nsCOMPtr_base::begin_assignment()" : unresolved external

I think this will also affect Firefox building with lib-xul but haven't gotten that far in building yet.

I have a patch that will follow but not entirely certain if it is the proper fix but I have verified it does work.

Comment 1

[Andy Willis (abwillis)]

Attachment: link xpcomglue_s for OS/2 builds

xpcomglue_s is statically linked for nsOS2VACLegacy which is needed for plugin support such as Java and Flash.  As I am fairly certain this will affect Firefox built with libxul as well as xulrunner.

Comment 2

[Peter Weilbacher]

Until I find time to build xulrunner or FF with libxul I cannot really comment, but it doesn't seem worse than the other hacks in that file. Although I wonder if not a simple "-lxpcomglue_s" would achieve the same...

Comment 3

[Andy Willis (abwillis)]

It may not need more than -lxpcomglue_s but I tried to copy other instances of xpcomglue_s usage.

Comment 4

[Andy Willis (abwillis)]

Attachment: cleaner method

Found another patch that had this method.

Comment 5

[Peter Weilbacher]

Comment on attachment 233521 [details] [diff] [review]
cleaner method

Yes, that looks fine to me.
Perhaps we should get confirmation from somebody knowing build config better.

Comment 6

[Peter Weilbacher]

Andy, I just tried to build Firefox from trunk with --enable-libxul. Even with the patch I get

   weakld: cannot open library file 'G:\Prog\GCC335\lib\xpcom_s.a'.
   emxomfld: weak prelinker failed. (rc=8)
   make.exe[1]: *** [xpcom.dll] Error 1
   make.exe[1]: Leaving directory `m:/trunk/fx_libxul/xpcom/stub'

Do you not get that? Your first patch works, as does
   EXTRA_DSO_LDOPTS += -lxpcomglue_s

XPCOM_GLUE_LDOPTS is defined as 
$(LIBXUL_DIST)/lib/$(LIB_PREFIX)xpcomglue_s.$(LIB_SUFFIX) $(XPCOM_FROZEN_LDOPTS)
in config/autoconf.mk but XPCOM_FROZEN_LDOPTS contains -lxpcom here, so that's where this comes from.

Not sure what to do...

Comment 7

[Benjamin Smedberg]

Comment on attachment 233521 [details] [diff] [review]
cleaner method

This is not correct. You need to be linking against libxul to pick up the correct symbols.

Comment 8

[Peter Weilbacher]

I am confused. libxul is linked already, at least if -lxul is meant by that, and the missing symbols are listed in toolkit/library/xul.map, but that doesn't help?!

Comment 9

[Andy Willis (abwillis)]

This seems to be a recurring theme.  I keep finding where the map file shows a symbol but it is not in libxul.  The last time it finally cleared itself up before I was able to track down why it was not there.  Usually, though, it has been a function in dlldeps.cpp so I recognized fairly quickly that it was supposed to be in libxul (I didn't recognize it this time so didn't think to look in the map file).  I really have not been able to figure out why they show in the map file but not the library.

Comment 10

[Andy Willis (abwillis)]

Attachment: better fix

There are two locations where nsOS2VACLegacy.cpp reside.  It may be possible/better to just use the one in build instead of stub entirely.

Comment 11

[Benjamin Smedberg]

Comment on attachment 236578 [details] [diff] [review]
better fix

I'm not sure I understand what this file is for. In particular, it looks like we're exporting nonfrozen vtable functions for an older compiler; why? Since we're going to stop exporting them for *any* compiler soon, the compatibility hacks make me nervous.

Comment 12

[Peter Weilbacher]

Perhaps Knut can enlighten us, is nsOS2VACLegacy.cpp still used in GCC builds for OS/2 plugins? Do we still need it in __declspec builds?

Comment 13

[knut st. osmundsen]

What kind of enlightenment do you seek from me? I think the file description in nsOS2VACLegacy.cpp should be pretty clear. Although I agree that it should've explicitly mentioned that these function must be exported from XPCOM.DLL for any of the code to be useful.

What about __declspec()? I don't see any connection here really, unless the __declspec changes broke something - but that's different bug and a different problem.

The question is very simply, are you going to support the oji plugin from IBM java 1.3.x or are you not? Seamonkey/Mozilla/Firefox/whatever will terminate with a SYS2070 if you try to use the oji plugin without these exports (and the ones from nspr4.dll IIRC). From the short exchange we had on the subject in #warpzilla a while back, I think we kind of concluded that OS/2 users are, well, OS/2 users, so we can probably forget about dropping it. :-)

Comment 14

[Mike Kaply [:mkaply]]

Could we create another DLL besides XPCOM.DLL that forwards and require a DLLRNAME on the Java plugin?

Comment 15

[knut st. osmundsen]

That could work.

Comment 16

[Benjamin Smedberg]

Comment on attachment 236578 [details] [diff] [review]
better fix

I don't think we have much choice about dropping the old OJI if libxul is actually going to stop exporting nonfrozen symbols.

Comment 17

[Walter Meinl]

(In reply to comment #0)
> 
> I think this will also affect Firefox building with lib-xul but haven't gotten
> that far in building yet.
> 
Andy, you were right, now that libxul is enabled by default again the firefox build brakes exactly with the same error output as you posted in comment #0 . So, how do we get further with this? Should we drop the support for java131?

Comment 18

[Andy Willis (abwillis)]

I haven't built too recently but the patch should allow you to build but the only options are:
1:  This patch.
2:  Create a new dll.
3.  Drop support for Java 131.  
I myself have no problem with dropping the support but right now I have my patch applied even though I haven't used Java 131 in years.

Comment 19

[Walter Meinl]

(In reply to comment #18)
> I haven't built too recently but the patch should allow you to build but the
> only options are:
> 1:  This patch.
The patch still works, but obviously has no chance to be checked in as such. I removed the first part of it (xpcom/build/Makefile.in). The second part alone is enough to build libxul. I'd guess only the patch for xpcom/stub/Makefile.in could get an r+, cause it omits the linkage of nsOS2VACLegacy.cpp in case we build libxul. Of course that would mean for the moment that we drop the support of java131.On the other hand we can build again.

> 2:  Create a new dll.
> 3.  Drop support for Java 131.  

In fact, it would allow us also to postpone the decision what we finally do with java131.
From my experience with a libxul-enabled build, neither java131 nor java142_09 do work, I guess libxul stops also exporting symbols needed by the ipluginw.dll, but that has to be confirmed in another bug.

Comment 20

[Peter Weilbacher]

Do we actually _want_ to continue supporting Java131? Even the latest Java for OS/2 (1.4.2_09 from Innotek) by now has a few known security holes, so I think that 1.3.1 would be really dangerous to use now. And Innotek's version is free... If somebody really still wants support they could pay GoldenCode for their version. So I don't really see the point.

And yes, the latest available version of ipluginw is outdated by now, too. I don't understand why it still works for (some?) people on trunk.

Comment 21

[Peter Weilbacher]

Attachment: don't compile nsOS2VACLegacy.cpp with libxul

This seems to work.

Comment 22

[Andy Willis (abwillis)]

Java 1.3.1 is possibly as secure as our 1.4.2 seeings as IBM released updates to it probably as recently as our 1.4.2... the question is our usable is 1.3.1 at this point.  Seeings as 1.5 has been out for some years now and 1.6 is available it is less and less useful.  Seeings as the change to get it to work is makefile change and OS/2 only it wouldn't affect anyone else if we retained the support.  I haven't used 1.3.1 in years so I really don't know how useful it even is now, I am not adverse to removing the support (though I would probably just remove it in general and not just from libxul builds).  Seeings as I don't use 1.3.1 I have no dog in the fight, I hate to remove functionality if there are people using it.  I am sure some are because they don't want to pay for the GC version and don't want to run a "non-native" version (we are talking about OS/2 user's after all).  Peter, I'll let you or Mike decide whether to remove it.
I just double checked with Knut and he is fairly certain that it will not affect the old Netscape plugins to remove this (if they even work at this point, not that they would be really useful).

Comment 23

[Peter Weilbacher]

We have little choice (see comment 16). After we have made this change we can post to the newsgroup and if anyone feels strongly about having Java131 again, he/she can make the DLL or pay someone to do it. None of us is motivated to do it, so that's the only way...

Btw, do you have details on bugs fixed in Java 1.3.1? What was the last release date? Innotek's version was last updated on 2005-09-20 AFAICS.

Comment 24

[Walter Meinl]

Just a little comment regarding nsOS2VACLegacy.cpp residing in two directories. The one in xpcom/build (linking against xpcomcor in non-libxul builds) should have been probably cvs removed, see bug 266785#24. Anyway, if I understood the discussion in bug 266785 correctly, the actual ipluginw.dll would only import from xpcom.dll (compiled in xpcom/stub) and not xpcomcor.dll (xpcom/build). In libxul builds xpcomcor.lib is embedded in xul.dll.

Comment 25

[Peter Weilbacher]

Patch checked in.