Replaced 'node' by 'nodejs' in Popen() and check_call() calls

[neveldo]

Since the commit bf31fc0, I get this error message when I run the build script build.py :

[[build.py RunSmokeTest]] - entering ...
Traceback (most recent call last):
File "./build.py", line 350, in
RunSmokeTest(out_dir='dist')
File "./build.py", line 265, in RunSmokeTest
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
File "/usr/lib/python2.7/subprocess.py", line 710, in init
errread, errwrite)
File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
raise child_exception
OSError: [Errno 2] No such file or directory

I think that 'node' should be replaced by 'nodejs' in subprocess.Popen() and subprocess.check_call() calls to fix it.

[dvoytenko]

/cc @powdercloud

[erwinmombay]

/cc @cramforce since i know he intentionally changed this to get travis to work

[cramforce]

See failure of travis https://travis-ci.org/ampproject/amphtml/builds/102483529

This was done to make Travis happy. In general having a node binary instead of a nodejs binary is much more common.

Since I know that ubuntu, when you install via apt-get, does the stupid nodejs I think the best would be to support both.

Of course, you could fix it for yourself by adding alias node=nodejs to your .bashrc

[powdercloud]

This command might be a good indicator:
node --eval 'console.log("42")'
If it manages to print "42", then we can assume node is the command. And same for nodejs.
If neither work, CheckPrereqs (after trying them) could give up in style. If one of them works, it could return that command name and then the rest of the build.py should use it, maybe even for the shebang lines (also since they've drifted a tad from the smoke test actually).

I think this is potentially a rabbit hole - the risk is to reinvent autoconf, but if it's a common enough problem maybe it's worth sticking in just this one big toe?

[cramforce]

👍

[neveldo]

Thanks for the explanation. Indeed, I run the validator on Ubuntu, so the default command for node is nodejs. As it is mentionned in the README.md of the validator :

Prerequisite: Linux Ubuntu 14 or similar

Install these packages using apt-get:

npm
nodejs
openjdk-7-jre
protobuf-compiler
python-protobuf
python2.7

So I think it could be a common problem because everyone who want to run the validator by following strictly the instructions could potentially encounter the same issue.

Where do we go ? Should I submit a new PR with the proposal of updating the CheckPrereqs() function from @powdercloud ? Or maybe, we could simply update the README.md to tell the users to add an alias from nodejs to node as @cramforce said ?

[cramforce]

I think supporting both would be great!