Use find-java-home to find java/javac

nikku / node-xsd-schema-validator

A schema (XSD) validator for NodeJS

https://www.npmjs.com/package/xsd-schema-validator

MIT License

51 stars 24 forks source link

Use find-java-home to find java/javac #2

Closed tadas-s closed 9 years ago

tadas-s commented 9 years ago

OS X by default does not provide JAVA_HOME variable and there's this fairly useful npm pacakge find-java-home to figure out java home on any platform.

Notice that I have a slight hack in this patch:

    JAVA = JAVA_HOME.trim() + '/bin/java';
    JAVAC = JAVA_HOME.trim() + '/bin/javac';

trim() is required since find-java-home probably has a bug and returns value with \n in the end.

I'll try to figure out what's wrong with that and maybe submit PR to find-java-home maintainer, but for now, here's this PR for your consideration.

nikku commented 9 years ago

Thanks for your PR.

nikku commented 9 years ago

I had a closer look at your pull request. I'd like not to pull it in, because

replaces a simple and robust solution (i.e. rely on JAVA_HOME as an environment variable) with something I cannot test (got no Mac OSX and no Windows either).
adds find-java-home, a dependency with known issues (you mentioned the trim() hack).

I added a hint on where to point JAVA_HOME to on Mac OSX to the documentation (fe7796dd6d0cd8d4b0cfee9c6640f501bc7f0af1).

As far is I know most JDK distributions seem to set this variable.

tadas-s commented 9 years ago

Hi,

find-java-home looks at JAVA_HOME first and then falls back to alternatives
Mac OSX Java install does not appear to configure it during install

I just didn't want to add extra step for our dev team to run test suite (we use your package to validate some xml documents which is great). We can rely on our own fork though so it's fine, but I thought it's not a bad idea to use find-java-home.