Created attachment 123066 [details] [review]
[PATCH] Bug 561585 – allow using self-built spidermonkey

 Makefile.am  |    4 +-
 configure.ac |  138 ++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 99 insertions(+), 43 deletions(-)

Part of the patch seems to be an unrelated check that we're using threadsafe spidermonkey; gjs isn't threadsafe at all, so I'm not sure we need this. In any case it should be a separate patch, I would think.

This seems unrelated also:
  ACLOCAL_AMFLAGS = -I m4

And this, should be implied by PKG_CHECK_MODULES right:
  PKG_PROG_PKG_CONFIG

Typo "Firefix"

A better fix for sure would be for spidermonkey to just come with a .pc file. I mean, this is ludicrous that we have all this crap in configure.

(In reply to comment #2)
> Part of the patch seems to be an unrelated check that we're using threadsafe
> spidermonkey; gjs isn't threadsafe at all, so I'm not sure we need this. In any
> case it should be a separate patch, I would think.

No, it's necessary. If you build spidermonkey without --enable-threadsafe (like I accidentally did), compiling gjs against that sm fails due to different JS_GetClass definitions (2arg vs. 1arg version).

> This seems unrelated also:
>   ACLOCAL_AMFLAGS = -I m4
Unrelated, yes. Running autogen tells you to add it, and I was too lazy to file a separate bug just for that.

> And this, should be implied by PKG_CHECK_MODULES right:
>   PKG_PROG_PKG_CONFIG

Without that, I get an error running configure:
checking for GJS... configure: error: The pkg-config script could not be found or is too old.  Make sure it
is in your PATH or set the PKG_CONFIG environment variable to the full
path to pkg-config.

I think it's because the first PKG_CHECK_MODULES is in a conditional (PKG_CHECK_MODULES(JS,$JS_PACKAGE) now.

> Typo "Firefix"
Fixed in my tree.

> A better fix for sure would be for spidermonkey to just come with a .pc file. I
> mean, this is ludicrous that we have all this crap in configure.

Right, I 1000% agree :)
I looked in bmo but didn't see any bug about adding such a .pc file. Their whole build system is crap, still using autoconf 2.13, not automake, not building a static lib...

Can you add a comment explaining the reason why we require --enable-threadsafe?

Looks fine with that.

Like this?

  # Check that SpiderMonkey is compiled with --enable-threadsafe, since
  # GJS needs JS_GetClass etc. to take a JSContext* argument.
  #
  # NOTE: This does not imply that GJS itself is thread-safe! (It isn't.)

Yep

Can't pass 'make check' with self-built spidermonkey due to missing includes.

Created attachment 123166 [details] [review]
[PATCH] Make "make check" compile too. Apply on top of the previous patch.

 configure.ac |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

With the above patch, make check compiles. Make check doesn't pass here though, neither with system (xulrunner) JS nor custom built one (they even abort at different tests). Is make check currently expected to pass?

Created attachment 125002 [details] [review]
check for self-built spidermonkey

This patch set works for me with
 * Ubuntu Hardy + xulrunner 1.9.0.5
 * OSX 10.4 + self built spidermonkey

Would be good to hear from Debian / Fedora users as well.

 configure.ac     |  181 +++++++++++++++++++++++++++++++++++++++----------------
 gjs-1.0.pc.in    |    3 
 gjs-gi-1.0.pc.in |    3 
 3 files changed, 132 insertions(+), 55 deletions(-)

Created attachment 129608 [details] [review]
check for self-built spidermonkey (rediffed)

 configure.ac     |  170 ++++++++++++++++++++++++++++++++++++++++--------------
 gjs-1.0.pc.in    |    3 +-
 gjs-gi-1.0.pc.in |    3 +-
 3 files changed, 128 insertions(+), 48 deletions(-)

I have no real idea if this is right but if it's a better starting point for patches as people complain about how it doesn't build, we may as well put it in.

Commited in 9448d4fca5a5525418ed85051c4817511efbb27c and before.