Create pre-install script #136

Open
raztus opened this Issue Feb 6, 2014 · 12 comments

Comments

Projects
None yet
2 participants
Collaborator

raztus commented Feb 6, 2014

A large portion of the issues opened are caused by incorrect environment conditions for the build. @vrvolle has created a pre-install shell script, and suggested the creation of something similar in Javascript for a cross-platform pre-build check.

raztus referenced this issue Feb 6, 2014

Closed

Publish Binaries #134

Collaborator

raztus commented Feb 6, 2014

From @vrvolle's comment in #134:

ok, started at https://github.com/vrvolle/node-oracle/tree/pre-install-check

Could someone please test the current version on Windows. I currently have no working Windows environment.

Current state:

Checks for

  • environment variables
  • OSX: DYLD_LIBRARY_PATH is set (not that it is set correctly)
  • links to libraries exist (and the targets exist)
  • Linux: libaio, libocci, ... are found by "ldconfig -p"

Necessary improvements:

  • OCI_VERSION is set correctly

Possible improvements:

  • create the correct symlinks itself
Collaborator

raztus commented Feb 6, 2014

Just quickly browsing through that script, I worry about forcing the existance of OCI_HOME, since my installations don't use that. Instead we have OCI_LIB DIR, and OCI_INCLUDE_DIR=$OCI_LIB_DIR/... Also, those environment variables are not even necessary if the instant client lives in one of the defaults specified in binding.gyp, so it would be a nice enhancement to check for those directories if the environment variables are not present.

Contributor

kontrafiktion commented Feb 6, 2014

ah, good point.
didn't know about default locations.

new version:

  • OCI_HOME is not checked anymore
  • OCI_VERSION is checked
  • if OCI_*_DIR are not set, default values are assumed
Contributor

kontrafiktion commented Feb 7, 2014

hitting a road block:

  1. if I add my script as 'preinstall', the software will never be built (node-gyp rebuild is normally invoked as preinstall)
  2. I cannot invoke node-gyp rebuild inside the pre-install-script, because npm's node-gyp is not on my path ...

any ideas?

Is it possible to run a scipt from within node-gyp?

Collaborator

raztus commented Feb 7, 2014

I think you'll want to put it in the "scripts" section in package.json. Something like:
"preinstall": "./pre_install_check.js"
See https://npmjs.org/doc/misc/npm-scripts.html.

Collaborator

raztus commented Feb 7, 2014

Another enhancement to the script would be to check for python < v3.0 (See node-gyp requirements at https://github.com/TooTallNate/node-gyp):

require('child_process').exec('python --version', function(err, stdout, stderr) {
    console.log(stderr);
});

Of course, node-gyp might already do this, so maybe this is redundant.

Contributor

kontrafiktion commented Feb 7, 2014

hi @raztus
I put my script into the scripts section as 'preinstall'. But that is exactly the issue. Normally node-gyp rebuild is executed by npm in the "preinstall"-Phase. When a preinstall-script is defined, "node-gyp" isn't invoked anymore.

my current solution is this: I have a shell script called "pre-install-script" (without extension!) and a Windows batch file "pre-install-script.cmd". Both call the pre-install-check.js script and afterwards invoke "node-gyp rebuild".

Collaborator

raztus commented Feb 7, 2014

I see what you're saying. This seems to work, but I don't know if it's the best way:

"scripts": {
    "preinstall": "./pre_install_check.js",
    "install": "node-gyp rebuild",
    "test": "nodeunit test/integration.js test/outparams.js"
  },
Contributor

kontrafiktion commented Feb 10, 2014

ok, I have a working version with the following caveats:

Either:
(1) on *nix the user has to add "." to his PATH, before invoking `npm install´
or
(2) on Windows the output is written to a log file and not directly visible

I would prefer the first solution (that's how the current implementation works, but it is simple to change)

Background:
(1) we can have a "preinstall" script in package.json that invokes node pre-install-check.js and node-gyp rebuild. But to be able to do that in a cross-platform way, we need a shell script (without any extension) and a windows cmd-file (with the .cmd-extension). And to be able to execute the shell script, the user must have "." in his PATH (using "./pre-install-check" does not work on Windows).
(2) we put the "node pre-install-check.js" invocation as custom action into the binding.gyp file, but then the output to stdout/stderr of the script is not shown (but I write it to a log file, which is a good idea anyway).

please try it out: https://github.com/vrvolle/node-oracle/tree/pre-install-check

@raztus: python is now checked (invocation and version == 2.7.x)

I will change the README and the INSTALL.

Are we sure that OCI_HOME is not needed?

Collaborator

raztus commented Feb 22, 2014

Yes, OCI_HOME is an invention in this library to avoid repeating the path to the instantclient in both OCI_LIB_DIR and OCI_INCLUDE_DIR, which are also inventions in this library.

Nice work on this! This will go a long way toward reducing users' configuration issues.

I plan to test this out across various platforms in the next few days.

Contributor

kontrafiktion commented Feb 22, 2014

Please wait, I learned something, which may resolve the 'caveats' from above. On Unix and on Windows I can exexcute two commands connected with '&&' (the semantics might be slightly different: I do not know (yet) if the second command on Windows gets executed, when the first one fails. But that may not be a problem. Will try it out this WE)

Contributor

kontrafiktion commented Feb 22, 2014

ok. go!

moved the invocation of the pre-install-check back to package.json:

  "scripts": {
    "preinstall": "node pre-install-check.js && node-gyp rebuild",
    [...]
  },

On Unix, we could even stop the invocation of node-gyp rebuild when the pre-install-checks fail.
I would not do that for now, there could be a constellation, where the pre-install fails even though the build would go through.

So for now, OSX/Linux/Windows should behave exactly the same (though some checks differ depending on the OS).

Example output:

checking prerequisites:
  INFO: OCI_VERSION not set, using: 11
  ERROR: file not found: /tmp/occi.h
    probably OCI_INCLUDE_DIR points to the wrong directory '/tmp'
done checking prerequisites

https://github.com/vrvolle/node-oracle/tree/pre-install-check is up to date

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment