Last Comment Bug 722262 - Port |Bug 552864 - Throw away wrapper shell script on unix and lazily load libxul| to SeaMonkey
: Port |Bug 552864 - Throw away wrapper shell script on unix and lazily load li...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Startup & Profiles (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.10
Assigned To: Justin Wood (:Callek)
:
Mentors:
Depends on: 549246 552864 668869 727258 727675
Blocks: 523773 670312 704835 729550
  Show dependency treegraph
 
Reported: 2012-01-30 03:19 PST by Serge Gautherie (:sgautherie)
Modified: 2012-07-12 03:22 PDT (History)
9 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wontfix
wontfix
wontfix


Attachments
WIP 0 (8.56 KB, patch)
2012-02-05 02:53 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
WIP 0.5 (10.14 KB, patch)
2012-02-05 23:27 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
WIP 1.0 (13.30 KB, patch)
2012-02-06 01:23 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
v1.0 (20.89 KB, patch)
2012-02-06 16:31 PST, Justin Wood (:Callek)
kairo: review+
Details | Diff | Splinter Review
v1.5 (21.94 KB, patch)
2012-02-09 23:47 PST, Justin Wood (:Callek)
bugspam.Callek: review+
standard8: review-
Details | Diff | Splinter Review
[m-c] v1 - include stuff from app (21.94 KB, patch)
2012-02-12 05:46 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
[c-c] v2 [Checked in: Comment 20] (22.78 KB, patch)
2012-02-12 05:48 PST, Justin Wood (:Callek)
mh+mozilla: review+
mnyromyr: feedback+
Details | Diff | Splinter Review
[m-c] v1 - include stuff from app (570 bytes, patch)
2012-02-12 05:49 PST, Justin Wood (:Callek)
no flags Details | Diff | Splinter Review
[m-c] v1.1 - include stuff from app [Checked in: Comment 19] (565 bytes, patch)
2012-02-12 06:02 PST, Justin Wood (:Callek)
mh+mozilla: review+
Details | Diff | Splinter Review
Fix ordering for extra app libs in dependentlibs.list [Checked in: Comment 23] (897 bytes, patch)
2012-02-14 11:50 PST, Mark Banner (:standard8) (afk until 26th July)
khuey: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2012-01-30 03:19:20 PST
See also TB bug 668869.
Comment 1 Justin Wood (:Callek) 2012-01-30 09:44:47 PST
I hope to do this this cycle (and have been tracking it to do so, just never got around to filing the bug, afaict)

It is too risky to take on aurora though so wontfix-2.9
Comment 2 Justin Wood (:Callek) 2012-02-05 02:53:53 PST
Created attachment 594527 [details] [diff] [review]
WIP 0

This is just a snapshot of where I am right now, I don't think this compiles (did not test yet) and I am pretty sure I still have a lot to do.

Notes:
* Yes I am adding features here at the same time (I think I need to turn on Telemetry for this code)
* I need to add some confvars code to support the gen app-ini stuff.
* Our Makefile for suite/app has a lot of cruft in it and will be (attempted) to be cleaned up here.
* I will be trying to limit the scope of this all to the bare minimum to support what I did in suite/app/*.cpp here and cleaning up that Makefile, beyond that (moving stuff, changing how extensions are packaged to match Firefox's test-pilot, etc.) will be out of scope here.
Comment 3 Justin Wood (:Callek) 2012-02-05 23:27:33 PST
Created attachment 594652 [details] [diff] [review]
WIP 0.5
Comment 4 Justin Wood (:Callek) 2012-02-06 01:23:03 PST
Created attachment 594658 [details] [diff] [review]
WIP 1.0

This is it for the night, this actually _does compile_ (on win anyway) and _does startup_.

I did not test much else yet, but this looks like a good WIP first step.
Comment 5 Justin Wood (:Callek) 2012-02-06 16:31:32 PST
Created attachment 594861 [details] [diff] [review]
v1.0

Ok, so far as I can tell, this should do it -- accurately.

I did not test on anything other than Win7, and I did add LIBXUL supporting code, even though we don't actively support it that way (yet).

I'm HOPING for a fast review, since this is relatively risky enough to want it to land early in the cycle
Comment 6 Robert Kaiser 2012-02-09 14:57:23 PST
Comment on attachment 594861 [details] [diff] [review]
v1.0

Review of attachment 594861 [details] [diff] [review]:
-----------------------------------------------------------------

I'm marking this reviewed, though I didn't exactly look at every line of this, in summary this looks good though, and if it builds on all platforms, I think it's a good step. Thanks for this work!
Comment 7 Justin Wood (:Callek) 2012-02-09 23:47:45 PST
Created attachment 595981 [details] [diff] [review]
v1.5

r+ from KaiRo, this has a few minor changes that should make a real difference for mac though
Comment 8 Mark Banner (:standard8) (afk until 26th July) 2012-02-10 12:22:46 PST
Comment on attachment 595981 [details] [diff] [review]
v1.5

This would break Linux builds - see bug 668869 comment 14.
Comment 9 Justin Wood (:Callek) 2012-02-12 02:06:51 PST
So following up and documenting here for sanity:

Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub
$ make echo-variable-DEPENDENT_LIBS_LIST
nspr4.dll plc4.dll plds4.dll mozalloc.dll  mozsqlite3.dll nssutil3.dll softokn3.dll nss3.dll ssl3.dl
l smime3.dll  mozjs.dll xul.dll

Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub
$ ls ../../dist/bin/*.dll
../../dist/bin/AccessibleMarshal.dll  ../../dist/bin/nsldappr32v60.dll
../../dist/bin/D3DCompiler_43.dll     ../../dist/bin/nsldif32v60.dll
../../dist/bin/IA2Marshal.dll         ../../dist/bin/nspr4.dll
../../dist/bin/MapiProxy.dll          ../../dist/bin/nss3.dll
../../dist/bin/crashinjectdll.dll     ../../dist/bin/nssckbi.dll
../../dist/bin/d3dx9_43.dll           ../../dist/bin/nssdbm3.dll
../../dist/bin/freebl3.dll            ../../dist/bin/nssutil3.dll
../../dist/bin/gkmedias.dll           ../../dist/bin/plc4.dll
../../dist/bin/libEGL.dll             ../../dist/bin/plds4.dll
../../dist/bin/libGLESv2.dll          ../../dist/bin/smime3.dll
../../dist/bin/mozMapi32.dll          ../../dist/bin/softokn3.dll
../../dist/bin/mozalloc.dll           ../../dist/bin/ssl3.dll
../../dist/bin/mozglue.dll            ../../dist/bin/vmwarerecordinghelper.dll
../../dist/bin/mozjs.dll              ../../dist/bin/xpcom.dll
../../dist/bin/mozsqlite3.dll         ../../dist/bin/xul.dll
../../dist/bin/nsldap32v60.dll

Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub
$ pushd ../../dist/bin/; rm -f `make -C ../../xpcom/stub echo-variable-DEPENDENT_LIBS_LIST`; ls *.d
ll; popd

AccessibleMarshal.dll  d3dx9_43.dll   mozMapi32.dll      nssckbi.dll
D3DCompiler_43.dll     freebl3.dll    mozglue.dll        nssdbm3.dll
IA2Marshal.dll         gkmedias.dll   nsldap32v60.dll    vmwarerecordinghelper.dll
MapiProxy.dll          libEGL.dll     nsldappr32v60.dll  xpcom.dll
crashinjectdll.dll     libGLESv2.dll  nsldif32v60.dll
Comment 10 Justin Wood (:Callek) 2012-02-12 05:44:53 PST
[seabld@cb-seamonkey-linux-01 bin]$ make -C ../../xpcom/stub echo-variable-DEPENDENT_LIBS_LIST
make: Entering directory `/builds/slave/comm-cen-trunk-lnx/build/objdir/mozilla/xpcom/stub'
libnspr4.so libplc4.so libplds4.so libmozalloc.so  libmozsqlite3.so libnssutil3.so libsoftokn3.so libnss3.so libssl3.so libsmime3.so  libxul.so
make: Leaving directory `/builds/slave/comm-cen-trunk-lnx/build/objdir/mozilla/xpcom/stub'

(I'm trimming out OS provided libs from the following, just for the record)

[seabld@cb-seamonkey-linux-01 bin]$ readelf -d libxpcom.so | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libxul.so]
 0x00000001 (NEEDED)                     Shared library: [libplds4.so]
 0x00000001 (NEEDED)                     Shared library: [libplc4.so]
 0x00000001 (NEEDED)                     Shared library: [libnspr4.so]
 0x00000001 (NEEDED)                     Shared library: [libmozalloc.so]
[seabld@cb-seamonkey-linux-01 bin]$ readelf -d libxul.so | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libsmime3.so]
 0x00000001 (NEEDED)                     Shared library: [libssl3.so]
 0x00000001 (NEEDED)                     Shared library: [libnss3.so]
 0x00000001 (NEEDED)                     Shared library: [libnssutil3.so]
 0x00000001 (NEEDED)                     Shared library: [libldap60.so]  <!--X
 0x00000001 (NEEDED)                     Shared library: [libprldap60.so] <!--X
 0x00000001 (NEEDED)                     Shared library: [libldif60.so] <!--X
 0x00000001 (NEEDED)                     Shared library: [libmozsqlite3.so]
 0x00000001 (NEEDED)                     Shared library: [libplds4.so]
 0x00000001 (NEEDED)                     Shared library: [libplc4.so]
 0x00000001 (NEEDED)                     Shared library: [libnspr4.so]
 0x00000001 (NEEDED)                     Shared library: [libmozalloc.so]

(all others have merely duplicates of the above or sys libraries)
Comment 11 Justin Wood (:Callek) 2012-02-12 05:46:51 PST
Created attachment 596465 [details] [diff] [review]
[m-c] v1 - include stuff from app

This will be used in v2 of the comm-central patch.
Comment 12 Justin Wood (:Callek) 2012-02-12 05:48:37 PST
Created attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]
Comment 13 Justin Wood (:Callek) 2012-02-12 05:49:09 PST
Created attachment 596467 [details] [diff] [review]
[m-c] v1 - include stuff from app
Comment 14 Justin Wood (:Callek) 2012-02-12 05:51:04 PST
Comment on attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]

I'll accept a review from Mark or Mike here. The key review point is as it relates to dependentlibs.

The rest of this patch was reviewed by KaiRo, but happy to have comments from either of you if you catch anything wrong.

f? from Karsten for basic functionality testing on mac. and f? from ewong for extra-sanity-checking functionality testing on linux. [I won't block landing on the feedbacks]
Comment 15 Justin Wood (:Callek) 2012-02-12 06:02:40 PST
Created attachment 596469 [details] [diff] [review]
[m-c] v1.1 - include stuff from app
[Checked in: Comment 19]

reattached a corrected patch per glandium.
Comment 16 Mark Banner (:standard8) (afk until 26th July) 2012-02-12 08:42:12 PST
Ooh, I like the solution. Want to test it out before I give r+ but the idea looks great.
Comment 17 Karsten Düsterloh 2012-02-12 15:17:55 PST
Comment on attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]

Mac looks okay (rebuild/clobber).
Comment 18 Mike Hommey [:glandium] 2012-02-13 00:09:08 PST
Comment on attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]

Review of attachment 596466 [details] [diff] [review]:
-----------------------------------------------------------------

Whitespace excluded, nsSuiteApp.cpp just becomes the same content as nsBrowserApp.cpp, so it's obviously good :) The corresponding changes to Makefile.in are ok, but I do wonder where some of the stuff that's not in browser/app/Makefile.in come from. The rest looks good.

::: suite/extradependlibs.mk
@@ +6,5 @@
> +# Presume all mozilla/autoconf.mk is included and that it does not have
> +# Direct access to c-c specific vars not present there.
> +
> +# We don't have access to MOZ_LDAP_XPCOM here, so cheat.
> +ifneq (,$(findstring ldap,$(MOZ_APP_COMPONENT_LIBS)))

$(filter mozldap,$(MOZ_APP_COMPONENT_LIBS)) would be better imho
Comment 19 Marco Bonardo [::mak] 2012-02-14 02:20:38 PST
https://hg.mozilla.org/mozilla-central/rev/0d95f0ca65a0
Comment 20 Jens Hatlak (:InvisibleSmiley) 2012-02-14 10:12:51 PST
This landed:
http://hg.mozilla.org/comm-central/rev/9eddb2d0b431

Hmm... Just built myself from current trunk in my Linux VM. Then:

jens@holodoc:~/usr/local/seamonkey-central > ./seamonkey
XPCOMGlueLoad error for file /home/jens/usr/local/seamonkey-central/libxpcom.so:
libxul.so: cannot open shared object file: No such file or directory
Couldn't load XPCOM.

Same with seamonkey-bin, which is binary-equal to seamonkey (unlike firefox vs firefox-bin for the latest Nightly).

Is it just me?
Comment 21 Justin Wood (:Callek) 2012-02-14 11:48:04 PST
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #20)
> This landed:
> http://hg.mozilla.org/comm-central/rev/9eddb2d0b431
> 
> Hmm... Just built myself from current trunk in my Linux VM. Then:
> 
> jens@holodoc:~/usr/local/seamonkey-central > ./seamonkey
> XPCOMGlueLoad error for file
> /home/jens/usr/local/seamonkey-central/libxpcom.so:
> libxul.so: cannot open shared object file: No such file or directory
> Couldn't load XPCOM.
> 
> Same with seamonkey-bin, which is binary-equal to seamonkey (unlike firefox
> vs firefox-bin for the latest Nightly).
> 
> Is it just me?

No not just you, Seems Standard8 noticed this as well, he may have a fix, but if not I'll back this out shortly.
Comment 22 Mark Banner (:standard8) (afk until 26th July) 2012-02-14 11:50:58 PST
Created attachment 597130 [details] [diff] [review]
Fix ordering for extra app libs in dependentlibs.list
[Checked in: Comment 23]

Unfortunately the original patch didn't realise that dependentlibs.list was ordered and therefore would still break on Linux. This patch fixes the ordering by including the extra libs for the app first. It works for at least both SM & TB, so hopefully we'll be ok.
Comment 23 Justin Wood (:Callek) 2012-02-14 12:13:52 PST
Thanks mark and kyle, pushed the fixed:

http://hg.mozilla.org/mozilla-central/rev/b3a7561624f9
Comment 24 Serge Gautherie (:sgautherie) 2012-02-14 20:36:29 PST
I didn't look too precisely, but tbpl hasn't reported any test run since "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one.

I assume this is not a result of bug 722187, which was later backed out, as it is js only.
Then this bug seems a likely culprit:
some builders may just have not (re)run since then,
but (some) Windows test builders seem to have a stuck SeaMonkey, which probably need a human maintenance or something!
	
Can you check these(/all) test builders?
Comment 25 Serge Gautherie (:sgautherie) 2012-02-14 20:41:01 PST
(In reply to Serge Gautherie (:sgautherie) from comment #24)

> I didn't look too precisely, but tbpl hasn't reported any test run since
> "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one.

Oh, that must be another regression to be fixed by bug 727258, hopefully.

> Can you check these(/all) test builders?

Actually, only 'cb-seamonkey-win32-02' seems affected, afaict from current
http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey
Comment 26 Serge Gautherie (:sgautherie) 2012-02-15 09:47:40 PST
(In reply to Serge Gautherie (:sgautherie) from comment #25)
> Actually, only 'cb-seamonkey-win32-02' seems affected, afaict from current
> http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey

Ftr, 'cb-seamonkey-win32-01' had been affected too (earlier), but seems fixed now.
And 'cb-seamonkey-win32-02' seems to (have) been fixed too (now).
Comment 27 Serge Gautherie (:sgautherie) 2012-02-15 17:27:58 PST
(In reply to Serge Gautherie (:sgautherie) from bug 722262 comment #25)
> (In reply to Serge Gautherie (:sgautherie) from bug 722262 comment #24)
> 
> > I didn't look too precisely, but tbpl hasn't reported any test run since
> > "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one.
> 
> Oh, that must be another regression to be fixed by bug 727258, hopefully.

I filed bug 722675.
Comment 28 Philip Chee 2012-02-16 10:51:50 PST
> I filed bug 722675.
I think you meant Bug 727675. SeaMonkey doesn't run on Android :P
Comment 29 Mark Banner (:standard8) (afk until 26th July) 2012-02-21 05:52:13 PST
Comment on attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]

Clearing obsolete requests.
Comment 30 Serge Gautherie (:sgautherie) 2012-02-24 05:08:54 PST
Comment on attachment 596466 [details] [diff] [review]
[c-c] v2
[Checked in: Comment 20]

Review of attachment 596466 [details] [diff] [review]:
-----------------------------------------------------------------

::: suite/installer/package-manifest.in
@@ -111,1 @@
>  @BINPATH@/@MOZ_APP_NAME@

Ftr, this part was from bug 549246.

Note You need to log in before you can comment on or make changes to this bug.