Created attachment 703082[details][diff][review]
install extra headers
For the standalone spidermonkey engine from ESR10 onwards there are a bunch of extra public headers that are required by jsapi, however these do not get installed currently.
We have been using the following patch, to install these headers in our test tarballs of ESR17.

I think this only installs the stuff that we've specifically approved, so it's not a problem in that sense. However, I don't know enough about the makefile to know why this is needed. Why do these things get installed during Firefox builds but not in other embeddings?

The effect of this change would be cause 'install' to install, in addition to the files listed in INSTALLED_HEADERS, which it already does, those listed in:
- EXPORTS_ds
- EXPORTS_gc
- EXPORTS_js
- EXPORTS_mozilla
Comment 4 suggests that we've already decided that those should go all the places INSTALLED_HEADERS go; if that's the case, then this patch is fine.
I don't *think* our build system uses the 'install' make target, though. It uses 'libs' and 'export' and some other stuff, but 'install' isn't part of the process as far as I know. So I don't think it affects the Firefox build what we do with those targets.

Created attachment 708906[details][diff][review]
Slightly updated patch, with a bit more commentary, that does what we discussed on IRC
The answer to the question in comment 4 is that includes show up to SpiderMonkey because |make export| exposes them. But this is specifically about |make install|, used to install SpiderMonkey alone onto a system.
The install target used by Firefox/Gecko/etc. is here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk
It's basically oriented around installing the stuff to *run* Firefox. There's really nothing here that makes sense for SpiderMonkey embedders, or people who want a system installation of SpiderMonkey. So it makes sense that we'd have to install all of these headers ourselves.
After some talking on IRC and me wrapping my head around things, I think the patch posted here is pretty much good, except for adding a /js to the end of one of the lines, and alphabetizing them for simplicity. And also -- and perhaps most important to me -- I wanted some comments explaining the what/why of this, so that it's not all just locked up in the minds of embedders and/or me.
Here's the resulting patch. What do people think?

Comment on attachment 708906[details][diff][review]
Slightly updated patch, with a bit more commentary, that does what we discussed on IRC
Sounds like jimb's a bit busy, forwarding another place as well...

https://hg.mozilla.org/integration/mozilla-inbound/rev/c06650ce0770
This is one of the patches we'll want to backport for a 17-based release, Sean. We'll also need to tell embedders to add js/RequiredDefines.h to their command lines, although the plan is for pkgconfig or whatever to spit it out as part of js-cflags somehow, so hopefully many won't need to take special steps here.

Comment on attachment 708906[details][diff][review]
Slightly updated patch, with a bit more commentary, that does what we discussed on IRC
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Necessary for the mozjs17 standalone release of the JS engine. Correctly installs header files into the necessary subdirectories. Does not affect non-standalone builds.
User impact if declined:
The patch is necessary for the upcoming JS standalone release based on esr17. If declined, the patch will have to be carried separately from the main repository for the lifetime of mozjs17 support.
Fix Landed on Version: 21
Risk to taking this patch (and alternatives if risky): None: only affects JS standalone builds.
String or UUID changes made by this patch: None.
See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.