search

mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

Use explicit python version to match script syntax. #120

Closed danpat closed 6 years ago

danpat commented 6 years ago

The generate_compile_commands.py script uses Python2 syntax, but has #!/usr/bin/env python at the shebang. Systems could alias python to either Python 2 or 3, so sometimes this script will fail with syntax errors if the user's system has the wrong default.

This change makes the shebang line explicit about which python version is used. It might still fail if the user doesn't have python2, but the error message will be a bit clearer, and it won't be for unexpected syntax errors.

springmeyer commented 6 years ago

I get:

$ ./scripts/generate_compile_commands.py 
env: python2: No such file or directory

On my system, OS X, without upgraded python:

$ python --version
Python 2.7.10
$ which python
/usr/bin/python
danpat commented 6 years ago

Ah, well that's just great :-( . Nothing is ever simple is it.

Looks like Ubuntu plans to default to python3 only for 18.04 LTS: https://wiki.ubuntu.com/Python

The Python PEP0394 recommends that python should be an alias for python2 for the time being (since 2011):

  • for the time being, all distributions should ensure that python refers to the same target as python2.
  • however, end users should be aware that python refers to python3 on at least Arch Linux (that change is what prompted the creation of this PEP), so python should be used in the shebang line only for scripts that are source compatible with both Python 2 and 3.
  • in preparation for an eventual change in the default version of Python, Python 2 only scripts should either be updated to be source compatible with Python 3 or else to use python2 in the shebang line.

and Apple will do whatever they like.

I've just pushed a change that lets this script work under both Python2 and Python3, and reverts the shebang to call python, as it shouldn't matter which version exists now.

springmeyer commented 6 years ago

👍 thanks, that looks good! Will merge now.