On Sun, Jun 23, 2013 at 11:28 AM, Jamie Nguyen <j(a)jamielinux.com> wrote:
Hi!
When packaging nodejs-lodash, I noticed this error:
<snip build log>
The reason appears to be because it's package.json doesn't specify a
version for node:
"engines": [
"node",
"rhino"
],
I attached a patch for nodejs-packaging with a simple workaround, though
it's possible another approach is better.
Also, is this considered a broken/non-compliant package.json? If so,
I'll send a pull request to lodash to fix this. (I've already patched
the package.json in the package to specify '*' as the version as a
workaround.)
`npm help json` [1] sez:
"engines
You can specify the version of node that your stuff works on:
{ "engines" : { "node" : ">=0.1.27 <0.1.30"
} }
And, like with dependencies, if you don´t specify the version (or if
you specify "*" as the version), then any version of node will do."
Not only is it legal for engines, but for dependencies too, and we
don't handle either case properly.
There are actually three different possible data types we need to handle here.
you could have a simple string:
{ "dependencies": "async"}
or a list:
{ "dependencies": [ "async", "slide" ] }
or a dictionary:
{ "dependencies": {
"async": "1",
"slide": "1"
}
At the moment we only handle the last case (which admittedly is the
most common).
Since there's only one member of "engines" we care about (right now
anyway), catching the exception is okay there, but that's not good
enough for the deps. We have to check the type and act accordingly
(how or whether you need to iterate over them in Python changes ever
so slightly as a result). So, just to be clear and consistent I
checked the type for "engines" as well instead of applying your patch
swallowing the exception.
This commit should fix this for "dependencies", "devDependencies",
and
"engines" in both the Requires generator and %nodejs_symlink_deps:
https://github.com/tchollingsworth/nodejs-packaging/commit/9ee6618c0bfedc...
Thanks for the report!
-T.C.