As we found out in bug 369791 the Innotek Plugin Wrapper doesn't work reliably on trunk any more, especially libxul builds seem to be problematic.

OK, this is basically the code that we were given by InnoTek. I just added proper tri-license file headers (including my name :-)) and the dummy StatusLine() method to the UpHTTPHeaderListener class to make it compile. As can be seen from the patch I thought a good location could be modules/plugin/os2wrapper/. I tried this with a Firefox debug build and a SeaMonkey build, so no libxul, yet.

OK, the libxul build needs to link against xpcomglue_s, but otherwise it seems to work. Updated patch in a second. I tested Firefox with - standard debug build (shared), standard optimized build (libxul), and static build - Java Plugin of GoldenCode Java 1.4.1_07 and Innotek Java 1.4.2_09. - The page http://javatester.org/version.html In all cases the Java plugin loaded now.

So, the only change with this patch is with modules\plugin\os2wrapper\Makefile.in that now contains three extra lines to handle linking with libxul. Asking Mike for review but if anybody else can test the patch, please do. Note that I didn't look through the code at all, I just tried to add proper license headers and fixed on build problem.

Walter, if you have time to test, would be great if you could.

(In reply to comment #3) > Created an attachment (id=318015) [details] > ipluginw into trunk v2 (fixed for libxul), zipped > > So, the only change with this patch is with > modules\plugin\os2wrapper\Makefile.in that now contains three extra lines to > handle linking with libxul. > I can confirm, that ipluginw.dll now works with static and libxul builds. But I had a compile issue with a libxul build, it built only in a _not_ cleaned objdir, then it found xpcom.lib. Building with a clean objdir resulted in: echo LIBRARY ipluginw INITINSTANCE TERMINSTANCE > ipluginw.def echo PROTMODE >> ipluginw.def echo CODE LOADONCALL MOVEABLE DISCARDABLE >> ipluginw.def echo DATA PRELOAD MOVEABLE MULTIPLE NONSHARED >> ipluginw.def echo EXPORTS >> ipluginw.def echo _NSGetModule >> ipluginw.def make.exe[5]: *** No rule to make target `../../../dist/lib/xpcom.lib', needed by `ipluginw.dll'. Stop. Looking at the build logs I saw, that xpcom.lib is built after xul.lib, thus relatively late in the build process, so we need to build ipluginw.dll after that in libxul builds. I did not yet build ipluginw.dll during a static build, but I guess this shouldn't be a problem, because xpcom.lib is built early IIRC. As a site note, in a firefox-on-top-of-xulrunner build (I had laying around), when I start a java tester page, I get a message that the correct nspr4.dll couldn't not be found (but that's probably for another bug).

I tested the libxul enabled ipluginw.dll in the meantime with Firefox-2.0.0.14 and Seamonkey-1.1.9 (both official) and it works also with the Branch versions. Another question is, if we should really integrate this in the main tree, such that it gets build every time. Wouldn't it be sufficient to treat it as an extension and let it live on Addons-mozilla-org or mozdev.org? I mean, as long as the mozilla code doesn't change such massively as it was with libxul a rebuild wouldn't be necessary too often. I will look this evening if I can overhaul the install script from the old xpi, that the script gets compatible with trunk (still the script works for branch installs). (Un)fortunately I'll be absent from beginning to mid of May, so it could be that it would take a while.

One of the most often asked support question is: why does my plugin not work. And very often the answer is: you need to install ipluginw. So I am very much in favor of integrating it into the tree, it doesn't cost much time to build it. We could restrict to build it only with Firefox and SeaMonkey, though. (Is the stuff in modules/plugin built at all for Thunderbird?) (In reply to comment #5) > make.exe[5]: *** No rule to make target `../../../dist/lib/xpcom.lib', needed > by `ipluginw.dll'. Stop. Oops. Had the same thing but even if I removed compilation of the os2wrapper subdir I got the same problem in the next directory (which was I think plugin/base/src). So I thought that was a general problem. So I did "make clean" and "make" in the top obj dir and it worked again. > Looking at the build logs I saw, that xpcom.lib is built after xul.lib, thus > relatively late in the build process, so we need to build ipluginw.dll after > that in libxul builds. Any idea how to do that? xpcom.lib is the lib that is built in xpcom/stub, right? So maybe it helps to _not_ set DIRS to os2wrapper in modules/plugin/Makefile.in but instead add it to tier_toolkit_dirs (in toolkit/toolkit-tiers.mk, as is done for xpcom/stub) but only for OS/2. > As a site note, in a firefox-on-top-of-xulrunner build (I had laying around), > when I start a java tester page, I get a message that the correct nspr4.dll > couldn't not be found (but that's probably for another bug). There are some build config options available in the ipluginw sources that we could use to circumvent this. There are at least _NO_XPCOMDLL_ and _MINIMAL_XPCOMDLL_ that switch dependency between NSPR and/or XPCOM.

Comment on attachment 318015 [details] ipluginw into trunk v2 (fixed for libxul), zipped Canceling review request for now, we need to fix some build settings first.

For posterity, the note from Achim. Mike, we had submitted these sources to you and Peter Weilbacher years ago with the explicit permission to put them into the Mozilla source tree for better interface stability. Back then we also gave you permission to apply the typical Mozilla MPL/GPL/LGPL licensing to the file. This permission is of course still valid. -- Achim Hasenmueller Director Engineering, VirtualBox Sun Microsystems GmbH Werkstrasse 24 71384 Weinstadt, Germany phone: +49 7151 604050 All this code was created by Innotek - there is no code written by any other entity.

OK, leaving away the new files in modules/plugin/os2wrapper for the moment, these changes should at least build Firefox in the configurations debug/shared, libxul/standard, and static. Walter, does this look OK for you? No clue about the firefox-on-xulrunner problem...

(In reply to comment #10) > Created an attachment (id=318240) [details] > build changes only, v3 > > OK, leaving away the new files in modules/plugin/os2wrapper for the moment, > these changes should at least build Firefox in the configurations debug/shared, > libxul/standard, and static. > Walter, does this look OK for you? > I could successfully build a libxul and a static build with your patch. I was also playing around with your suggestion, made probably a typo, so my similar patch didn't work. However, cause I failed with this approach I was searching further and looking at the log, what gets build _after_ xpcom.lib in a libxul build I found, that npnulos2 is one of those. It is not added to the Makefile.in with DIRS but with TOOL_DIRS. I then added also os2wrapper to TOOL_DIRS and ipluginw has gotton build after xpcom.lib with enable-libxul and also with enable-static. I attached the patch here (sorry, the attachment might have DOS-format). Later I found also bug 273598 introducing the TOOL_DIRS variable for the reason, we failed with your second patch. So, now you have the choice, in principle, both patches would work. Some further questions: in a static build, ipluginw.lib gets linked into the exe, is this your intention? If not, you'd have to exchange 'EXPORT_LIBRARY =1' against 'FORCE_SHARED_LIB = 1' in os2wrapper/Makefile.in In the build log I saw that the Makefile in os2wrapper isn't created during the initial Phase when all Makefiles are created. We would have to add probably modules/plugin/os2wrapper/Makefile to toolkit/toolkit-makefiles.sh and also the new directory to client.mk under 'MODULES_core :=' to get it checked out. Probably, we need a review from a build core owner. > No clue about the firefox-on-xulrunner problem... > Never mind, who is building it on OS/2 except me. I'll try something when I have time later and if I see some success I'll open a follow up bug.

Ok, I've tried building Firefox with Patch #1 which died due to no xpcom.lib. Now I tried with the 3rd patch manually applied to patch #1 (didn't worry about patch #2 as you stated it was for xulrunner) This patch still did not create a ipluginw.dll but the compile got much further though it seems to of taken a weird route. It died here Creating Resource file: xulrunos2.res rc.exe -n -i /mozilla/widget/src/os2 -i \mozilla\toolkit\library -r /mozilla/too lkit/library/xulrunos2.rc xulrunos2.res Compile ending. CLB:Informational 4025:The MOZILLA/ flag is not a valid option CLB:Informational 4025:The MOZILLA/ flag is not a valid option RC :Error 2002:RC expected at least one argument for -i. RC :Error 2003:This input is incorrect: [/mozilla/toolkit/library/xulrunos2.rc] RC :Error 2003:This input is incorrect: [/mozilla/widget/src/os2] RC :Error 1008:The input and output file names must be different; both are xulru nos2.res. This is building a regular shared Firefox

(In reply to comment #12) > Ok, I've tried building Firefox with Patch #1 which died due to no xpcom.lib. > Now I tried with the 3rd patch manually applied to patch #1 (didn't worry about > patch #2 as you stated it was for xulrunner) > This patch still did not create a ipluginw.dll but the compile got much Dave, try again to adapt Patch #3 to Patch #2. Patch #2 is needed for libxul, that is not necessary the same as XULRunner. When you build a firefox without specifying --enable-shared or --enable-static, the buildsystem will create a huge xul.dll out of the most dll's that are else separate, and then you'll need the linking against xpcomglue_s. And recreate your objdir otherwise you may run into trouble. > > This is building a regular shared Firefox > Fine, for a regular shared Firefox Patch #1 should work, but Patch #2 should work as well. (In reply to comment #10) > No clue about the firefox-on-xulrunner problem... > The problem seems to be the directory structure, in such a build you have top-directory containing the files only needed by the browser. Firefox.exe is a renamed xulrunner-stub.exe. Below this directory you have a complete xulrunner build, with the nspr dll's. Ipluginw.dll needs to have the main exe (the renamed xulrunner-stub.exe) to be in the same directory as xpcom.dll and nspr4.dll (maybe also xul.dll) due to using libpathstrict. However, you can start also firefox by invoking xulrunner from it's own directory when you point to the application.ini of the firefox minimal build (e.g. xulrunner ..\firefoxontop\application.ini), then the main exe is in the path of xpcom.dll and nspr4.dll and ipluginw.dll works. As stated above I'll open a separate bug in case I'll find at least something to work with.

Walter, thanks for your many comments. I was away the last two nights, so I didn't look at this again. I don't think we should add the os2wrapper Makefile to toolkit-makefiles.sh. While it is then created upfront, right after the configure run, it gets perfectly well created later in the process with only minimal addition to build time. And then we don't force other platforms to create a (for them) useless Makefile. I before had the impression that TOOL_DIRS was only used for debugging stuff (plugin samples etc.) but apparently that was wrong. So, yes I think we should use that. Then your patch in attachment 318351 [details] [diff] [review] would be the only necessary change outside the os2wrapper directory, right? You are also right that we should use FORCE_SHARED_LIB. It doesn't matter for the functionality (I think) but I would like people who might blindly copy over old versions of this DLLs because they are used to (which could lead to crashes) to notice that it is already there. Dave, your build failure looks really weird, I haven't seen forward slashes passed to rc.exe. I hope Walter's tips helped, although that problem looks somewhat unrelated. Anything else I have forgotten to address? Will upload a new full patch with all these changes in a minute.

Mike, is this OK for you from the OS/2 perspective?

I thought maybe we should ask someone from build to review, too, but actually the build changes are minimal, so that doesn't seem to be necessary any more.

Comment on attachment 318784 [details] [diff] [review] ipluginw into trunk v4 Looks great to me. Asking for 1.9 approval. This is totally OS/2 only code for getting the java plugin working. It looks like a lot of new code but it has been around for quite a while, just not in the tree.

Following Walters comments I did get Firefox to build fine (doesn't run though, patched or unpatched so separate issue). As for the build failure, I'm pretty sure forward slashes have been being passed to rc.exe for years. At that when it first started happening I got an updated rc.exe from Mike which handles slashes fine. Looking at the successful build of Firefox the only difference is the successful one included the drive letter in what was passed to rc.exe eg rc.exe -n -i I:/mozilla/widget/src/os2 -i I:\mozilla\toolkit\library -r I:/mozilla/toolkit/library/xulrunos2.rc xulrunos2.res RC is creating binary resource file xulrunos2.res Anyways will be good to not have to remember to copy ipluginw.dll over.

Comment on attachment 318784 [details] [diff] [review] ipluginw into trunk v4 a1.9=beltzner

Patch checked into trunk, yay! This finally resolves the last advantage my enhanced builds had over the standard packages.