611495 - add $JSPATH or equivalent to find CommonJS modules

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P3)

Description

[Brian Warner [:warner :bwarner]]

It would be great if the SDK had some equivalent to $PATH or $PYTHONPATH: an environment variable which specifies a list of directories in which CommonJS packages can be found. During the 'cfx xpi' link step, these directories should be searched to find dependencies that need to be copied into the XPI.

This would make it easier to manage third-party packages (those which weren't included in the SDK, nor were written by the developer who is running cfx, but rather useful libraries which came from The Internet). We don't want to have to drop these into the SDK (where they'd disappear when you replace the SDK with a newer version).

Comment 1

[Brian Warner [:warner :bwarner]]

Oh, and this relates to bug 556582, which is about at least documenting the current package/module search path.

Comment 2

[Brian Warner [:warner :bwarner]]

This is closely related to (and might be fixed by) bug 591338, since that one proposes a module search algorithm that requires a $JSPATH or equivalent. It's not focussed on providing access to CommonJS modules (like from NPM), but that would be a likely side-effect.

Comment 3

[Brian Warner [:warner :bwarner]]

Attachment: first pass at adding --jspath

Here's an initial sketch. This would add a --jspath command-line argument (you can use it multiple times) that adds to the same list of directories that includes the SDK's top-level "packages/" directory. Every subdirectory of everything on the JSPATH should be a package, with a package.json file. Each package must have a distinct name, otherwise you'll get an error.

Questions:

* "--jspath" isn't the right name for this.. maybe --package-path?
* should there also be an environment variable to set this? (for parallelism with gcc/glibc's $LD_LIBRARY_PATH and the like). Or maybe use "-L" to match what gcc/ld uses?
* this patch looks in *subdirectories* of each entry in JSPATH, which matches what Python and gcc do. But another option would be to have JSPATH list actual packages (with the package name taken from the last component of the JSPATH entry). Not sure if that'd be better or worse.
* the patch (thanks to the existing code) searches recursively below each entry if it sees a package.json with a "packages" key: the key contains pointers to other directories that should be searched. I'm not sure how standard this feature is, whether it's part of CommonJS or something special in jetpack.

Comment 4

[Brian Warner [:warner :bwarner]]

Attachment: add --jspath, basic tests

Here's a better patch, with tests. Note that until the new linker (bug 627607) lands, the SDK doesn't understand package names very well, so if you use --jspath=~/lib/js and put a package named foo in ~/lib/js/foo (described by a ~/lib/js/foo/package.json) and want to import a module named bar.js from ~/lib/js/foo/lib/bar.js , you'll have to do require("bar"), and the linker will search through all packages (both from the SDK and from --jspath) until it finds bar.js . Later, once bug 627607 lands, you'll use require("foo/bar") and it won't look at any other package.

Also note that adding --jspath doesn't excuse you from editing your package.json to include a line like:

  dependencies: ["foo"],

since the linker first looks for packages named by these dependencies lines, then searches within them for modules named by require() statements.

All the earlier open questions are still open, starting with the name of the "--jspath" argument.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #3)
> * "--jspath" isn't the right name for this.. maybe --package-path?

--package-path makes sense to me.


> * should there also be an environment variable to set this? (for parallelism
> with gcc/glibc's $LD_LIBRARY_PATH and the like).

I would start with the command-line argument (which can be made default via the existing mechanism for doing so, I reckon).  If we do need an environment variable in the future, however, then CJS_PACKAGE_PATH seems reasonable, where CJS is short for COMMONJS.


> Or maybe use "-L" to match what gcc/ld uses?

Let's wait and see how popular --package-path is before we assign it one of the scarce one-letter aliases.


> * this patch looks in *subdirectories* of each entry in JSPATH, which matches
> what Python and gcc do. But another option would be to have JSPATH list actual
> packages (with the package name taken from the last component of the JSPATH
> entry). Not sure if that'd be better or worse.

I like what Python and gcc do, but it seems pretty trivial to support both in the future if the other proves useful.

Comment 6

[Brian Warner [:warner :bwarner]]

Attachment: rename to --package-path

Updated the argument name to --package-path, following Myk's advice.

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 528424 [details] [diff] [review]
rename to --package-path

Review of attachment 528424 [details] [diff] [review]:

Looks and works good! Thanks

Comment 8

[Brian Warner [:warner :bwarner]]

Great! Landed in https://github.com/mozilla/addon-sdk/commit/77f50e25c1a556dfeb5937d9cd9789a8e25e81fd .