It would be great if you download a SpiderMonkey tarball, untar it, and then do: ./configure && make && make install && make clean and have it Just Work. I have a project that depends on the JS engine and I'd rather not ask people to go through a lot of trouble to install the libraries/headers manually.

Assigning this one to rginda, expert in these matters, for consideration. Please reassign as necessary; thanks. cc'ing Roger, Patrick, Kenton -

I'm not going to have the time to do this, especially when the only compelling reason is, "it would be great." (Not that I disagree.) If all you're *really* worried about is that your clients have to install the headers and library by hand, then you can submit a patch to add an install: target to Makefile.ref (we don't need autoconf to do that.) Reassigning to nobody@mozilla.org, adding helpwanted keyword.

assigning to me

I have very little experience with GNU make, autoconf, or automake, unfortunately. That patch is a major hack, but it's good enough for me. ;) jcore? Wanna do the autoconf stuff? ;)

The only headers that *should* need be installed are jsapi.h, jspubtd.h, jstypes.h, jscompat.h, and jslong.h.

jsdbgapi.h, too? jsxdrapi.h was meant to be publicly consumable, but I don't think it really is.

Of course! How could I forget jsdbgapi.h.

jscpucfg.h -- are we there yet? /be

I hope so. Here comes patch that include bug 107002's change, and defaults to JS_READLINE on Linux. The library used by JS_EDITLINE is not on redhat 7.1 systems (and maybe others too) by default.

Extract the tarball into js/src/. I couldn't put the autoconf/automake files in the js directory because the Mozilla build system uses the filename Makefile.in already. Therefore, js/src/autoconf/ has a bootstrap script which creates symlinks to the original source files. After running that, ./autogen.sh && ./configure && make should build the js interactive interpreter and library. I haven't tried |make dist| or |make install| yet.

Okay, make dist (and building from the resulting tree) and make install work properly. For the record, the other two important headers are jsosdep.h and jsotypes.h. I'll try building in Cygwin later tonight.

Can't get it to build in Cygwin... Not too important, I guess. Something about how it can't find some suitable types: gcc -DXP_PC=1 -DPACKAGE=\"SpiderMonkey\" -DVERSION=\"1.5.0\" -DHAVE_DLFCN_H=1 -D STDC_HEADERS=1 -DHAVE_SYS_WAIT_H=1 -DHAVE_LIMITS_H=1 -DHAVE_STRINGS_H=1 -DHAVE_S YS_TIME_H=1 -DHAVE_UNISTD_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_ST DLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_UNISTD_H= 1 -DTIME_WITH_SYS_TIME=1 -DHAVE_TZNAME=1 -DHAVE_STRFTIME=1 -DHAVE_FTIME=1 -DHAVE _GETCWD=1 -DHAVE_GETTIMEOFDAY=1 -DHAVE_MKTIME=1 -DHAVE_STRDUP=1 -DHAVE_STRERROR= 1 -I. -I. -I fdlibm -g -O2 -c jsapi.c -Wp,-MD,.deps/jsapi.TPlo -DDLL_EXPORT -DP IC -o .libs/jsapi.lo In file included from jsapi.c:43: jstypes.h:254: #error No suitable type for JSInt8/JSUint8 jstypes.h:267: #error No suitable type for JSInt16/JSUint16 jstypes.h:287: #error No suitable type for JSInt32/JSUint32 jstypes.h:338: #error 'sizeof(int)' not sufficient for platform use I'm completely clueless. Any ideas?

Anyone care to review patch 56443?

ping?

I am not the right guy to review this stuff; cc'ing cls@seawood.org -- Chris, can you have a look, maybe try this out? /be

I have increased my autoconf/automake-fu. This version has correct dependencies.

Review? Please? Pretty please? It's a pain to maintain my own fork of automake spidermonkey.

cls, can you review? pschwartau, can you try this stuff out and comment? Thanks, /be

Chad: could you give me the steps I need to follow to try this out? I've unzipped the autoconf directory into a "JS" directory: JS (dir) - autoconf (dir) bootstrap configure.in Makefile.am - fdlibm (dir) Makefile.am - config (dir) config.guess etc. Now what steps to I follow? Do I have to download any JS source from CVS, or are your scripts going to do that automatically? If I do have to download, exactly what directories should I pull, and where should I save them in relation to the structure above?

You should extract it into js/src/ so that you have js/src/autoconf/ cd into that directory and run ./bootstrap. It will build a local source tree of symlinks to the real sources and will create the autoconf/automake build environment. You can then run ./configure && make && make install or whatever you want.

Something is going wrong for me when I try the above steps. ------------------ On WinNT, using Cygwin 1.1.8: ------------------ [//d/JS_97954/src/autoconf] ./bootstrap + aclocal + libtoolize --force --copy libtoolize: not found [//d/JS_97954/src/autoconf] $ ./configure bash: ./configure: No such file or directory ------------------ On RedHat Linux 7.2: ------------------ [/d/JS_97954/src/autoconf] ./bootstrap + aclocal + libtoolize --force --copy You should update your `aclocal.m4' by running aclocal. Putting files in AC_CONFIG_AUX_DIR, `config'. + automake --foreign --add-missing --copy + autoconf [/d/JS_97954/src/autoconf] ./configure configure: error: can not find sources in . or ..

Your Cygwin has old versions of automake, autoconf, and libtool. (Cygwin also has lots of other troubles with them; I usually tell the installer to not install the autotools and install them by hand) Try again after installing them by hand: http://www.gnu.org/software/autoconf/ http://www.gnu.org/software/automake/ http://www.gnu.org/software/libtool/ In Red Hat: Are you sure the bootstrap script created the symlinks? The bootstrap script symlinks all of the JS and fdlibm sources into the autoconf directory to trick the autotools and gcc into thinking they are in the real source directory. It then calls aclocal, libtoolize, automake, and autoconf to generate the configure script.

Sorry, Brendan, I'm not the proper person to review automake changes. I stopped following automake years ago. The entire automake/autoconf/libtool triangle was too messy when just autoconf would do. I downloaded the tarball and tested it on RH7.2 which still has the old versions of autotools installed. It compiled fine. However, I think the real test would be attempting to use this setup with a non-GNU compiler, which is where automake/libtool has been known to have problems. Makefile.am & configure.in also seem to be missing the various platform specific defines from the other js/src/config/<platform>.mk files. Nit: 'make distclean' didn't remove the symlinks created by bootstrap.

I created this because I wrote a game engine which depends on SpiderMonkey, and people were complaining about how difficult it was to install the headers and libraries (it wasn't automatic). I figured that if they could just download a standard automake dist of the sources, the install would be as simple as ./configure && make && make install... and they could later do a make uninstall if needed. Eventually I gave up on this getting checked in and actually used (i.e. make dist tarballs on ftp.mozilla.org), so I just made my own make dist and told people to download that. The problem there is that Sphere users don't get new updates to SpiderMonkey. Right now, it builds on Linux and IRIX, and that's good enough for me. If people on other platforms want to use the automake build, they can just use Makefile.ref or file/fix new bugs. Can we just get 'make dist' tarballs on ftp.mozilla.org?

I just want to throw my $0.02 in and say that figuring out how to install a standalone SpiderMonkey is beyond me. The documentation is not in agreement. The src page tells me to use Makefile.ref for standalone builds; the README says, to not use it for standalone builds. Further, even if I fly blind and use Makefile.ref, it builds fine, but I can't figure out how I'm supposed to install it. Just to say that this could use some mo' better documentation, maybe from someone who's actually succeeded?

Um, the documentation says don't use Makefile or makefile.win, while it doesn't use <code> to indicate those are filenames it does pretty clearly indicate that you want Makefile.ref for standalone.

> ------------------ On RedHat Linux 7.2: -------------- > [/d/JS_97954/src/autoconf] ./bootstrap > + aclocal > + libtoolize --force --copy > You should update your `aclocal.m4' by running aclocal. > Putting files in AC_CONFIG_AUX_DIR, `config'. > + automake --foreign --add-missing --copy > + autoconf OT, but from libtoolize's error message, aclocal and libtoolize should be called in reverse order. (Should probably make that "aclocal -I config" too, or set "ACLOCAL_AMFLAGS = -I config" in a friendly Makefile.am file)

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

(In reply to comment #27) > Nit: 'make distclean' didn't remove the symlinks created by bootstrap. it's not supposed to, the stuff created by bootstrap is meant to be distributed (so that people who compile need not have automake installed). thus, distclean is not supposed to delete them.

Taking this over

related to 341912

Fixed in bug 361268 as per http://www.newn.cam.ac.uk/prlw/jsref-1.80.tar.gz which was created with make dist. If someone would like to help test/comment/fold in changes (odd printing problem in sparc-sun-solaris2.9 - I couldn't compile with Makefile.ref, so seeing the problem at all feels like an improvement!)

Patrick: The autoconf setup you have provided in that bug is great, but I am running into a few issues with it, and maybe you can help me with those. a) can you provide those changes as a patch to this bug, instead of as a tarball in the other bug? b) can you make your script/macros/etc., compatible with autoconf 2.13 (this is the autoconf tool required by the rest of the mozilla tree)? c) can you please remove any dependency on automake? d) Can you add license-headers to the new files, matching the licenses of the other files in js/src? e) XXX comments: 1. "narcissus" is self-hosting JavaScript (javascript in javascript as your XXX comment suggests). 2. Thin locks are, as you guessed, meaningless without JS_THREADSAFE 3. File object should -not- be on by default. 4. I believe we need the stack-growth direction to check for unbounded recursion in some routines. (note to self: make sure bug 380998 changes make it in here) Thanks!

a) I could, but for the moment, I think it is easier for folks to test by downloading the tar file, uncompress/untar, configure, make, make check, than by starting to patch.. b) No, that would be a regression. Autoconf 2.13 was released January 1999. It won't even recognise at least one of my systems. Also, if we find a problem in autoconf, we aren't going to get it fixed in 2.13! (I'm pleased to see there was a small exchange on Bug 104642) I agree that libnspr would be a good next target... c) Again, I don't really see the point - and note that you just changed the name of the bug to remove automake ;-) If there are problems with automake, I'm sure we would like to hear about them on the automake lists... d) I thought I had cut'n'pasted into the real the files (configure.ac/Makefile.am) - do you mean the autogenerated files? Have I missed some? e) I'll adjust accordingly. BTW my reason for asking about the need to test for stack-growth direction is that it is actually very hard to get a believable result, as can be seen from http://lists.gnu.org/archive/html/autoconf/2006-12/msg00050.html The method I popped into configure.ac was an attempt at preventing inlining, and was better than the jscpucfg.c I was working from - I see this was improved on 21st May - I must take another look!

(In reply to comment #38) > a) I could, but for the moment, I think it is easier for folks to test by > downloading the tar file, uncompress/untar, configure, make, make check, than > by starting to patch.. It may be easier to test this way but it is not easier to review, and it is impossible to land and difficult for anyone but you to maintain in an up-to-date fashion. Providing a patch here will make it must easier to deal with bit-rot in the future until we're able to land it. Furthermore, I can't land anything that isn't in a patch format. > b) No, that would be a regression. Autoconf 2.13 was released January 1999. It > won't even recognise at least one of my systems. Also, if we find a problem in > autoconf, we aren't going to get it fixed in 2.13! (I'm pleased to see there > was a small exchange on Bug 104642) I agree that libnspr would be a good next > target... It wouldn't be a regression for us, since right now we don't have any autoconf functionality in js/src (by itself, at least), at all! I realize 2.13 is old, can you at least push this back to 2.59 so that I has a chance of working on my Mac and/or the build tinderboxes? I can try to do the 2.59 => 2.13 work myself as a follow-up project. > c) Again, I don't really see the point - and note that you just changed the > name of the bug to remove automake ;-) If there are problems with automake, > I'm sure we would like to hear about them on the automake lists... The point is that the current build system does not require automake, and I do not intend to apply a patch which adds such a requirement. We need to remain build-compatible with the rest of the tree. If you don't want to do this, please give me some hints as to how. We won't be able to land anything until it is agreeably with the rest of the tree. > d) I thought I had cut'n'pasted into the real the files > (configure.ac/Makefile.am) - do you mean the autogenerated files? Have I > missed some? Yeah, perhaps I was looking at autogenerated stuff... need to look more (this is just the sort of thing that would be easier to review in a patch file, as opposed to a "make dist"ed tarball. > e) I'll adjust accordingly. BTW my reason for asking about the need to test > for stack-growth direction is that it is actually very hard to get a > believable result, as can be seen from > http://lists.gnu.org/archive/html/autoconf/2006-12/msg00050.html > The method I popped into configure.ac was an attempt at preventing inlining, > and was better than the jscpucfg.c I was working from - I see this was > improved on 21st May - I must take another look! Yeah, this issue is probably worth another bug. We must be getting lucky so far...

Brian Crowder asked me to submit my patch to this bug. I'll quote myself: --- I have done some preliminary work, so I've only tested on a single Linux machine. So there's still work to be done, but I would like a developer to review my patch, and tell me if it's useful enough for me to continue my effords. Requirements: - Autoconf - Automake - Libtool - C compiler, make, etc. Instructions: - Apply the patch on the mozilla/js directory from CVS (I had network issues using mercurial): "patch -p0 < autotools.patch" - Run "autoreconf -i" in the mozilla/js directory, this creates a few scripts and utilities - Run "./configure" to build and test To do: - Create more autoconf checks to detect all options automatically (check the nspr library for threadsafety and if so, eneble threadsafety by default, etc.) --- I would like to emphasize that the Mozilla build system is probably good for building Mozilla products, but I had quite some trouble building on a PowerPC machine. This is no fix for that, I'll try and fix that later. The key point is that this patch makes the spidermonkey library usable for developers NOT familiar with the Mozilla build system. Especially since I think it's flawed. In the current Makefile.ref file, config.mk is referenced, and that file references a platform specific file. That's not really great. Usable ./configure options is what developers want. "--enable-shell" for instance, or "--with-readline", or "--enable-threadsafe", or automatic detection of threadsafety by examining the nspr library. Sure, Mozilla could use autoconf 2.13 forever, but really, this really shouldn't stop development. The autoconf and automake requirement only exists on the maintainers' machines. A proper ./configure && make distcheck produces a single tarball that's much more familiar to e.g. GNU/Linux developers, and it doesn't contain files that aren't important to the platform, e.i. no Visual Studio files on Linux or BSD machines. My patch only adds files, no files need to be modified. Only the Makefile.in is overwritten by automake, but that Makefile.in is not suitable for standalone compilation anyway... This post will probably look like a commercial by now, but Spidermonkey would be much more attractive to BSD or GNU/Linux developers if it's provided with a familiar build system.

Edwin: Is there at least a way to prevent any files being overwritten, so that this build process doesn't -conflict- in any way with the build process for the main mozilla tree? If we could make sure Makefile.in isn't overwritten, then it would be a lot easier to accept this patch without fearing that someone might use the spidermonkey autoconf build and break their mozilla build accidentally.

By e.g. calling them Makefile-standalone.am Makefile-standalone.in etc

Unfortunately I don't know any automake/autoconf feature to do so. I understand your fears, but the Makefile.in need not be added to CVS. An automated tool could check out the latest version from CVS, apply "autoreconf -i && ./configure && make distcheck", and distribute the resulting tarball. People grabbing the tarball won't need the Mozilla Makefile.in, nor would they need the GNU autotools. Just make, libtool and a C compiler should be enough.

This patch is very preliminary. It regresses a lot of platforms, I'm guessing, including at least Mac OS X. Edwin is working on it.

I can get autoconf and automake work with e.g. Makefile.vi .am to generate a Makefile.ref.in, so that ./configure generates a Makefile.ref, but I guess some fairly ugly hacking in the rules is required, somything like the LOOP_OVER_DIRS from rules.mk For now I'll just focus on getting the autoconf rules to detect what config/*.mk try to define. Any CFLAGS, ASFLAGS etc. that are not required, I skip for now, since people who want to compile the software may want to provide them themselves. The XP_UNIX etc defines are the main concern. So I'll try to post a better patch later this weekend.

The longer I look at the code, the more I believe some things are fundamentally wrong. I suggest removing XP_UNIX and stuff like that altogether, and modify the config/*.mk files, to define stuff like HAVE_UNISTD_H and HAVE_SYS_TIME_H. Now I don't know exactly how much impact a change like this would have on the Mozilla build, but a change like this is done fairly easily for the supported operating systems. Note that the patch is ONLY for Linux, but if you take a look at te OS_CFLAGS definition in config/Linux_All.mk you should be able to adjust for other unix-like operating systems just as easily. I'm not entirely sure about some ifdef's I've changed... This could be a first step towards a better build system, since currently e.g. FreeBSD isn't supported. This patch should NOT break the mozilla build, or at least that is my intention for now. The HAVE_* definitions should be generated by the configure script in a future step, I have a totally working GNU build system for Linux and FreeBSD now, for i386 and PPC platform (autoconf/automake) and I like that. Could some users of UNIX-like systems (Linux, Mac OS X, etc) take a look and see if they can get this working by adjusting their config/*.mk files?

I agree we should use specific feature-checking macros. This is a Good Thing generally. Please file a new bug for this small, incremental change; I think the shortcomings you mention can all be addressed. Now: where are we going with the SpiderMonkey build?? bsmedberg and I agree that we should just have one build system, not two. Maintaining both sucks. I think we all agree we want autoconf and we want to junk config/*.mk. bsmedberg thinks it'd be easier to use the Mozilla build system always, even for standalone SpiderMonkey. Mozilla already uses autoconf and already generates a sufficient mozilla-config.h header. We could add rules to Makefile.in for the js shell and for building without JS_THREADSAFE. The main missing piece is a way to build a SpiderMonkey source distribution that includes the necessary parts of the Mozilla build system. I sort of disagree with bsmedberg. It sounds to me like the source distribution would end up being a halfheartedly-maintained stepchild. I think it would be better to put mozilla/js in its own repository, with its own configure scripts, for Mozilla 2. (This, of course, won't happen unless someone volunteers the work.) Note that it will otherwise become impossible to check out just SpiderMonkey sources in Moz2-- embedders will have to "hg clone" the whole mozilla-central tree and run client.py. This is a side effect of the switch from CVS to Mercurial. But either way seems better than the status quo. Comments welcome.

I for one find it extremely convenient to have a standalone JS checkout, which I keep in ~/moz/js, next to my Mozilla tree checkouts. If I'm working on a bug with limited impact on Mozilla, where I can be reasonably certain that my changes, once completed, will not break anything in Mozilla, it's handy to do development in a small directory structure where I don't have to check out lots of code to edit the very little of it that JS uses. Even for patches which might affect Mozilla, like, for example, the one in bug 376957 when I was working on it, it's still more convenient not to have to have a whole separate Mozilla tree just to work on SpiderMonkey, and I can copy the patch over to a Mozilla tree to check that nothing there breaks with it applied. I'd vote for a separate repository. (This is all from a centralized VCS approach, not distributed -- I still have yet to take the time to play with distributed systems, so this might be completely the wrong way to handle things. It's how I work right now, tho, and I think it works reasonably well for my purposes.)

I firmly believe we need something that is compatible with the mozilla build system, but works 100% standalone (ie., with just src/js downloaded)

This patch modifies the standalone build files and some source files. It was my intention not to influence the Mozilla build, so the #ifdef XP_UNIX checks are replaced by #if defined(HAVE_UNISTD_H) || defined(XP_UNIX) style checks. Makefile.ref is now automatically generated from Makefile.ref.in, which is essentially the same, but with a AC_SUBST for @DEFS@ which means Operating System features are supposed to be detected automatically. Note that the patch is for Linux_All only, but users of e.g. Mac OS X should be capable of modifying their config/*.mk file easily by observing the config/Linux_All.mk example. My patch didn't break the standalone build, and didn't introduce new bugs (I ran the testsuite), but is untested in a full build. It's still preliminary work. Instructions: - apply the patch, run autoconf and the configure script patch -Np0 -i autoconf.patch autoconf ./configure - build spidermonkey as usual: make -f Makefile.ref

For the sake of touching less code, can we just add a -DXP_UNIX to the CFLAGS if HAVE_UNISTD_H (and no other XP_ is defined? -- Does Win32 have unistd.h?)

Actually the goal should be to get rid of any platform-related defines, and to solely use capability-related defines. As a matter of fact, it's not very different from browser-detection using JS. That's concidered "not done" nowadays. Next step would be to move build options to configure.in and modify Makefile.ref.in to be able to pass arguments to ./configure, like ./configure --enable-shell --with-readline --disable-debug

Yes, I suppose so... This works on linux, then? I'll test Mac and Win32 in the next couple of days and try to update accordingly.

I have modified my previous patch a bit. This patch modifies only *.h and *.c files. It's designed to leave both the Mozilla and the standalone build in tact. None of the modifications should break anything. Please review my patch, and concider adding it to the HEAD branch. I have tried to add support for autoconf, and have Makefile.ref generated by it. It works quite okay. I have also succesfully created an autoconf, automake and libtool build system. That even works better. For now, it's pretty okay for me to use, but it needs more work to add liveconnect, editline and fdlibm support. Linking against an already installed readline, libedit2 or editline library works flawlessly. Both the autoconf and de autoconf/automake/libtool variants require the modifications that are done by this patch. I would appreciate a review.

I've made two errors in my previous patch. jslock.c and jsfile.c included config.h AFTER the check for JS_THREADSAFE and JS_HAS_FILE_OBJECT. I have now corrected that.

This is a patch for spidermonkey to use the GNU build system. Autoconf, automake and libtool. Usage: $ cd mozilla/js $ patch -Np0 -i js-autotools.patch $ autoreconf -i $ ./configure --enable-shell --with-readline $ make $ ./src/js Tested platforms: Debian 4.0 i386 Debian 4.0 amd64 Mac OS X ppc Debian 4.0 ppc FreeBSD 6.2 i386 OpenBSD i386 Cygwin/GCC i386 Please note that it's possible to do: $ make dist This drops a .tar.gz, a .tar.bz2 and a .zip file with all required files. Anyone who uses such a tarball will NOT need autoconf and automake, just libtool is enough.

Fixed something in jsxdrapi.c that I broke.

An update on what's up with this bug: Ben Smedberg persuaded me that we have an opportunity to kill two birds with one stone here: - We want to be able to build SpiderMonkey out-of-tree. - We want both out-of-tree and in-tree builds of SpiderMonkey to use the same build machinery. Ben recommended that we emulate what we have now for NSPR for SpiderMonkey. Thus: - The 'js' tree will have its own configure script. The top-level configure.in will call it, as it does nsprpub's now. - We will not use automake or libtool, or an autoconf newer than 2.13, pace Edwin. If those are valuable changes, they can be a separate patch. - A spidermonkey install will provide a js-config script, in the style of nspr/config/nspr-config.in. - js/src/config will probably change quite a bit, acquiring much of the same kinds of stuff that nsprpub/config has now. I'm expecting this will be using much of Edwin's patch. Certainly most of the C changes are the same, except that since we're switching to a single build system, XP_UNIX (etc.) should be able to go away completely.

Hmm. If js/ is to build on its own, that means duplicating the big honkin' case statement in the root configure.in that sets per-platform parameters like how to build shared libraries. nsprpub/configure.in already seems to have its own copy of this; introducing a third isn't appealing. But if js is to be compiled on its own, it obviously needs its own copy. What can we do to reduce the maintenance burden here, and automatically catch discrepancies? What would folks think of moving the top-level configure.in's copy into a file in build/autoconf that we incorporate into configure.in with 'include'? Then js could have its own copy in js/build/autoconf, making mechanical comparison easy. If we wanted, at some point in an in-tree build we could actually 'cmp' the two and die if they're not the same. Then every in-tree build would catch discrepancies.

Jim: Sounds like a good strategy to me.

Quick status report on this: I have stand-alone builds working. I'm putting out my work in progress via a public Mercurial repository. Full instructions are on the wiki at http://developer.mozilla.org/en/docs/Building_only_ActionMonkey. I haven't yet gotten in-tree builds working. It's going to be a bit of a challenge to make this all work the way it should, as it seems the ActionMonkey header files we need to install need MMgc.h from Tamarin; Dehydra, for example, needs jsobj.h. However, MMgc.h has not at all been written to be namespace-clean, depends on preprocessor symbols from the Tamarin build system, and so on. We can't install that. So I'm going to set this aside for a second and work on making the ActionMonkey and Tamarin header files installable.

Jim, the MMGc build produces MMgc-config.h which should be sufficient for installation, I think. But it might make more sense to do this for mozilla-central first and then forward-port to actionmonkey, rather than the other way round?

D'oh. Yes, obviously mozilla-central is better. MMgc.h also wants winbuild.h, macbuild.h, and so on. It's got #ifs that test things like SCRIPT_DEBUGGER, GCHEAP_LOCK, and so on; if client code #defines those things for its own purposes, they'll see a different MMgc.h than libMMgc.so did when it was built. I don't see any reason, though, that the portion of the GC interface needed for public headers should have those kinds of dependencies. It should be possible to choose a subset of MMgc.h sufficient for jsobj's needs that doesn't also depend on the internals of Tamarin's configuration.

(In reply to comment #64) > MMgc.h also wants winbuild.h, macbuild.h, and so on. It's got #ifs that test > things like SCRIPT_DEBUGGER, GCHEAP_LOCK, and so on; if client code #defines > those things for its own purposes, they'll see a different MMgc.h than > libMMgc.so did when it was built. True, but the short-term answer is going to be "don't do that." Those symbols should be defined if and only if MMgc-config.h or foobuild.h defines them. It's robust against everything except a Mozilla header file #defining something like SCRIPT_DEBUGGER, which seems unlikely. A quick mxr search can confirm we don't do this (currently). Need a separate bug to clean up the header files, renaming GCHEAP_LOCK -> MMGC_HEAP_LOCK etc. We can fix that after this. (Adobe might have compatibility headaches that would delay it, and I would hate to delay this for that.) Splitting gigantic bugs that never get fixed into smaller units of work is good!

This was discussed more on #mmgc and #jsapi. The outcome: - For uses of header files installed outside the Mozilla tree, it's moot: jsapi.h is the only suppored public header, and so should be the only thing installed. (We sorted out why Dehydra wanted jsobj.h, and found a better solution.) - For uses of header files within the Mozilla tree, we should eventually clean up that header.

Created bug 422265.

Status report: - Both in-tree and out-of-tree builds seem to work now. - SpiderMonkey can be built with NSPR if you don't need multi-threaded support. - The build system checks that the files in 'js/config' and 'js/build' duplicated from the corresponding top-level directories are actually in sync with their originals. - The wiki instructions have been updated and simplified: http://developer.mozilla.org/en/docs/Building_only_SpiderMonkey That has the latest changeset ID as well. Now I can finally start breaking this up into patches for review.

These are all minor prep. Three big patches coming up.

This is the main patch.

That's it.

Updated for Windows.

Updated for changeset 5cb5ebda3312, jorendorff's Apr 2 merge to Mozilla Central. Windows fixes.

Comment on attachment 313227 [details] [diff] [review] Check at compile time that SpiderMonkey's build files are in sync with their originals Note that, once this patch is applied, if you're not testing the patches against exactly the sources I generated them against, the test this patch adds will notice differences between the top-level config/build files and js's copies. If this happens, just copy the originals into the js tree and try again. If this patch lands, then nobody will be able to build unless the top-level and js copies are in sync, so committed changesets should always pass this test.

Comment on attachment 313119 [details] [diff] [review] Compute LIBXUL_DIST in configure.in, not in autoconf.mk.in. I'm not sure why you need to do this, and have comments: * general configure.in style uses test -n * MOZ_BUILD_ROOT is an absolute path. $(DIST) is in many/most cases a relative path. Why are you changing this? We prefer the relative path in most cases. * Please quote $LIBXUL_SDK etc

Comment on attachment 313226 [details] [diff] [review] Record configuration details in an installable header. I'd prefer js-config.h, to mirror xpcom-config.h and mozilla-config.h

Comment on attachment 313226 [details] [diff] [review] Record configuration details in an installable header. js-config.h is fine with me; there is already a jsconfig.h, and I thought it might be confusing.

Comment on attachment 313226 [details] [diff] [review] Record configuration details in an installable header. Agreement on IRC was to rename jsconfig.h to jsversion.h.

Comment on attachment 313226 [details] [diff] [review] Record configuration details in an installable header. Obsoleted by new js-config.h patch.

Updated for latest mozilla-central sources.

In case it helps, here's the current 'series' file. ignore-id.patch altoptions-effect-free.patch client-combine-configure-targets.patch compute-libxul-dist-in-configure.patch rules-mk-doc-fixes.patch configure-keep-NSPR-LIBS-CFLAGS.patch js-src-Makefile-useless-LDFLAGS.patch js-src-Makefile-use-EXTRA_LIBS.patch js-separate-configure.patch rename-jsversion-h.patch js-config-header.patch js-build-check-sync.patch

Revised based on bsmedberg's comments. Ted, Ben and I agreed that using MOZ_BUILD_ROOT here is okay, since it's known to be an absolute path, and we've already got absolute paths getting written into makefiles and such. It'd be nice to fix that eventually, and there are a bunch of ways to approach it; at the moment, here's the most intractable issue: JS_THREADSAFE SM needs to able to link against NSPR, so it needs CFLAGS and LIBS values for doing so. If the surrounding build system provides those as strings, then they either include absolute paths, or they're relative and can thus only be used from one directory. Since they're strings of compiler options, it's hard to adjust them as one passes them around from one directory to another.

Revised to treat 'js/src' as the root of the SpiderMonkey tree, not js.

Revise for js -> js/src changes in js-separate-configure.patch.

Update for js -> js/src change.

Revise for js -> js/src change.

Before we can make js/src the root of an independent SpiderMonkey tree, we need to move the existing config directory used by Makefile.ref out of the way.

Here's the current patch sequence: delete-fdlibm.patch ignore-id.patch altoptions-effect-free.patch client-combine-configure-targets.patch compute-libxul-dist-in-configure.patch rules-mk-doc-fixes.patch configure-keep-NSPR-LIBS-CFLAGS.patch js-src-Makefile-useless-LDFLAGS.patch js-src-Makefile-use-EXTRA_LIBS.patch js-remove-Makefile.ref.patch js-separate-configure.patch rename-jsversion-h.patch js-config-header.patch js-build-check-sync.patch

Fans of this work wonder when it will land in mozilla-central. Thanks, /be

Comment on attachment 317381 [details] [diff] [review] Give jsconfig.h a better name, and make room for the new js-config.h. This patch doesn't say that you `hg move`d jsconfig.h, but I'm assuming you did... r=me if that's the case. I'm also hoping you can some of these patches, such as this one, before you crash-land the main change.

Comment on attachment 317374 [details] [diff] [review] Allow SpiderMonkey to be built on its own, or as part of Mozilla. >+++ b/config/js/Makefile.in Tue Apr 22 22:02:20 2008 -0700 >+JS_LIBS = $$($(JS_CONFIG) --lib-filenames) Not sure where this came from, but you should be using $(shell) I think >--- a/js/src/Makefile.in Tue Apr 22 17:24:20 2008 -0700 >+NSDISTMODE = copy why? if you have some files which must be copied instead of symlinked, you should use SYSINSTALL instead of INSTALL >+# Compute the linker flags that programs linking against SpiderMonkey should >+# pass to get SpiderMonkey and its dependencies, beyond just the -L and -l >+# for the SpiderMonkey library itself. >+# - EXTRA_DSO_LDOPS includes the NSPR -L and -l flags. >+# - OS_LIBS includes libraries selected by the configure script. >+# - EXTRA_LIBS includes libraries selected by this Makefile. >+JS_CONFIG_LIBS=$(EXTRA_DSO_LDOPTS) $(OS_LIBS) $(EXTRA_LIBS) I'm confused about all this stuff for js-config: why can't we create js-config at configure time like any other makefile with substitutions? I'm having trouble reviewing the rest of this patch because it's hard to tell what files are bit-perfect copies and which are new code: could you use `hg cp` to avoid printing the new files, or separate the reviewable changes into a separate patch?

Comment on attachment 313116 [details] [diff] [review] Ignore idutils index files. I'm not sure what this patch is about, but let's move it to another bug, ok?

Comment on attachment 317391 [details] [diff] [review] Delete SpiderMonkey's custom build system for separate builds. Will the old build system stop working when you land this, or can this wait until after we've got people using it?

Comment on attachment 317385 [details] [diff] [review] Compare SpiderMonkey's copies of build files with originals at compile time. I'm a little hesitant about adding a whole bunch of file I/O to the build just to make sure that these two stay in check. How about running this in |make check| instead? That way you'd get pretty quick feedback, but it doesn't have to slow down everyone's build by a little bit.

(In reply to comment #103) > (From update of attachment 317381 [details] [diff] [review]) > This patch doesn't say that you `hg move`d jsconfig.h, but I'm assuming you > did... r=me if that's the case. Yes. > I'm also hoping you can some of these patches, such as this one, before you > crash-land the main change. I'll see if I can do that. Thanks for the review.

Comment on attachment 317385 [details] [diff] [review] Compare SpiderMonkey's copies of build files with originals at compile time. Minusing until you reply to my comments. :)

Comment on attachment 313116 [details] [diff] [review] Ignore idutils index files. This patch is now on separate bug: bug 439651.

(In reply to comment #103) > (From update of attachment 317381 [details] [diff] [review]) > This patch doesn't say that you `hg move`d jsconfig.h, but I'm assuming you > did... r=me if that's the case. > > I'm also hoping you can some of these patches, such as this one, before you > crash-land the main change. I'll put together an hg branch of the approved patches that someone can pull and merge to get them all at once.

(In reply to comment #106) > (From update of attachment 317391 [details] [diff] [review]) > Will the old build system stop working when you land this, or can this wait > until after we've got people using it? The issue here is that Makefile.ref uses a 'config' subdirectory to hold its platform-specific stuff, but a js/src-rooted SM tree wants 'config' to hold the duplicate of the top-level 'config' tree. Mixing the two trees together seems like chaos. I'll see if I can just rename Makefile.ref's 'config' subdir.

(In reply to comment #107) > (From update of attachment 317385 [details] [diff] [review]) > I'm a little hesitant about adding a whole bunch of file I/O to the build just > to make sure that these two stay in check. How about running this in |make > check| instead? That way you'd get pretty quick feedback, but it doesn't have > to slow down everyone's build by a little bit. If you think 'make check' will actually get run often enough, sure. The script takes .3 seconds on a MacBook Pro running Linux.

Well |make check| gets run on the unit test machines, so it will get caught one way or the other!

Wouldn't it be vastly preferable to catch such problems before they burn the tree? I'm happy to go with whatever you recommend, since you have the experience, but I'm surprised.

We should, that's what "make check" is for. burdening the main build with extra I/O seems like it's not worthwhile.

(In reply to comment #116) > We should, that's what "make check" is for. burdening the main build with extra > I/O seems like it's not worthwhile. Okay, I'll make this change.

What additional work is needed to get this bug home?

Comment on attachment 317391 [details] [diff] [review] Delete SpiderMonkey's custom build system for separate builds. Add r? bs

Jim, what's up? You don't call, you don't write.

Sorry. Here's the series as it stands: jimb.compute-libxul-dist-in-configure.patch jimb.configure-keep-NSPR-LIBS-CFLAGS.patch jimb.rename-jsversion-h.patch jimb.js-remove-Makefile.ref.patch # First unapproved patch, and patches that don't commute with it jimb.js-separate-configure.patch jimb.js-config-header.patch jimb.js-build-check-sync.patch jimb.js-shell.patch I'm refreshing and re-testing those first four; I think the first three can land on their own. Naturally, removing Makefile.ref and providing the alternative should land at the same time, so that people aren't left temporarily unable to build separately at all. The rest can land together or separately, although I assume "together" is preferred.

Patches up to jimb.rename-jsversion-h.patch are landed. Remaining: jimb.js-remove-Makefile.ref.patch # First unapproved patch, and patches that don't commute with it jimb.js-separate-configure.patch jimb.js-config-header.patch jimb.js-build-check-sync.patch # jimb.js-shell.patch

Just a status update: Main patch now uses 'hg cp' where it should, so it should be easier to review. With these patches against current trunk, 'make' works, but 'make -j4' doesn't. Parallel builds fail to get the system header wrappers in place before the libmozjs objects get built, causing a failure when we try to link libmozjs.so (claiming that libc things like 'free' and 'fopen' are undefined). Once I've got it working, I'll upload it here and r?.

I'm not sure which patch does most of the autoconf magic, but it looks like jsautocfg is still being generated at build time. The mechanism to do this is horribly broken for cross-compilation -- at the very least, could configure grow a --with-jscpucfg=foo.h flag that the top-level mozilla configure could pass down, so that a custom file can be created without needing to patch the build?

To get this particular bug finished, can we postpone discussion of the complete evil of jscpucfg to another bug? Yes it sucks! Jim, which system wrappers are you using? In mozilla this works by building config/ before anything else, but I figure in js/src you could just add the system-headers target to EXTRA_DEPS?

I agree, jsautocfg should be a separate bug; we really don't need anything else in there. I'd sorted out the system wrappers in one way; I'll look at EXTRA_DEPS and see if that's any cleaner. Thanks!

I believe EXTRA_DEPS won't work. The system wrappers are generated as part of the 'export' target in config/Makefile.in, which is listed in the 'base' tier and thus gets done at the start of the build. So there isn't any 'system-headers' target provided by a .mk file that js/src/Makefile includes.

Attaching the latest versions of the patches: jimb.js-remove-Makefile.ref.patch # First unapproved patch, and patches that don't commute with it jimb.js-separate-configure.patch jimb.js-config-header.patch jimb.js-build-check-sync.patch

This patch is essentially unchanged from the one Benjamin approved. It has been updated to delete the newer versions of the files, and to use a git-style diff.

Not requesting review for this until I get some 'try' results.

Same as patch Ben approved (317388), just refreshed, as a git diff.

Revised to run the checks on 'make check'.

Comment on attachment 316268 [details] [diff] [review] Compute LIBXUL_DIST in configure.in, not in autoconf.mk.in. Applied.

Comment on attachment 317381 [details] [diff] [review] Give jsconfig.h a better name, and make room for the new js-config.h. Landed.

Comment on attachment 313117 [details] [diff] [review] Remove side-effects from altoptions.m4. Landed.

Comment on attachment 313118 [details] [diff] [review] Minor cleanups to config.mk configure script regeneration. Landed.

Comment on attachment 313120 [details] [diff] [review] Trivial doc fixes for GNU make conditionals in rules.mk. Landed.

Comment on attachment 313122 [details] [diff] [review] js/src/Makefile.in: Remove useless assignment. Landed.

Comment on attachment 313123 [details] [diff] [review] Place extra libraries needed to link against libmozjs in a more selective variable. Landed.

Comment on attachment 313302 [details] [diff] [review] configure.in: Keep the final values of NSPR_LIBS and NSPR_CFLAGS. Landed.

Comment on attachment 317391 [details] [diff] [review] Delete SpiderMonkey's custom build system for separate builds. Updated patch posted.

Comment on attachment 337779 [details] [diff] [review] Bug 97954: Allow SpiderMonkey to be built on its own, or as part of Mozilla. I've got green try trees on all three tier-1 platforms, so I think this is ready for re-inspection by Ben.

(In reply to comment #125) > To get this particular bug finished, can we postpone discussion of the complete > evil of jscpucfg to another bug? Yes it sucks! In old bug 361268 I already did away with jscpucfg. (The autoconfigury in that distfile is probably worth reading, even though I say so myself...)

Updated for qparent 068e5618d7f4.

Updated for qparent 068e5618d7f4. Fixed 'make check' recursion into js/src.

Updated for qparent 068e5618d7f4.

Updated for qparent 068e5618d7f4.

All patches are refreshed to apply cleanly to Mozilla Central changeset 068e5618d7f4, as of this afternoon. These patches include deletions of files and of large stretches of text, which will conflict with any changes to the deleted text. So they go bad rather quickly, although the updates are easy.

Comment on attachment 338259 [details] [diff] [review] Bug 97954: Compare SpiderMonkey's copies of build files with originals at compile time. r=me, but can you make the failure output start with TEST-FAIL | ... ? That will be picked up by our tinderbox error parser.

Updated as requested in ted's r=me: failure message starts with TEST-FAIL | ...

Comment on attachment 338256 [details] [diff] [review] Bug 97954: Allow SpiderMonkey to be built on its own, or as part of Mozilla. diff --git a/config/js/Makefile.in b/config/js/Makefile.in +# Note that we actually need to use the shell's $(...) construct here, +# and not make's $(shell ...) function. Make expands all the variable +# references in a string of commands before it runs the commands, but +# we shouldn't try to run js-config until we've installed it. +JS_LIBS = $$($(JS_CONFIG) --lib-filenames) This makes me sad because we're recomputing it several times, plus it's hard to read. Can we instead do this with a dependent rule, or even better have the spidermonkey configure process generate this file, instead of the spidermonkey build? Then we could even call it once, from the Mozilla configure script. I find later that the spidermonkey configure *does* pre-create this... why can't we just use it? +# We clear NSDISTMODE when we run 'make install' to get symlinks from +# dist into the js build tree. That's not appropriate when building +# the SpiderMonkey tree stand-alone, so it's not the default. +export:: + $(MAKE) -C $(JS_OBJDIR) install NSDISTMODE= `make install` target should use SYSINSTALL, not INSTALL, and should not respect NSDISTMODE. I think if you want a symlink-install target, we should have a separate one. Note that mozilla rules.mk install target is out of date and should probably be removed; we implement `make install` by packaging up dist/bin the same way we would for a .tar.bz2 release, and then unpackaging it. diff --git a/js/src/Makefile.in b/js/src/Makefile.in +# This is the appropriate behavior for 'install' when we're building +# SpiderMonkey on its own. For builds as part of the Mozilla tree, +# config/js/Makefile.in overrides this, setting it to +# relative-symlink. +NSDISTMODE = copy As noted above, we should use SYSINSTALL, not INSTALL, and skip the NSDISTMODE mess. + +# SpiderMonkey can be built as part of the Mozilla source tree, or by +# pulling out 'js/src' and building it on its own. For the latter +# case, this makefile implements the usual behaviors for 'make' and +# 'make install'. For builds within the Mozilla tree, +# 'config/js/Makefile.in' implements the required 'make export' and +# 'make libs' behaviors in terms of this makefile's targets. +# +# We can't use the default rules from config/rules.mk here. Under the +# default rules, 'make' and 'make all' do an export, which installs +# files; that's inappropriate for stand-alone use. I don't really understand this sentence. How would `make export` be bad? Is rules.mk one of those files that must be the same in spidermonkey and mozilla? That sounds pretty fragile, because our rules.mk assumes interesting things about $(DIST) which AFAICT just don't apply to spidermonkey, right? +# Compute the linker flags that programs linking against SpiderMonkey should +# pass to get SpiderMonkey and its dependencies, beyond just the -L and -l +# for the SpiderMonkey library itself. +# - EXTRA_DSO_LDOPS includes the NSPR -L and -l flags. +# - OS_LIBS includes libraries selected by the configure script. +# - EXTRA_LIBS includes libraries selected by this Makefile. +JS_CONFIG_LIBS=$(EXTRA_DSO_LDOPTS) $(OS_LIBS) $(EXTRA_LIBS) + +# The configure script invokes this rule explicitly at configure time! +# It's important that js-config be ready by the time we're done +# configuring, because we may be running other configure scripts that +# would like to run js-config themselves, before js is built. +# +# This file and rules.mk go through a certain amount of work to decide +# which libraries to build, what to name them, and what flags to pass +# when linking them (and thus what flags its own clients must pass). +# All this information needs to go into the js-config script. To +# avoid trying to re-compute all that in the configure script, we just +# have the configure script generate this Makefile, and then invoke +# this rule. +at=@ +js-config: js-config.in Makefile $(DEPTH)/config/autoconf.mk $(topsrcdir)/config/config.mk $(topsrcdir)/config/rules.mk + rm -f js-config.tmp + sed < $< > js-config.tmp \ + -e 's|$(at)prefix$(at)|$(prefix)|' \ + -e 's|$(at)exec_prefix$(at)|$(exec_prefix)|' \ + -e 's|$(at)includedir$(at)|$(includedir)|' \ + -e 's|$(at)libdir$(at)|$(libdir)|' \ + -e 's|$(at)MOZILLA_VERSION$(at)|$(MOZILLA_VERSION)|' \ + -e 's|$(at)LIBRARY_NAME$(at)|$(LIBRARY_NAME)|' \ + -e 's|$(at)LIBRARY$(at)|$(LIBRARY)|' \ + -e 's|$(at)SHARED_LIBRARY$(at)|$(SHARED_LIBRARY)|' \ + -e 's|$(at)IMPORT_LIBRARY$(at)|$(IMPORT_LIBRARY)|' \ + -e 's|$(at)JS_CONFIG_LIBS$(at)|$(JS_CONFIG_LIBS)|' \ + && mv js-config.tmp $@ && chmod +x $@ What are the reasons we can't do this directly from configure, rather than going through a makefile? +# Ensure that we build in DIRS before building in the current directory. +# For example, config/Makefile.in runs make-system-wrappers, which must +# be complete before we can compile any libmozjs code. +all default:: + +$(LOOP_OVER_DIRS) + $(MAKE) build Can we avoid the recursive-call-of-self by moving the system-wrappers generation into this makefile? MOZ_PREF_EXTENSIONS = @MOZ_PREF_EXTENSIONS@ You don't need this in autoconf.mk -MOZ_OPTIMIZE_SIZE_TWEAK = @MOZ_OPTIMIZE_SIZE_TWEAK@ You don't need this? -MOZ_PROFILE_GUIDED_OPTIMIZE_DISABLE = @MOZ_PROFILE_GUIDED_OPTIMIZE_DISABLE@ or this? -INTEL_CC = @INTEL_CC@ -INTEL_CXX = @INTEL_CXX@ or these? I think we want JS to support ICC even more than the tree in general. MOZ_DISTRIBUTION_ID = @MOZ_DISTRIBUTION_ID@ NS_OSSO = @NS_OSSO@ MOZ_PLATFORM_HILDON = @MOZ_PLATFORM_HILDON@ can't see why you'd need these +++ b/js/src/configure.in dnl check whether to enable glitz There's lots more to remove from here, but IIRC you said you weren't sure what we need. I can go through this as a second step after landing if necessary (file a new bug if so). dnl ======================================================== -dnl Use ARM userspace kernel helpers; tell NSPR to enable -dnl their usage and use them in spidermonkey. This command sounds important... are we sure we don't need it in the spidermonkey configure? -dnl ======================================================== -dnl vtune -dnl ======================================================== -MOZ_ARG_ENABLE_BOOL(vtune, -[ --enable-vtune Enable vtune profiling], - MOZ_VTUNE=1, - MOZ_VTUNE= ) -if test -n "$MOZ_VTUNE"; then - AC_DEFINE(MOZ_VTUNE) -fi Definitely need this: see jsdbgapi.cpp +AC_DEFINE(NEED_MMGC_CONFIG_H) !? diff --git a/js/src/js-config.in b/js/src/js-config.in + [--lib-filenames] Why is this option necessary? Is it only so that mozilla can copy stuff from dist/lib to dist/bin? +if test "$echo_libs" = "yes"; then + echo "-L$libdir -l$LIBRARY_NAME $JS_CONFIG_LIBS" I can't believe this is correct... especially on MSVC, which doesn't accept -L (uses -libpath:)

I've got Windows and Mac issues to sort out, so nothing new to review yet, but here are the comments I've got so far: ---- I've changed the way config/js/Makefile.in install rules work to invoke js-config only once, and not use the shell's $(...) construct. I've done this in a way that doesn't involve config/js/Makefile.in knowing where js-config is before it's installed, which was my original reason for not wanting to run it directly out of obj-foo/js/src.. We no longer fiddle with NSDISTMODE; the js/src 'install' targets use SYSINSTALL. js/src/Makefile.in no longer sets SUPPRESS_DEFAULT_RULES; js/src uses an 'export; libs' process like everything else, both in- and out-of-tree. config/rules.mk is still one of the files shared between the top level and js/src, because it includes a decent amount of knowledge about how to build shared libraries, and so on. We shouldn't duplicate that. Perhaps that stuff should be pulled out into its own .mk file and included as necessary, but that can be a separate patch. > +# Compute the linker flags that programs linking against SpiderMonkey should > +# pass to get SpiderMonkey and its dependencies, beyond just the -L and -l > +# for the SpiderMonkey library itself. > +# - EXTRA_DSO_LDOPS includes the NSPR -L and -l flags. > +# - OS_LIBS includes libraries selected by the configure script. > +# - EXTRA_LIBS includes libraries selected by this Makefile. > +JS_CONFIG_LIBS=$(EXTRA_DSO_LDOPTS) $(OS_LIBS) $(EXTRA_LIBS) > + > +# The configure script invokes this rule explicitly at configure time! > +# It's important that js-config be ready by the time we're done > +# configuring, because we may be running other configure scripts that > +# would like to run js-config themselves, before js is built. > +# > +# This file and rules.mk go through a certain amount of work to decide > +# which libraries to build, what to name them, and what flags to pass > +# when linking them (and thus what flags its own clients must pass). > +# All this information needs to go into the js-config script. To > +# avoid trying to re-compute all that in the configure script, we just > +# have the configure script generate this Makefile, and then invoke > +# this rule. > +at=@ > +js-config: js-config.in Makefile $(DEPTH)/config/autoconf.mk > $(topsrcdir)/config/config.mk $(topsrcdir)/config/rules.mk > + rm -f js-config.tmp > + sed < $< > js-config.tmp \ > + -e 's|$(at)prefix$(at)|$(prefix)|' \ > + -e 's|$(at)exec_prefix$(at)|$(exec_prefix)|' \ > + -e 's|$(at)includedir$(at)|$(includedir)|' \ > + -e 's|$(at)libdir$(at)|$(libdir)|' \ > + -e 's|$(at)MOZILLA_VERSION$(at)|$(MOZILLA_VERSION)|' \ > + -e 's|$(at)LIBRARY_NAME$(at)|$(LIBRARY_NAME)|' \ > + -e 's|$(at)LIBRARY$(at)|$(LIBRARY)|' \ > + -e 's|$(at)SHARED_LIBRARY$(at)|$(SHARED_LIBRARY)|' \ > + -e 's|$(at)IMPORT_LIBRARY$(at)|$(IMPORT_LIBRARY)|' \ > + -e 's|$(at)JS_CONFIG_LIBS$(at)|$(JS_CONFIG_LIBS)|' \ > + && mv js-config.tmp $@ && chmod +x $@ > > What are the reasons we can't do this directly from configure, rather > than going through a makefile? The comment explains those reasons, doesn't it? We need values that are computed in the makefiles. I've fixed up the mistakes in js/src/config/autoconf.mk.in. I've fixed up the helpful/necessary code dropped from js/src/configure.in; thanks for catching those. I think I'm going blind. > diff --git a/js/src/js-config.in b/js/src/js-config.in > > + [--lib-filenames] > > Why is this option necessary? Is it only so that mozilla can copy > stuff from dist/lib to dist/bin? Yes. It seems weird to have js/src/Makefile install them in both places. How should this be done? > +if test "$echo_libs" = "yes"; then > + echo "-L$libdir -l$LIBRARY_NAME $JS_CONFIG_LIBS" > > I can't believe this is correct... especially on MSVC, which doesn't > accept -L (uses -libpath:) This is the way nspr's nspr-config behaves. I assume we just hard-code stuff under Windows.

> > + -e 's|$(at)LIBRARY_NAME$(at)|$(LIBRARY_NAME)|' \ > > + -e 's|$(at)LIBRARY$(at)|$(LIBRARY)|' \ > > + -e 's|$(at)SHARED_LIBRARY$(at)|$(SHARED_LIBRARY)|' \ > > + -e 's|$(at)IMPORT_LIBRARY$(at)|$(IMPORT_LIBRARY)|' \ > The comment explains those reasons, doesn't it? We need values that > are computed in the makefiles. Sorry, I should have been more specific. The list above are the only values computed by the makefile, correct? I think we can figure out LIBRARY_NAME in configure, and won't need the others if the js-config script isn't needed to copy files to dist/bin from dist/lib. > Yes. It seems weird to have js/src/Makefile install them in both > places. How should this be done? I tend to think that calling an extra target in js/src/Makefile might be better and simpler: $(MAKE) -C $(DEPTH)/js/src install-runtime-libs DESTDIR=$(DIST)/bin > > I can't believe this is correct... especially on MSVC, which doesn't > > accept -L (uses -libpath:) > > This is the way nspr's nspr-config behaves. I assume we just > hard-code stuff under Windows. If it is possible to fix this so that it provides correct MSVC flags, please do so. If that's really infeasible, I guess it's ok to use whatever override/hack we use for NSPR.

Status: the try server says my latest patch fails to build on Mac, as the unify phase for building universal binaries says the js-config script differs. I've gotten my build environment set up on my Mac now, and am trying to reproduce.

You should probably either not put js-config into dist/bin to begin with, or remove it before unification. You can do so here: http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#224

Ah, thanks very much.

Eagerly awaiting this to be out -- excellent work, I owe you a beer or six. :) Several issues while I was testing the latest Mercurial checkout of the jsuni repo. First one: On Linux, the MOZ_MEMORY define in configure is set to 1 by default, even if you're trying to build JS outside of the Mozilla source tree. (e.g. cp jsuni/js/src somewhere/spidermonkey ; cd somewhere/spidermonkey ; autoconf-2.13 ; ./configure) When MOZ_MEMORY is defined, jsgc.cpp tries to include ../../memory/jemalloc/jemalloc.h, which is not inside of the SpiderMonkey source dir. Passing an explicit --disable-jemalloc to ./configure fixes the issue. This shouldn't be necessary, though. Second issue (minor): js/src/configure.in offers a number of options and settings that aren't relevant to stand-alone SpiderMonkey, like --enable-glitz. Third issue: there is no dist target in js/src/Makefile.in, so making a stand-alone SpiderMonkey distribution has to be done manually with something like tar -zcf dist.tgz js/src/. I'd assume any such target need not include things like xpconnect and other stuff that (I think) are only interesting to gecko.

(In reply to comment #157) > Eagerly awaiting this to be out -- excellent work, I owe you a beer or six. :) Ooh, I accept! I've pushed the latest to jsuni (jsuni-mq is more likely to be current). > Several issues while I was testing the latest Mercurial checkout of the jsuni > repo. First one: On Linux, the MOZ_MEMORY define in configure is set to 1 by > default, even if you're trying to build JS outside of the Mozilla source tree. > (e.g. cp jsuni/js/src somewhere/spidermonkey ; cd somewhere/spidermonkey ; > autoconf-2.13 ; ./configure) > > When MOZ_MEMORY is defined, jsgc.cpp tries to include > ../../memory/jemalloc/jemalloc.h, which is not inside of the SpiderMonkey > source dir. > > Passing an explicit --disable-jemalloc to ./configure fixes the issue. This > shouldn't be necessary, though. I'll fix this. Thanks very much. > Second issue (minor): js/src/configure.in offers a number of options and > settings that aren't relevant to stand-alone SpiderMonkey, like --enable-glitz. I know; the log entry text for the big patch admits as much. There's plenty to go through and clean out. But I want to land something that works first. > Third issue: there is no dist target in js/src/Makefile.in, so making a > stand-alone SpiderMonkey distribution has to be done manually with something > like tar -zcf dist.tgz js/src/. I'd assume any such target need not include > things like xpconnect and other stuff that (I think) are only interesting to > gecko. Good point. Again, I think I'll put this off until after landing the main patch.

For the curious, ssh://hg.mozilla.org/users/jblandy_mozilla.com/jsuni and ssh://hg.mozilla.org/users/jblandy_mozilla.com/jsuni-mq have the changes and issues through comment #152 addressed, and pass the try servers.

(In reply to comment #153) > > > + -e 's|$(at)LIBRARY_NAME$(at)|$(LIBRARY_NAME)|' \ > > > + -e 's|$(at)LIBRARY$(at)|$(LIBRARY)|' \ > > > + -e 's|$(at)SHARED_LIBRARY$(at)|$(SHARED_LIBRARY)|' \ > > > + -e 's|$(at)IMPORT_LIBRARY$(at)|$(IMPORT_LIBRARY)|' \ > > > The comment explains those reasons, doesn't it? We need values that > > are computed in the makefiles. > > Sorry, I should have been more specific. The list above are the only values > computed by the makefile, correct? I think we can figure out LIBRARY_NAME in > configure, and won't need the others if the js-config script isn't needed to > copy files to dist/bin from dist/lib. Going back, it seems that the one that's hard to compute is JS_CONFIG_LIBS: # Compute the linker flags that programs linking against SpiderMonkey should # pass to get SpiderMonkey and its dependencies, beyond just the -L and -l # for the SpiderMonkey library itself. # - EXTRA_DSO_LDOPTS includes the NSPR -L and -l flags. # - OS_LIBS includes libraries selected by the configure script. # - EXTRA_LIBS includes libraries selected by this Makefile. JS_CONFIG_LIBS=$(EXTRA_DSO_LDOPTS) $(OS_LIBS) $(EXTRA_LIBS) Actually, EXTRA_DSO_LDOPTS is the interesting one: its value depends on whether we're using jemalloc and VTUNE, whether we're on Darwin (in which case we want the value of SHARED_LIBRARY, too).

This implements all the changes suggested above, except for having js/src/configure generate js-config; comment #160 explains why this would duplicate more logic than I'd like. The try server likes the complete queue, and it passes 'make check' on Linux and the descendant of MS-DOS.

Comment on attachment 342117 [details] [diff] [review] Bug 97954: Allow SpiderMonkey to be built on its own, or as part of Mozilla. I finally read through this entire patch and I don't see anything to comment on. I guess that means it's done! (Sorry that took so long...)

IRC discussion: we'd better be able to build the js shell before landing this.

comm-central has been busted by this checkin, but due to advance warning, I was aware of the potential problem, as we need to apply the client.mk changes there as well. Still looking for someone to review this bustage fix.

Comment on attachment 345321 [details] [diff] [review] fix comm-central client.mk for that change Checked in this fix as http://hg.mozilla.org/comm-central/rev/08cec1de1b42

Comment on attachment 345321 [details] [diff] [review] fix comm-central client.mk for that change >+CONFIGURES := $(TOPSRCDIR)/configure >+CONFIGURES += $(TOPSRCDIR)/mozilla/configure >+CONFIGURES += $(TOPSRCDIR)/mozilla/js/src/configure > CONFIG_STATUS_DEPS := \ > $(wildcard $(CONFIGURES)) \ > $(TOPSRCDIR)/allmakefiles.sh \ > $(TOPSRCDIR)/.mozconfig.mk \ > $(TOPSRCDIR)/mozilla/allmakefiles.sh \ > $(wildcard $(TOPSRCDIR)/mozilla/nsprpub/configure) \ >+ $(wildcard $(TOPSRCDIR)/mozilla/js/src/configure) \ Already included via $(CONFIGURES) above... (doesn't this break clobber builds?)

can't say if it is related, but starting a build from scratch gave me this error log, with a nsinstall which cannot be found : c++ -DMDCPUCFG=\"md/_linux.cfg\" -o host_jskwgen -I/home/fred/logs/suite/src/mozilla/js/src -I. -I./dist/include -I./dist/include/js -I/home/fred/logs/suite/src/objdir-sm/mozilla/dist/include/nspr -I/sdk/include -I/home/fred/logs/suite/src/mozilla/js/src host_jskwgen.o ./host_jskwgen jsautokw.h /home/fred/logs/suite/src/objdir-sm/mozilla/js/src/config/nsinstall -t -m 644 js-config.h jsautocfg.h jsautokw.h /home/fred/logs/suite/src/mozilla/js/src/js.msg /home/fred/logs/suite/src/mozilla/js/src/jsapi.h /home/fred/logs/suite/src/mozilla/js/src/jsarray.h /home/fred/logs/suite/src/mozilla/js/src/jsarena.h /home/fred/logs/suite/src/mozilla/js/src/jsatom.h /home/fred/logs/suite/src/mozilla/js/src/jsbit.h /home/fred/logs/suite/src/mozilla/js/src/jsbool.h /home/fred/logs/suite/src/mozilla/js/src/jsclist.h /home/fred/logs/suite/src/mozilla/js/src/jscntxt.h /home/fred/logs/suite/src/mozilla/js/src/jscompat.h /home/fred/logs/suite/src/mozilla/js/src/jsdate.h /home/fred/logs/suite/src/mozilla/js/src/jsdbgapi.h /home/fred/logs/suite/src/mozilla/js/src/jsdhash.h /home/fred/logs/suite/src/mozilla/js/src/jsdtoa.h /home/fred/logs/suite/src/mozilla/js/src/jsemit.h /home/fred/logs/suite/src/mozilla/js/src/jsfun.h /home/fred/logs/suite/src/mozilla/js/src/jsgc.h /home/fred/logs/suite/src/mozilla/js/src/jshash.h /home/fred/logs/suite/src/mozilla/js/src/jsinterp.h /home/fred/logs/suite/src/mozilla/js/src/jsiter.h /home/fred/logs/suite/src/mozilla/js/src/jslock.h /home/fred/logs/suite/src/mozilla/js/src/jslong.h /home/fred/logs/suite/src/mozilla/js/src/jsmath.h /home/fred/logs/suite/src/mozilla/js/src/jsnum.h /home/fred/logs/suite/src/mozilla/js/src/jsobj.h /home/fred/logs/suite/src/mozilla/js/src/json.h /home/fred/logs/suite/src/mozilla/js/src/jsopcode.tbl /home/fred/logs/suite/src/mozilla/js/src/jsopcode.h /home/fred/logs/suite/src/mozilla/js/src/jsotypes.h /home/fred/logs/suite/src/mozilla/js/src/jsparse.h /home/fred/logs/suite/src/mozilla/js/src/jsprf.h /home/fred/logs/suite/src/mozilla/js/src/jsproto.tbl /home/fred/logs/suite/src/mozilla/js/src/jsprvtd.h /home/fred/logs/suite/src/mozilla/js/src/jspubtd.h /home/fred/logs/suite/src/mozilla/js/src/jsregexp.h /home/fred/logs/suite/src/mozilla/js/src/jsscan.h /home/fred/logs/suite/src/mozilla/js/src/jsscope.h /home/fred/logs/suite/src/mozilla/js/src/jsscript.h /home/fred/logs/suite/src/mozilla/js/src/jsstddef.h /home/fred/logs/suite/src/mozilla/js/src/jsstr.h /home/fred/logs/suite/src/mozilla/js/src/jstracer.h /home/fred/logs/suite/src/mozilla/js/src/jstypes.h /home/fred/logs/suite/src/mozilla/js/src/jsutil.h /home/fred/logs/suite/src/mozilla/js/src/jsversion.h /home/fred/logs/suite/src/mozilla/js/src/jsxdrapi.h /home/fred/logs/suite/src/mozilla/js/src/jsxml.h /home/fred/logs/suite/src/objdir-sm/mozilla/dist/include/js make[4]: /home/fred/logs/suite/src/objdir-sm/mozilla/js/src/config/nsinstall : commande introuvable make[4]: *** [install] Erreur 127 Kinda annoying.