Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node-gyp Python 3 support #445

Closed
bsrdjan opened this issue Feb 11, 2019 · 27 comments
Closed

node-gyp Python 3 support #445

bsrdjan opened this issue Feb 11, 2019 · 27 comments

Comments

@bsrdjan
Copy link

bsrdjan commented Feb 11, 2019

Considering Python 2 will not be maintained past 2020 and node-gyp Python 3 support looks unclear, any thoughts here how building N-API modules could look like in a year from now? Any alternatives to start exploring now?

@NickNaso
Copy link
Member

NickNaso commented Feb 11, 2019

@bsrdjan You are right, we start thinking about the problem you reported and in general it's a problem that interests all the Node.js ecosystem. In today meeting I want discuss of this problem with the rest of the team and then I will update this issue.

@mhdawson
Copy link
Member

@bsrdjan thanks for adding this to the radar. It's not specific to N-API as it will affect all modules. We should probably consider how this aspect should be included in: nodejs/TSC#642

@mhdawson
Copy link
Member

Related: nodejs/node-gyp#1337

@Yzrsah
Copy link

Yzrsah commented Feb 13, 2019

@darknoon
Copy link

darknoon commented Apr 28, 2019

I have been using CMake + ninja via CMake.js and am pretty happy with it so far.

Seems to "just work" a lot more. Very nice if you're linking modules that it knows how to find across platforms (been messing w/ PyTorch & generic Python3 modules for N-API).

I have nodemon setup to watch the fs for C++ changes and run my jest tests. Compilation is basically instant.

@pmahend1
Copy link

yes we need python 3 support. @darknoon could you please document the steps using Cmake.js instead of plain old npm build with python2?

@bsrdjan
Copy link
Author

bsrdjan commented Aug 25, 2019

Porting my node-gyp binding.gyp to cmake-js CMakeLists.txt was relatively simple. It works now on Linux, Darwin and Windows.

As @darknoon mentioned, the build itself "just works" and here what I feel missing from node-gyp:

  1. The node-gyp installs node headers, cmake-js not. My node version managers install headers on Linux and Darwin but not on Windows: nvm-windows#473

  2. The node-pre-gyp can download platform/node dependent binaries from github, as configured in binary package.json section. Cmake alternative pre-cmake-js is inactive and quick test shows that package.json "binary" node_abi parameter is ignored and remote path needs to be fixed but basically works (install from github and fallback to build). Another workaround to be investigated: Support for alternative build tools (node-cmake) mapbox/node-pre-gyp#205

  3. node-pre-gyp addon loader must be replaced, bindings work fine.

  4. --fallback-to-build

@mhdawson
Copy link
Member

@jschlight I now you did some work related to cmake, can you provide a link here.

@bsrdjan
Copy link
Author

bsrdjan commented Aug 29, 2019

Packing binaries for all platforms and node versions into single npm package, would increase the package size to several MB, which is considered a npm anti-pattern. A mechanism like node-pre-gyp is required, to fetch a single platform/node-abi binary from github during npm install. Fixing rather minor pre-cmake-js gaps would work for me and I am interested in how other node addon developers using cmake solve this problem?

@mhdawson
Copy link
Member

I'm pretty sure that @jschlight metioned he'd done some related work for one of the packages which allows you to build with cmake, just not sure if it was pre-cmake-js. @jschlight can you let us know which one it was?

@sam-github
Copy link
Contributor

sam-github commented Sep 4, 2019

For the record:

node-gyp Python 3 support looks unclear

python3 should be supported: nodejs/node-gyp#1337 (comment)

If there are problems, they will have to be fixed.

Starting to support cmake as an alternative to gyp sounds fine, but I don't think the node.js/npm/node-gyp projects have any choice other than to support gyp for building node addons for a fair while, certainly pass the EOL of python2.

Which in no way means that exploring alternatives should not move ahead.

@bsrdjan
Copy link
Author

bsrdjan commented Sep 20, 2019

My status:

  1. cmake still the same: node-gyp Python 3 support #445 (comment). Here my cmake branch, ported from napi.

  2. With the node-gyp latest release, I am still getting "Python 3 not supported" error. Reading Support for Python 3 node-gyp#1337 (comment), I could not figure out which node-gyp should work with Python 3 and how to install it? Did anyone try building NAPI add-on with node-gyp and Python 3 and how to start?

I have no preferences for either of these solutions, just need a working one. Have a feeling the cmake way might be simpler/shorter, for the time being, especially if mapbox/node-pre-gyp#205 works.

@sam-github
Copy link
Contributor

Did you set EXPERIMENTAL_NODE_GYP_PYTHON3="yes" in the environment?

@bsrdjan
Copy link
Author

bsrdjan commented Sep 24, 2019

Yes and the configure ends with: ModuleNotFoundError: No module named 'compiler'

$ node-gyp --version
v5.0.3
$ node-pre-gyp --version
v0.13.0
$ python -V
Python 3.7.4
$ export EXPERIMENTAL_NODE_GYP_PYTHON3=yes
$ node-pre-gyp configure
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.13.0
node-pre-gyp info using node@10.16.3 | darwin | x64
gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.3
gyp info using node@10.16.3 | darwin | x64
gyp info find Python using Python version 3.7.4 found at "/Users/d037732/.pyenv/versions/py374/bin/python"
gyp info spawn /Users/d037732/.pyenv/versions/py374/bin/python
gyp info spawn args [ '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/Library/Caches/node-gyp/10.16.3/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/d037732/Library/Caches/node-gyp/10.16.3',
gyp info spawn args   '-Dnode_gyp_dir=/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/d037732/Library/Caches/node-gyp/10.16.3/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/d037732/src/NG-APPS/node-rfc',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
Traceback (most recent call last):
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/gyp_main.py", line 13, in <module>
    import gyp
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 10, in <module>
    import gyp.input
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/input.py", line 7, in <module>
    from compiler.ast import Const
ModuleNotFoundError: No module named 'compiler'

According to Python documentation, the compiler module is removed in Python 3: https://docs.python.org/2/library/compiler.html

@sam-github
Copy link
Contributor

@bsrdjan v5.0.3 doesn't support py3, try master. I've asked node-gyp to publish, see nodejs/node-gyp#1791 (comment) and preceeding conversation.

@bsrdjan
Copy link
Author

bsrdjan commented Sep 25, 2019

Thanks @sam-github. With EXPERIMENTAL_NODE_GYP_PYTHON3="yes" and master branch it works on Linux and fails on macOS and Windows:

macOS (Xcode and CLT are installed)

$ node-pre-gyp configure

node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.13.0
node-pre-gyp info using node@10.16.3 | darwin | x64
gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.3
gyp info using node@10.16.3 | darwin | x64
gyp info find Python using Python version 3.7.4 found at "/Users/d037732/.pyenv/versions/py374/bin/python"
gyp info spawn /Users/d037732/.pyenv/versions/py374/bin/python
gyp info spawn args [ '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/Library/Caches/node-gyp/10.16.3/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/d037732/Library/Caches/node-gyp/10.16.3',
gyp info spawn args   '-Dnode_gyp_dir=/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/d037732/Library/Caches/node-gyp/10.16.3/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/d037732/src/NG-APPS/node-rfc',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
No receipt for 'com.apple.pkg.DeveloperToolsCLILeo' found at '/'.
No receipt for 'com.apple.pkg.DeveloperToolsCLI' found at '/'.
gyp: No Xcode or CLT version detected!

Windows

$ node-pre-gyp configure
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.13.0
node-pre-gyp info using node@10.15.3 | win32 | x64
gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.3
gyp info using node@10.15.3 | win32 | x64
gyp info find Python using Python version 3.7.4 found at "C:\Tools\Python374\python.exe"
gyp info find VS using VS2017 (15.7.27703.2026) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Tools\Python374\python.exe
gyp info spawn args [ 'C:\\src\\node-rfc\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\src\\node-rfc\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\src\\node-rfc\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\d037732\\AppData\\Local\\node-gyp\\Cache\\10.15.3\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\d037732\\AppData\\Local\\node-gyp\\Cache\\10.15.3',
gyp info spawn args   '-Dnode_gyp_dir=C:\\src\\node-rfc\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\\\Users\\\\d037732\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\10.15.3\\\\<(target_arch)\\\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\src\\node-rfc',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\src\\node-rfc\\build',
gyp info spawn args   '-Goutput_dir=.' ]
Traceback (most recent call last):
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\gyp_main.py", line 44, in <module>
    sys.exit(gyp.script_main())
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\__init__.py", line 554, in script_main
    return main(sys.argv[1:])
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\__init__.py", line 547, in main
    return gyp_main(args)
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\__init__.py", line 532, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\generator\msvs.py", line 2040, in GenerateOutput
    version=msvs_version)
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\MSVSNew.py", line 208, in __init__
    self.Write()
  File "C:\src\node-rfc\node_modules\node-gyp\gyp\pylib\gyp\MSVSNew.py", line 233, in Write
    all_entries = sorted(all_entries)
TypeError: '<' not supported between instances of 'MSVSFolder' and 'MSVSProject'
gyp ERR! configure error

@sam-github
Copy link
Contributor

@bsrdjan I'm a bit confused. You are using node-pre-gyp, not node-gyp, and node-pre-gyp is using gyp info using node-gyp@5.0.3 according to your output, not node-gyp master. I think you might need to wait until node-gyp is published, and the downstreams use it, unless you want to manually patch node-gyp@master into your deps, just to give feedback on what is broken with py3 (and there are surely things broken).

@cclauss
Copy link

cclauss commented Sep 25, 2019

The Python stack trace is saying that all_entries contains at least one MSVSFolder and at least one MSVSProject and that is like trying to do sort(apples, oranges). This is probably caused by __cmp__() being removed in Python 3... I will dig in but it is probably an old gyp.

The changes in nodejs/node-gyp#1793 should have fixed this one. It would be helpful to have a repeatable test case that fails. Perhaps you are spot on Sam that a new release might fix this.

@cclauss
Copy link

cclauss commented Sep 25, 2019

The macOS issue should be fixed in nodejs/node-gyp#1890

The No module named 'compiler' issue #445 (comment) was fixed in nodejs/node-gyp#1820

@bsrdjan
Copy link
Author

bsrdjan commented Sep 26, 2019

@sam-github it shows 5.0.3 but it is the master branch, here my package.json.zip. Testing with pre-gyp is convenient for me because no changes in my binding.gyp needed.

I re-tested with macOS and the error message is now different:

$ export EXPERIMENTAL_NODE_GYP_PYTHON3=yes                                                                                                                                    
$ node-pre-gyp configure
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.13.0
node-pre-gyp info using node@10.16.3 | darwin | x64
gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.3
gyp info using node@10.16.3 | darwin | x64
gyp info find Python using Python version 3.7.4 found at "/Users/d037732/.pyenv/versions/py374/bin/python"
gyp info spawn /Users/d037732/.pyenv/versions/py374/bin/python
gyp info spawn args [ '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Users/d037732/Library/Caches/node-gyp/10.16.3/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Users/d037732/Library/Caches/node-gyp/10.16.3',
gyp info spawn args   '-Dnode_gyp_dir=/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Users/d037732/Library/Caches/node-gyp/10.16.3/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/Users/d037732/src/NG-APPS/node-rfc',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
Traceback (most recent call last):
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/gyp_main.py", line 44, in <module>
    sys.exit(gyp.script_main())
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 554, in script_main
    return main(sys.argv[1:])
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 547, in main
    return gyp_main(args)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 532, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 2215, in GenerateOutput
    part_of_all=qualified_target in needed_targets)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 802, in Write
    self.WriteCopies(spec['copies'], extra_outputs, part_of_all)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 1145, in WriteCopies
    env = self.GetSortedXcodeEnv()
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 1885, in GetSortedXcodeEnv
    additional_settings)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 1616, in GetSortedXcodeEnv
    additional_settings)
  File "/Users/d037732/src/NG-APPS/node-rfc/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 1527, in _GetXcodeEnv
    if XcodeVersion() >= '0500' and not env.get('SDKROOT'):
TypeError: '>=' not supported between instances of 'tuple' and 'str'
gyp ERR! configure error 

@sam-github
Copy link
Contributor

@bsrdjan That makes sense, I forgot that master's package has 5.0.3 in it :-)

@cclauss might have ideas on the specific error.

You package isn't on github or otherwise publically available/reproducible, is it? If the build was reproducible, it's probably be faster to fix than round-tripping through here.

rvagg pushed a commit to nodejs/node-gyp that referenced this issue Sep 27, 2019
rvagg pushed a commit to nodejs/node-gyp that referenced this issue Sep 27, 2019
@bsrdjan
Copy link
Author

bsrdjan commented Sep 27, 2019

Yes, it would simplify the testing and conversation.

The github project is sap/node-rfc and the working branch, built with Python2, is napi. The releases of this branch are linked to npm node-rfc package pre-releases (npm install node-rfc@next).

Above mentioned issues should be reproducible with gyp3 branch, used for Python3 builds.

The cmake branch has no issues with Python 3 but others: #445 (comment).

@jschlight
Copy link
Contributor

CMake.js is a viable alternative to node-gyp for building Node N-API native addons. The N-API Team has created an example project that shows how this can be accomplished.

In addition, prebuild now supports CMake.js as a build backend for N-API native addons. The specifics can be found in the documentation. prebuild is of interest to Node native addon maintainers using GitHub as it facilitates the building and uploading of native addon binaries as GitHub releases. This permits users of the native addon to download the binary from GitHub instead of needing to have the necessary build tools installed on their development and deployment machines.

@bsrdjan
Copy link
Author

bsrdjan commented Oct 1, 2019

Thanks @jschlight, for the the prebuild hint.

My project works with CMake now, on Linux, Windows and macOS and this issue can be closed AFAIAC.

@mhdawson
Copy link
Member

mhdawson commented Oct 1, 2019

@bsrdjan thanks for the update, closing

@jschlight
Copy link
Contributor

@bsrdjan You might consider adding an N-API Badge to your README file. The details are here.

@bsrdjan
Copy link
Author

bsrdjan commented Oct 7, 2019

thanks, done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants