search

jordwalke / CommonML

Simple OCaml Development Workflow on CommonJS
MIT License
96 stars 11 forks source link

Shell is required to pick up opam environment #19

Open ul opened 8 years ago

ul commented 8 years ago

Not sure how to properly fix it, but having

"findlibPackages": [{"dependency": "js_of_ocaml"}]

gives me an error:

ocamlfind: Package `js_of_ocaml' not found
Dependencies scanned and verified, but failed to build

Stack Trace:
------------
 Error: Command failed: ocamlfind ocamldep  -package js_of_ocaml -only-show
ocamlfind: Package `js_of_ocaml' not found

    at checkExecSyncError (child_process.js:470:13)
    at Object.execSync (child_process.js:510:13)
    at getFindlibCommand (/home/ul/Projects/ludwig/node_modules/CommonML/build.js:1149:24)
    at discoverDeps (/home/ul/Projects/ludwig/node_modules/CommonML/build.js:1604:32)
    at dirtyDetectingBuilder (/home/ul/Projects/ludwig/node_modules/CommonML/build.js:2044:5)
    at onAllSubpackagesDone (/home/ul/Projects/ludwig/node_modules/CommonML/build.js:2266:11)
    at /home/ul/Projects/ludwig/node_modules/async/lib/async.js:52:16
    at replenish (/home/ul/Projects/ludwig/node_modules/async/lib/async.js:317:29)
    at /home/ul/Projects/ludwig/node_modules/async/lib/async.js:333:15
    at Object.async.forEachLimit.async.eachLimit (/home/ul/Projects/ludwig/node_modules/async/lib/async.js:226:35)

If I change https://github.com/jordwalke/CommonML/blob/ccea57fa15f9ecad27ec3e47e139783864430ba0/build.js#L1148 to

return child_process.execSync(findLib, {shell: "fish"}).toString().trim();

it picks up environment fine.

Probably worth to add shell as a config parameter to CommonML entry in package.json?

jordwalke commented 8 years ago

I don't know if it should go in the package.json because that is shared to everyone regardless of their shell. Maybe as a param to node build.js?

I wonder why this is not handled automatically by execSync.

ul commented 8 years ago

Yes, you are right, making it a param to the build script is a better idea.