554088 - Only install public headers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mike Hommey [:glandium]]

Currently, all headers from js/src are installed, and the result is that some applications tend to abuse this state of affairs and use APIs that are supposed to be private (like gxine using JS_ISSPACE ; see bug 553997).

There are IMHO two parts in this bug:
- make install from js/src
- SDK includes.

While the former would be easy to fix, the latter is more problematic as basically anything that is in dist/include (i.e. all EXPORTS) are installed as part of the SDK.

The other tricky part is that more headers than the ones included from js*api.h are actually providing JS_PUBLIC_API, and I think some headers providing public functions even provide private functions definitions, too.

Also, jsprvtd.h and jsopcode.h, which quite don't look like things that should be public, are included from jsdbgapi.h and jsxdrapi.h.

As for files providing JS_PUBLIC_APIs but not included from any js*api.h file, they are:
jsarena.h
jsbit.h
jsdhash.h
jsfile.h
jshash.h
jsprf.h

Comment 1

[Brendan Eich [:brendan]]

This bug needs an owner.

Related, either here or in a dependent bug, work item: remove JS_*_EXTERN_C macro calls from private and therefore now C++-only .h files.

/be

Comment 5

[Nicholas Nethercote [inactive]]

What's the status of this?  Is there work remaining to be done?  I'm not sure I understand exactly what the goal is.

Comment 6

[Nicholas Nethercote [inactive]]

We've been gradually moving exported stuff from headers in js/src/ to headers in js/public/.  Does that process help with this bug?

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

So, what remains exported is:

* js/public/Anchor.h
* js/public/CallArgs.h
* js/public/CharacterEncoding.h
* js/public/Date.h
* js/public/GCAPI.h
* js/public/HashTable.h
* js/public/HeapAPI.h
* js/public/LegacyIntTypes.h
* js/public/MemoryMetrics.h
* js/public/ProfilingStack.h
* js/public/PropertyKey.h
* js/public/RequiredDefines.h
* js/public/RootingAPI.h
* js/public/StructuredClone.h
* js/public/Utility.h
* js/public/Value.h
* js/public/Vector.h
* js/src/js-config.h
* js/src/js.msg
* js/src/jsalloc.h
* js/src/jsapi.h
* js/src/jsbytecode.h
* js/src/jsclass.h
* js/src/jsclist.h
* js/src/jscpucfg.h
* js/src/jsdbgapi.h
* js/src/jsfriendapi.h
* js/src/jsprf.h
* js/src/jsprototypes.h
* js/src/jsproxy.h
* js/src/jspubtd.h
* js/src/jstypes.h
* js/src/jsversion.h
* js/src/jswrapper.h
* js/src/perf/jsperf.h

I'm not sure which, if any, of the ones in src can still be removed.

Comment 8

[Jeff Walden [:Waldo]]

jsclist.h we should be able to eliminate by using mozilla::LinkedList.

Comment 9

[Jeff Walden [:Waldo]]

Bleh, submitted too early.

jsprf.h is functionality we'd be better off not having, somehow, but there's no replacement yet.

jsprototypes.h is pretty dubious as an export, but we need to eliminate JSProtoKey from the JSAPI before we can kill that.

jscpucfg.h looks like something we could not have if we wanted, or at least the bits that matter any more could be moved into some sort of mfbt thing.

jsproxy.h, some bits of it need exposure right now, but ultimately we want to eliminate that too.  Not a shorter-term thing, tho, I suspect.  Same for jswrapper.h.

jspubtd.h should be eliminated and its types moved into more narrowly-scoped headers.  Possibly the same for bits of jstypes.h.  I'm not sure where the latters API macros should go instead, maybe js/public/APIMacros.h or something.

jsversion.h is less and less useful, a push might let us get rid of it.

js/public/LegacyIntTypes.h we can probably remove now, people have had time to adjust to those type removals at this point, I think.

Comment 10

[Nicholas Nethercote [inactive]]

Everything Waldo says is true enough, but we can make progress in the short term just by moving these, e.g. jsproxy.h --> js/Proxy.h.  This has been the general trend and is a good thing and I plan to accelerate it, but it's not clear to me if it will aid this specific bug.

Comment 11

[Jeff Walden [:Waldo]]

I don't see a reason to move jsproxy -> js/Proxy.h.  The work of moving stuff to js/ has coincided with cleaning up the APIs, and making them generally saner and more likely to be roughly future-proof.   Flat-out moves of APIs that we want to get rid of, doesn't fit that mold.

Comment 12

[Mike Hommey [:glandium]]

Waldo answered more completely than i would have.

Comment 13

[Nicholas Nethercote [inactive]]

I discussed this with Waldo on IRC.  The presence of INSTALLED_HEADERS suggests that this bug is fixed.  We can continue trimming down the contents of INSTALLED_HEADERS, as comment 9 suggests, but that should be in a separate bug.  Any complaints if I mark this bug as fixed and open a new one for the trimming down?