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

Default to python3 for gyp #1539

Closed
wants to merge 1 commit into from
Closed

Default to python3 for gyp #1539

wants to merge 1 commit into from

Conversation

MasterOdin
Copy link

If npm_config_python is not set, then the build process for sqlite3 defaults to searching for python command. Using this command is ambiguous at best on what version of python will run, and at worst does not exist (e.g. on alpine). It is encouraged that scripts use an explicit python version (python2 or python3) to ensure best compatibility with supported python platform. In this case, python3 works perfectly, and so no reason to use the EOL python2.

Signed-off-by: Matthew Peveler <matt@popsql.com>
@MasterOdin
Copy link
Author

Hm, I guess this breaks the Windows test build at least as it only ever provides python.exe in the environment. However, defaulting to python breaks various Linux distros, and may at some point break on macOS:

WARNING: Python 2.7 is not recommended.
This version is included in macOS for compatibility with legacy software.
Future versions of macOS will not include Python 2.7.
Instead, it is recommended that you transition to using 'python3' from within Terminal.

Not sure what the ideal solution here is to get this to work consistently out of the box for everyone, without encoding some sort of "python look-up" into node.

@MasterOdin
Copy link
Author

Seems like sqlite3 may not be installable on a fresh mac running 12.3 as python might be removed? https://developer.apple.com/documentation/macos-release-notes/macos-12_3-release-notes#Python

@rwaldron
Copy link

Can confirm @MasterOdin's prediction.

gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
  ACTION deps_sqlite3_gyp_action_before_build_target_unpack_sqlite_dep Release/obj/gen/sqlite-autoconf-3310100/sqlite3.c
/bin/sh: python: command not found
make: *** [Release/obj/gen/sqlite-autoconf-3310100/sqlite3.c] Error 127

@MasterOdin
Copy link
Author

For both mac and linux environments, using node-sqlite3 has mandated first running npm config set python python3.

@xPaw
Copy link
Contributor

xPaw commented Apr 12, 2022

Replacing this python script with a js one should solve the python wonkery: #1570

@MasterOdin
Copy link
Author

Closing this as I think #1570 is the superior solution where as you say, tar is already a transitive dependency so just doing it all in node costs nothing in that regard, and avoids dealing with looking up python vs python3 for the executable.

@MasterOdin MasterOdin closed this Apr 12, 2022
@MasterOdin MasterOdin deleted the patch-1 branch April 12, 2022 18:07
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

Successfully merging this pull request may close these issues.

3 participants