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

configure fails on linux for distributions using python 3 as default #9512

Closed
shannon-nii opened this issue Nov 8, 2016 · 32 comments
Closed
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.

Comments

@shannon-nii
Copy link

Currently the configure script executes via "python", this works perfectly fine on systems that use python 2.x as their default python but it fails on systems that use python 3.x. Would it be possible/feasible to adjust it to execute via "python2" instead of "python"? Alternatively an updated configure script that works on 2.x and 3.x python would be good.

Python 3 error when trying to run configure:
vpxclient_2016-11-08_13-37-44

Is there possibly just a missing module for python3?

@Fishrock123 Fishrock123 added the build Issues and PRs related to build files or the CI. label Nov 8, 2016
@silverwind
Copy link
Contributor

silverwind commented Nov 8, 2016

This also applies to Arch Linux on which python is actually python3. I think it'd be great if we could apply __future__ and friends to support both versions.

@bnoordhuis
Copy link
Member

That won't help. configure does import gyp and gyp does not pretend to be python3-compatible. Changing the shebang to python2 breaks systems with no python2 binary. I think that includes OS X.

@silverwind
Copy link
Contributor

Indeed, macOS has no python2, only python2.6 and python2.7. The version from homebrew does feature all the symlinks including python2, but I'm not sure we can require that.

As for gyp: There's https://bugs.chromium.org/p/gyp/issues/detail?id=36, but it's unlikely gyp will ever gain support for Python3 as it's pretty much a dead project from what I understand.

@jbergstroem
Copy link
Member

There's a bunch of issues in the node-gyp repo that are similar to this. Unfortunately, the conclusion that @silverwind mentioned is likely what we can expect; upstream gyp won't get ported to python 3.

@shannon-nii
Copy link
Author

manjaro is based on arch linux and it has python2 and python2.7 links
This should be the same for arch linux but i'm not sure about other linux distributions.
For now I run the configure script directly via python 2.x.
Would it be possible to at least add a check for the python version and show an error message if python3 is detected with an advise to run the script via python2?

@bnoordhuis
Copy link
Member

I don't think so. The python interpreter will throw syntax errors before the script even gets a chance to run.

The only way to work around that is to split off the main configure script into a separate script and leave only a loader stub that is legal python 2 + 3 but that seems like overcomplicating things when the python 2 prerequisite is listed in BUILDING.md.

@silverwind
Copy link
Contributor

silverwind commented Nov 9, 2016

I think it should be possible to make the file parseable in Python 3 and print a error when python >= 3. This should work as long as Python does not evaluate import before execution.

For the except issue in the original post, we can apply this workaround.

@silverwind silverwind added the good first issue Issues that are suitable for first-time contributors. label Nov 9, 2016
@kalrover
Copy link
Contributor

Can you guys help me check my first PR on node. If there is any problems, let me know.

@zenhack
Copy link

zenhack commented Nov 28, 2016

A couple ideas:

  1. We demand specifically python 2.7, and then do #!/usr/bin/env python2.7, or
  2. We put some logic like this at the top of the script:
#!/usr/bin/env sh

# Find a python 2 executable:
python_executable="`which python2"
if [ -z "$python_executable" ]; then
  python_executable="`which python2.7`"
fi
if [ -z "$python_executable" ]; then
  python_executable="`which python2.6`"
fi
if [ -z "$python_executable" ]; then
  printf 'Error: could not find python 2 executable\n' >&2
  exit 1
fi

# Copy the remainder of this script (which is python) to a new file and execute it with
# python instead of sh:
tmpfile="`mktemp`"
cat "$0" | tail -n +22 > "$tmpfile"
chmod +x "$tmpfile"
exec "$python_executable" "tmpfile" $@

(1) is certainly simpler.

@zenhack
Copy link

zenhack commented Nov 28, 2016

Αnother option is to special-case MacOS, doing something like:

case `uname` in
  darwin) python_executable="`which python2.7`" ;; # Or whatever uname returns on OS X; I don't have a live system handy.
  *) python_executable="`which python`";;
esac

@silverwind
Copy link
Contributor

This is fixed by #9657.

@zenhack generally good ideas, but I think the current solution is good enough as the user will know what to do from the error message. Spawning another python child process doesn't seem like the cleanest solution to me.

@zenhack
Copy link

zenhack commented Nov 30, 2016

So, this actually does't fix the problem:

$ python2 configure 
creating icu_config.gypi
* Using ICU in deps/icu-small
Using version-specific floating patch tools/icu/patches/58/source/i18n/digitlst.cpp
creating icu_config.gypi
{ 'target_defaults': { 'cflags': [],
                       'default_configuration': 'Release',
                       'defines': [],
                       'include_dirs': [],
                       'libraries': []},
  'variables': { 'asan': 0,
                 'coverage': 'false',
                 'debug_devtools': 'node',
                 'force_dynamic_crt': 0,
                 'gas_version': '2.26',
                 'host_arch': 'x64',
                 'icu_data_file': 'icudt58l.dat',
                 'icu_data_in': '../../deps/icu-small/source/data/in/icudt58l.dat',
                 'icu_endianness': 'l',
                 'icu_gyp_path': 'tools/icu/icu-generic.gyp',
                 'icu_locales': 'en,root',
                 'icu_path': 'deps/icu-small',
                 'icu_small': 'true',
                 'icu_ver_major': '58',
                 'node_byteorder': 'little',
                 'node_enable_d8': 'false',
                 'node_enable_v8_vtunejit': 'false',
                 'node_install_npm': 'true',
                 'node_module_version': 51,
                 'node_no_browser_globals': 'false',
                 'node_prefix': '/usr/local',
                 'node_release_urlbase': '',
                 'node_shared': 'false',
                 'node_shared_cares': 'false',
                 'node_shared_http_parser': 'false',
                 'node_shared_libuv': 'false',
                 'node_shared_openssl': 'false',
                 'node_shared_zlib': 'false',
                 'node_tag': '',
                 'node_use_bundled_v8': 'true',
                 'node_use_dtrace': 'false',
                 'node_use_etw': 'false',
                 'node_use_lttng': 'false',
                 'node_use_openssl': 'true',
                 'node_use_perfctr': 'false',
                 'node_use_v8_platform': 'true',
                 'openssl_fips': '',
                 'openssl_no_asm': 0,
                 'shlib_suffix': 'so.51',
                 'target_arch': 'x64',
                 'uv_parent_path': '/deps/uv/',
                 'uv_use_dtrace': 'false',
                 'v8_enable_gdbjit': 0,
                 'v8_enable_i18n_support': 1,
                 'v8_inspector': 'true',
                 'v8_no_strict_aliasing': 1,
                 'v8_optimized_debug': 0,
                 'v8_random_seed': 0,
                 'v8_use_snapshot': 'true',
                 'want_separate_host_toolset': 0,
                 'want_separate_host_toolset_mkpeephole': 0}}
creating config.gypi
creating config.mk
/bin/sh: python: command not found
gyp: Call to 'python -c "import sys; print sys.byteorder"' returned exit status 127 while in /home/isd/scratch/node/deps/v8/src/v8.gyp. while loading dependencies of /home/isd/scratch/node/node.gyp while trying to load /home/isd/scratch/node/node.gyp
Error running GYP

There are a whole bunch of other python scripts in the tree that also hard-code the name of the python executable, so they'd all have to be run with python2, and it isn't entirely straightforward to do that short of patching the source tree, since they aren't invoked directly by the user. Here's a list:

$ grep -r '^#!/usr/bin/env python$' .
./deps/cares/build/gcc_version.py:#!/usr/bin/env python
./deps/cares/gyp_cares:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/setup.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/MSVSSettings_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/easy_xml_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/ninja_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/msvs_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/xcode_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/mac_tool.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/input_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/common_test.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/flock_tool.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/pylib/gyp/win_tool.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/buildbot/buildbot_run.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/gyp_main.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/tools/pretty_sln.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/tools/pretty_gyp.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/tools/graphviz.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/tools/pretty_vcproj.py:#!/usr/bin/env python
./deps/npm/node_modules/node-gyp/gyp/gyptest.py:#!/usr/bin/env python
./deps/v8/test/test262/list.py:#!/usr/bin/env python
./deps/v8/test/test262/archive.py:#!/usr/bin/env python
./deps/v8/tools/unittests/run_perf_test.py:#!/usr/bin/env python
./deps/v8/tools/js2c.py:#!/usr/bin/env python
./deps/v8/tools/find-commit-for-patch.py:#!/usr/bin/env python
./deps/v8/tools/stats-viewer.py:#!/usr/bin/env python
./deps/v8/tools/run-tests.py:#!/usr/bin/env python
./deps/v8/tools/grokdump.py:#!/usr/bin/env python
./deps/v8/tools/gyp_flag_compare.py:#!/usr/bin/env python
./deps/v8/tools/dump-cpp.py:#!/usr/bin/env python
./deps/v8/tools/run-deopt-fuzzer.py:#!/usr/bin/env python
./deps/v8/tools/trace-maps-processor.py:#!/usr/bin/env python
./deps/v8/tools/eval_gc_nvp.py:#!/usr/bin/env python
./deps/v8/tools/release/auto_push.py:#!/usr/bin/env python
./deps/v8/tools/release/test_scripts.py:#!/usr/bin/env python
./deps/v8/tools/release/script_test.py:#!/usr/bin/env python
./deps/v8/tools/release/mergeinfo.py:#!/usr/bin/env python
./deps/v8/tools/release/check_clusterfuzz.py:#!/usr/bin/env python
./deps/v8/tools/release/releases.py:#!/usr/bin/env python
./deps/v8/tools/release/create_release.py:#!/usr/bin/env python
./deps/v8/tools/release/auto_tag.py:#!/usr/bin/env python
./deps/v8/tools/release/search_related_commits.py:#!/usr/bin/env python
./deps/v8/tools/release/test_search_related_commits.py:#!/usr/bin/env python
./deps/v8/tools/release/merge_to_branch.py:#!/usr/bin/env python
./deps/v8/tools/release/push_to_candidates.py:#!/usr/bin/env python
./deps/v8/tools/release/common_includes.py:#!/usr/bin/env python
./deps/v8/tools/release/roll_merge.py:#!/usr/bin/env python
./deps/v8/tools/release/test_mergeinfo.py:#!/usr/bin/env python
./deps/v8/tools/release/auto_roll.py:#!/usr/bin/env python
./deps/v8/tools/release/git_recipes.py:#!/usr/bin/env python
./deps/v8/tools/android-run.py:#!/usr/bin/env python
./deps/v8/tools/mingw-generate-makefiles.sh:#!/usr/bin/env python
./deps/v8/tools/callstats.py:#!/usr/bin/env python
./deps/v8/tools/perf-to-html.py:#!/usr/bin/env python
./deps/v8/tools/run-valgrind.py:#!/usr/bin/env python
./deps/v8/tools/concatenate-files.py:#!/usr/bin/env python
./deps/v8/tools/generate_shim_headers/generate_shim_headers.py:#!/usr/bin/env python
./deps/v8/tools/try_perf.py:#!/usr/bin/env python
./deps/v8/tools/gcmole/download_gcmole_tools.py:#!/usr/bin/env python
./deps/v8/tools/gcmole/run-gcmole.py:#!/usr/bin/env python
./deps/v8/tools/gcmole/parallel.py:#!/usr/bin/env python
./deps/v8/tools/verify_source_deps.py:#!/usr/bin/env python
./deps/v8/tools/isolate_driver.py:#!/usr/bin/env python
./deps/v8/tools/ll_prof.py:#!/usr/bin/env python
./deps/v8/tools/run.py:#!/usr/bin/env python
./deps/v8/tools/process-heap-prof.py:#!/usr/bin/env python
./deps/v8/tools/test-server.py:#!/usr/bin/env python
./deps/v8/tools/dev/v8gen.py:#!/usr/bin/env python
./deps/v8/tools/presubmit.py:#!/usr/bin/env python
./deps/v8/tools/gen-postmortem-metadata.py:#!/usr/bin/env python
./deps/v8/tools/objdump-v8:#!/usr/bin/env python
./deps/v8/tools/run_perf.py:#!/usr/bin/env python
./deps/v8/tools/gc-nvp-to-csv.py:#!/usr/bin/env python
./deps/v8/tools/sanitizers/sancov_formatter.py:#!/usr/bin/env python
./deps/v8/tools/sanitizers/sancov_merger.py:#!/usr/bin/env python
./deps/v8/tools/sanitizers/sanitize_pcs.py:#!/usr/bin/env python
./deps/v8/tools/gc-nvp-trace-processor.py:#!/usr/bin/env python
./deps/v8/tools/external-reference-check.py:#!/usr/bin/env python
./deps/v8/tools/testrunner/local/testsuite_unittest.py:#!/usr/bin/env python
./deps/v8/tools/testrunner/local/statusfile_unittest.py:#!/usr/bin/env python
./deps/v8/tools/testrunner/local/pool.py:#!/usr/bin/env python
./deps/v8/tools/testrunner/local/pool_unittest.py:#!/usr/bin/env python
./deps/v8/tools/testrunner/server/daemon.py:#!/usr/bin/env python
./deps/v8/tools/disasm.py:#!/usr/bin/env python
./deps/v8/tools/jsfunfuzz/download_jsfunfuzz.py:#!/usr/bin/env python
./deps/v8/tools/generate-builtins-tests.py:#!/usr/bin/env python
./deps/v8/gypfiles/landmines.py:#!/usr/bin/env python
./deps/v8/gypfiles/detect_v8_host_arch.py:#!/usr/bin/env python
./deps/v8/gypfiles/get_landmines.py:#!/usr/bin/env python
./deps/v8/gypfiles/vs_toolchain.py:#!/usr/bin/env python
./deps/v8/gypfiles/coverage_wrapper.py:#!/usr/bin/env python
./deps/v8/gypfiles/gyp_v8:#!/usr/bin/env python
./deps/v8/gypfiles/download_gold_plugin.py:#!/usr/bin/env python
./deps/uv/gyp_uv.py:#!/usr/bin/env python
./deps/v8_inspector/third_party/markupsafe/bench/runbench.py:#!/usr/bin/env python
./deps/v8_inspector/third_party/WebKit/Source/platform/inspector_protocol/CheckProtocolCompatibility.py:#!/usr/bin/env python
./deps/v8_inspector/third_party/WebKit/Source/platform/inspector_protocol/ConcatenateProtocols.py:#!/usr/bin/env python
./deps/v8_inspector/third_party/jinja2/scripts/jinja2-debug.py:#!/usr/bin/env python
./deps/v8_inspector/third_party/jinja2/scripts/make-release.py:#!/usr/bin/env python
./deps/v8_inspector/src/inspector/build/rjsmin.py:#!/usr/bin/env python
./deps/v8_inspector/src/inspector/build/generate_protocol_externs.py:#!/usr/bin/env python
./deps/v8_inspector/src/inspector/build/compile-scripts.py:#!/usr/bin/env python
./deps/v8_inspector/src/inspector/build/check_injected_script_source.py:#!/usr/bin/env python
./deps/v8_inspector/src/inspector/PRESUBMIT.py:#!/usr/bin/env python
./test/gc/node_modules/weak/build/gyp-mac-tool:#!/usr/bin/env python
./tools/js2c.py:#!/usr/bin/env python
./tools/mkssldef.py:#!/usr/bin/env python
./tools/install.py:#!/usr/bin/env python
./tools/cpplint.py:#!/usr/bin/env python
./tools/configure.d/nodedownload.py:#!/usr/bin/env python
./tools/gyp/setup.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/MSVSSettings_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/easy_xml_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/generator/ninja_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/generator/msvs_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/generator/xcode_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/mac_tool.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/input_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/common_test.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/flock_tool.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/__init__.py:#!/usr/bin/env python
./tools/gyp/pylib/gyp/win_tool.py:#!/usr/bin/env python
./tools/gyp/buildbot/buildbot_run.py:#!/usr/bin/env python
./tools/gyp/gyp_main.py:#!/usr/bin/env python
./tools/gyp/tools/pretty_sln.py:#!/usr/bin/env python
./tools/gyp/tools/pretty_gyp.py:#!/usr/bin/env python
./tools/gyp/tools/graphviz.py:#!/usr/bin/env python
./tools/gyp/tools/pretty_vcproj.py:#!/usr/bin/env python
./tools/gyp/gyptest.py:#!/usr/bin/env python
./tools/run-valgrind.py:#!/usr/bin/env python
./tools/icu/shrink-icu-src.py:#!/usr/bin/env python
./tools/compress_json.py:#!/usr/bin/env python
./tools/gyp_node.py:#!/usr/bin/env python
./tools/test.py:#!/usr/bin/env python
./tools/genv8constants.py:#!/usr/bin/env python
./tools/check-imports.py:#!/usr/bin/env python
./tools/specialize_node_d.py:#!/usr/bin/env python
./configure:#!/usr/bin/env python

The Archlinux PKGBUILD just applies a sed command to the whole source tree:

https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/nodejs#n27

I agree the solution I proposed isn't pretty, and I wouldn't want to apply it to many files. It also looks like a lot of these are in dependencies that node pulls in in-tree? Probably don't want to be patching those.

This probably ought to be re-opened, since the problem persists. I don't have another solution immediately popping into my head, but will think on the matter.

@jbergstroem
Copy link
Member

@zenhack: only a small fraction of those files are used in build. The problem is still that python3 default installs (or python3 itself) is supported, so I think that operating systems/package managers that runs python3 as default (kudos btw) are better off handling the issue.

@gibfahn
Copy link
Member

gibfahn commented Nov 30, 2016

If ./configure were to create a symlink called python that pointed towards whatever was used to run it, and were to prepend the directory containing it to the path, then that would work right?

@zenhack
Copy link

zenhack commented Nov 30, 2016

@gibfahn, I think that would do the right thing.

Re: having systems handle the issue, I really don't think that's the right place in the stack to fix this. Arch right now has to patch a really excessive number of packages because of this kind of thing, and any other system that wants to default to python 3 will have to do the same. Doing it upstream seems much more sustainable.

@zenhack
Copy link

zenhack commented Nov 30, 2016

(The most elegant solution would be to write portable python, but that's a bit more work from where we are now -- though it's not hard when writing new code).

@silverwind
Copy link
Contributor

What distribution is that that doesn't provide a python (symlink or not)?

@gibfahn
Copy link
Member

gibfahn commented Nov 30, 2016

@silverwind I think the problem is that if which python points to Python 3, then even if you run the main configure file with python2 configure, when subshells are spawned with the #!/usr/bin/env python, then they end up still running with python3, which doesn't work.

@zenhack
Copy link

zenhack commented Dec 1, 2016 via email

@gunar
Copy link
Contributor

gunar commented Feb 14, 2017

For future reference, here's how I got it to work on Manjaro (~Arch):

mkdir /home/gcg/python2
ln -s /usr/bin/python2 /home/gcg/python2/python

Then I run tests with

PATH=/home/gcg/python2:$PATH make -j4 test

(credits go to @zenhack)

@jbergstroem
Copy link
Member

jbergstroem commented Feb 14, 2017

@gunar: try PYTHON=/usr/bin/python2 ./configure instead.

@silverwind
Copy link
Contributor

@jbergstroem maybe we should add this info on the error print?

sys.stdout.write("Please use either Python 2.6 or 2.7\n")

@jbergstroem
Copy link
Member

@silverwind sounds good to me!

@gunar
Copy link
Contributor

gunar commented Feb 14, 2017

@jbergstroem your method did not work :-(

> PYTHON=/usr/bin/python2 ./configure
Please use either Python 2.6 or 2.7
> uname -a
Linux zeno 4.4.45-1-MANJARO #1 SMP PREEMPT Thu Jan 26 11:32:11 UTC 2017 x86_64 GNU/Linux

But thanks!

@silverwind
Copy link
Contributor

Hmm, where's this PYTHON variable actually documented? Can't find it:

https://docs.python.org/3.7/using/cmdline.html#environment-variables

@jbergstroem
Copy link
Member

Sorry; its used in Makefile.

@silverwind
Copy link
Contributor

So we need to suggest PYTHON=path make too, right?

@jbergstroem
Copy link
Member

@silverwind said:
So we need to suggest PYTHON=path make too, right?

No; passing PYTHON=path to configure should write it to config.mk which gets included in Makefile. I was just providing a poor example.

@silverwind
Copy link
Contributor

Alright, so PYTHON=/usr/bin/python2 /usr/bin/python2 configure? 😆

@silverwind
Copy link
Contributor

Nevermind, I see 'PYTHON': sys.executable is already handed down. I'll land #11375 later as-is.

@Siecje
Copy link

Siecje commented Oct 23, 2017

EDIT: I had a problem but when I updated to the latest 8.7.0 code it works.

@gibfahn
Copy link
Member

gibfahn commented Oct 23, 2017

If ./configure were to create a symlink called python that pointed towards whatever was used to run it, and were to prepend the directory containing it to the path, then that would work right?

So this was implemented in #16058, so this should now finally work without having to set PYTHON, even if your python is a python3 (as long as you have a python2 on your system).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

10 participants