From ea0e0a06a0723ef0336ca309c877b5df46abe641 Mon Sep 17 00:00:00 2001 From: Zhaoqi Xu Date: Sat, 10 Oct 2026 10:48:19 +0800 Subject: [PATCH 1/3] fix(nodeenv): do not require python2 to build node from source build_node_from_src() raised OSError whenever nodeenv ran on Python 3 and no python2 command was on PATH, and otherwise prepended a tmpbin dir that points `python` at python2. node.js has built with Python 3 for years and its ./configure picks python3.x on its own, so drop the shim and let the build use the inherited environment. Fixes #419. --- nodeenv.py | 26 +++----------------------- tests/nodeenv_test.py | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/nodeenv.py b/nodeenv.py index eb3d571..e5c843e 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -1066,7 +1066,6 @@ def copy_node_from_prebuilt(env_dir, src_dir, node_version): def build_node_from_src(env_dir, src_dir, node_src_dir, args): - env = {} make_param_names = ['load-average', 'jobs'] make_param_values = map( lambda x: getattr(args, x.replace('-', '_')), @@ -1078,25 +1077,6 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): if value is not None ] - if getattr(sys.version_info, 'major', sys.version_info[0]) > 2: - # Currently, the node.js build scripts are using python2.*, - # therefore we need to temporarily point python exec to the - # python 2.* version in this case. - python2_path = shutil.which('python2') - if not python2_path: - raise OSError( - 'Python >=3.0 virtualenv detected, but no python2 ' - 'command (required for building node.js) was found' - ) - logger.debug(' * Temporarily pointing python to %s', python2_path) - node_tmpbin_dir = join(src_dir, 'tmpbin') - node_tmpbin_link = join(node_tmpbin_dir, 'python') - mkdir(node_tmpbin_dir) - if not os.path.exists(node_tmpbin_link): - callit(['ln', '-s', python2_path, node_tmpbin_link]) - env['PATH'] = '{}:{}'.format(node_tmpbin_dir, - os.environ.get('PATH', '')) - conf_cmd = [ './configure', '--prefix=%s' % _quote(env_dir) @@ -1110,11 +1090,11 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): make_cmd = args.make_path - callit(conf_cmd, args.verbose, True, node_src_dir, env) + callit(conf_cmd, args.verbose, True, node_src_dir) logger.info('.', extra=dict(continued=True)) - callit([make_cmd] + make_opts, args.verbose, True, node_src_dir, env) + callit([make_cmd] + make_opts, args.verbose, True, node_src_dir) logger.info('.', extra=dict(continued=True)) - callit([make_cmd + ' install'], args.verbose, True, node_src_dir, env) + callit([make_cmd + ' install'], args.verbose, True, node_src_dir) def install_node(env_dir, src_dir, args): diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index d79a599..802b995 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -2902,3 +2902,26 @@ def test_append_goes_before_a_powershell_signature(self, tmpdir): '# SIG # Begin signature block\n' '# MIIF...\n' '# SIG # End signature block\n') + + +def test_build_node_from_src_does_not_need_python2(tmpdir): + """ + node.js build scripts run on Python 3, so a source build must not + require a python2 command or rewrite PATH to point at one + """ + args = mock.Mock(load_average=None, jobs='2', without_ssl=False, + debug=False, profile=False, make_path='make', + verbose=False) + src_dir = tmpdir.join('src') + node_src_dir = src_dir.join('node-src') + with mock.patch.object(nodeenv.shutil, 'which', return_value=None), \ + mock.patch.object(nodeenv, 'callit') as mck: + nodeenv.build_node_from_src( + str(tmpdir.join('env')), str(src_dir), str(node_src_dir), args) + + assert [c[0][0][0] for c in mck.call_args_list] == [ + './configure', 'make', 'make install'] + for c in mck.call_args_list: + assert c[0][3] == str(node_src_dir) + assert len(c[0]) < 5 or not c[0][4] + assert not src_dir.join('tmpbin').exists() From 9bb0da62667060f82727b0babad11ad80fdad83f Mon Sep 17 00:00:00 2001 From: Zhaoqi Xu Date: Tue, 13 Oct 2026 04:58:44 +0800 Subject: [PATCH 2/3] fix(nodeenv): keep the python2 shim for node.js before 8.5 Before 8.5, node's ./configure is a `#!/usr/bin/env python` script that only runs on Python 2, so source builds of those versions (e.g. the README's 0.10.25 example) still need `python` pointed at python2. Only set up the tmpbin link and PATH override when args.node < 8.5, and keep a clear OSError naming the version when python2 is missing. Tests cover a current version with python2 on PATH (no tmpbin, no extra_env, positional or keyword), an old version with python2 (tmpbin link created, PATH starts with it) and an old version without python2. Add a CHANGES entry for #419. --- CHANGES | 5 +++ nodeenv.py | 25 ++++++++++++-- tests/nodeenv_test.py | 77 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 91 insertions(+), 16 deletions(-) diff --git a/CHANGES b/CHANGES index 3e71939..0f125df 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,11 @@ Nodeenv changelog Version [unreleased] -------------------- +- `--source` builds of node.js 8.5 and later no longer need a `python2` + command: their `./configure` finds a Python on its own, so the `python` + to python2 link in `PATH` is only set up for older versions, which still + fail early with a clear error when `python2` is missing + `#419 `_ - The node.js archive download also retries when the connection is reset or times out in the middle of the body, pauses between the attempts and ends with an error message instead of a traceback when all of them fail; a diff --git a/nodeenv.py b/nodeenv.py index e5c843e..d25cdfa 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -1066,6 +1066,7 @@ def copy_node_from_prebuilt(env_dir, src_dir, node_version): def build_node_from_src(env_dir, src_dir, node_src_dir, args): + env = {} make_param_names = ['load-average', 'jobs'] make_param_values = map( lambda x: getattr(args, x.replace('-', '_')), @@ -1077,6 +1078,24 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): if value is not None ] + if parse_version(args.node) < (8, 5): + # before 8.5, ./configure is a `#!/usr/bin/env python` script + # that only runs on Python 2, so point `python` at python2 + python2_path = shutil.which('python2') + if not python2_path: + raise OSError( + 'no python2 command (required for building ' + 'node.js %s) was found' % args.node + ) + logger.debug(' * Temporarily pointing python to %s', python2_path) + node_tmpbin_dir = join(src_dir, 'tmpbin') + node_tmpbin_link = join(node_tmpbin_dir, 'python') + mkdir(node_tmpbin_dir) + if not os.path.exists(node_tmpbin_link): + callit(['ln', '-s', python2_path, node_tmpbin_link]) + env['PATH'] = '{}:{}'.format(node_tmpbin_dir, + os.environ.get('PATH', '')) + conf_cmd = [ './configure', '--prefix=%s' % _quote(env_dir) @@ -1090,11 +1109,11 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): make_cmd = args.make_path - callit(conf_cmd, args.verbose, True, node_src_dir) + callit(conf_cmd, args.verbose, True, node_src_dir, env) logger.info('.', extra=dict(continued=True)) - callit([make_cmd] + make_opts, args.verbose, True, node_src_dir) + callit([make_cmd] + make_opts, args.verbose, True, node_src_dir, env) logger.info('.', extra=dict(continued=True)) - callit([make_cmd + ' install'], args.verbose, True, node_src_dir) + callit([make_cmd + ' install'], args.verbose, True, node_src_dir, env) def install_node(env_dir, src_dir, args): diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index 802b995..13fdb2c 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -2904,24 +2904,75 @@ def test_append_goes_before_a_powershell_signature(self, tmpdir): '# SIG # End signature block\n') -def test_build_node_from_src_does_not_need_python2(tmpdir): - """ - node.js build scripts run on Python 3, so a source build must not - require a python2 command or rewrite PATH to point at one - """ - args = mock.Mock(load_average=None, jobs='2', without_ssl=False, - debug=False, profile=False, make_path='make', - verbose=False) +def _build_node_from_src(tmpdir, node_version, python2_path): + args = mock.Mock(node=node_version, load_average=None, jobs='2', + without_ssl=False, debug=False, profile=False, + make_path='make', verbose=False) src_dir = tmpdir.join('src') - node_src_dir = src_dir.join('node-src') - with mock.patch.object(nodeenv.shutil, 'which', return_value=None), \ + node_src_dir = src_dir.join('node-v%s' % node_version) + with mock.patch.object(nodeenv.shutil, 'which', + return_value=python2_path), \ mock.patch.object(nodeenv, 'callit') as mck: nodeenv.build_node_from_src( str(tmpdir.join('env')), str(src_dir), str(node_src_dir), args) + return src_dir, node_src_dir, mck.call_args_list + + +def _build_env(call): + """extra_env passed to callit, positionally or as a keyword""" + args, kwargs = call + if 'extra_env' in kwargs: + return kwargs['extra_env'] + return args[4] if len(args) > 4 else None + + +def _build_calls(calls): + return [c for c in calls if c[0][0][0] != 'ln'] + + +@pytest.mark.parametrize('node_version', ['8.5.0', '22.11.0']) +def test_build_node_from_src_current_node_ignores_python2(tmpdir, + node_version): + """ + since 8.5, node's ./configure finds a Python on its own, so a source + build neither needs a python2 command nor rewrites PATH, even when + python2 is installed + """ + src_dir, node_src_dir, calls = _build_node_from_src( + tmpdir, node_version, '/usr/bin/python2') - assert [c[0][0][0] for c in mck.call_args_list] == [ + assert [c[0][0][0] for c in calls] == [ './configure', 'make', 'make install'] - for c in mck.call_args_list: + for c in calls: assert c[0][3] == str(node_src_dir) - assert len(c[0]) < 5 or not c[0][4] + assert not _build_env(c) assert not src_dir.join('tmpbin').exists() + + +def test_build_node_from_src_old_node_points_python_at_python2(tmpdir): + """ + before 8.5, ./configure only runs on Python 2, so `python` in PATH + is pointed at python2 for the build + """ + src_dir, node_src_dir, calls = _build_node_from_src( + tmpdir, '0.10.25', '/usr/bin/python2') + + tmpbin = src_dir.join('tmpbin') + assert tmpbin.isdir() + assert calls[0][0][0] == [ + 'ln', '-s', '/usr/bin/python2', str(tmpbin.join('python'))] + build = _build_calls(calls) + assert [c[0][0][0] for c in build] == [ + './configure', 'make', 'make install'] + for c in build: + assert c[0][3] == str(node_src_dir) + assert _build_env(c)['PATH'].startswith(str(tmpbin) + ':') + + +def test_build_node_from_src_old_node_without_python2(tmpdir): + """ + building node.js before 8.5 without python2 fails early with a clear + error instead of a failed ./configure + """ + with pytest.raises(OSError, match='python2.*0.10.25'): + _build_node_from_src(tmpdir, '0.10.25', None) From 979d8831e756ca586745a5cb14d16089e3d46170 Mon Sep 17 00:00:00 2001 From: Zhaoqi Xu Date: Wed, 14 Oct 2026 02:06:27 +0800 Subject: [PATCH 3/3] fix(nodeenv): link python to python2 before 8.8, require it before 13 The 8.5 cutoff only looked at the first line of ./configure. Before 8.8 gyp and make still run `python` by name (toolchain.gypi during configure, the js2c actions during make) and node does not point it at Python 2 itself: 8.5/8.6 have no override and 8.7's get_bin_override() crashes when there is no `python` at all. So keep the tmpbin link for < 8.8. Up to 12.x ./configure only accepts Python 2.7, so 8.8-12.x still need python2 even though they find it on their own; keep the early "no python2" OSError for < 13 so a missing python2 is not reported as a bare `./configure ... failed` without --verbose. Tests: 8.8.0/12.22.12/22.11.0 with python2 get no link, 13.0.0/22.11.0 build without python2, 0.10.25/8.5.0/8.7.0 get the link, and 0.10.25/8.7.0/12.22.12 without python2 raise OSError. Fix the CHANGES entry accordingly. --- CHANGES | 9 +++++---- nodeenv.py | 27 +++++++++++++++----------- tests/nodeenv_test.py | 45 ++++++++++++++++++++++++++++++------------- 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/CHANGES b/CHANGES index 0f125df..59b4974 100644 --- a/CHANGES +++ b/CHANGES @@ -4,10 +4,11 @@ Nodeenv changelog Version [unreleased] -------------------- -- `--source` builds of node.js 8.5 and later no longer need a `python2` - command: their `./configure` finds a Python on its own, so the `python` - to python2 link in `PATH` is only set up for older versions, which still - fail early with a clear error when `python2` is missing +- `--source` builds of node.js 13 and later no longer need a `python2` + command; 8.8 to 12.x still need one but point `python` at it on their + own, so the `python` to python2 link in `PATH` is only set up for older + versions, and versions before 13 still fail early with a clear error + when `python2` is missing `#419 `_ - The node.js archive download also retries when the connection is reset or times out in the middle of the body, pauses between the attempts and ends diff --git a/nodeenv.py b/nodeenv.py index d25cdfa..b33829a 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -1078,23 +1078,28 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): if value is not None ] - if parse_version(args.node) < (8, 5): - # before 8.5, ./configure is a `#!/usr/bin/env python` script - # that only runs on Python 2, so point `python` at python2 + node_version = parse_version(args.node) + if node_version < (13, 0): + # up to 12.x, ./configure only accepts Python 2.7, so fail early + # with a clear error instead of a failed ./configure python2_path = shutil.which('python2') if not python2_path: raise OSError( 'no python2 command (required for building ' 'node.js %s) was found' % args.node ) - logger.debug(' * Temporarily pointing python to %s', python2_path) - node_tmpbin_dir = join(src_dir, 'tmpbin') - node_tmpbin_link = join(node_tmpbin_dir, 'python') - mkdir(node_tmpbin_dir) - if not os.path.exists(node_tmpbin_link): - callit(['ln', '-s', python2_path, node_tmpbin_link]) - env['PATH'] = '{}:{}'.format(node_tmpbin_dir, - os.environ.get('PATH', '')) + if node_version < (8, 8): + # before 8.8, gyp and make run `python` by name and node does + # not point it at Python 2 itself, so point `python` at python2 + logger.debug(' * Temporarily pointing python to %s', + python2_path) + node_tmpbin_dir = join(src_dir, 'tmpbin') + node_tmpbin_link = join(node_tmpbin_dir, 'python') + mkdir(node_tmpbin_dir) + if not os.path.exists(node_tmpbin_link): + callit(['ln', '-s', python2_path, node_tmpbin_link]) + env['PATH'] = '{}:{}'.format(node_tmpbin_dir, + os.environ.get('PATH', '')) conf_cmd = [ './configure', diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index 13fdb2c..0badd52 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -2930,13 +2930,12 @@ def _build_calls(calls): return [c for c in calls if c[0][0][0] != 'ln'] -@pytest.mark.parametrize('node_version', ['8.5.0', '22.11.0']) -def test_build_node_from_src_current_node_ignores_python2(tmpdir, - node_version): +@pytest.mark.parametrize('node_version', ['8.8.0', '12.22.12', '22.11.0']) +def test_build_node_from_src_current_node_does_not_link_python(tmpdir, + node_version): """ - since 8.5, node's ./configure finds a Python on its own, so a source - build neither needs a python2 command nor rewrites PATH, even when - python2 is installed + since 8.8, node points `python` at the right interpreter itself, so a + source build does not rewrite PATH, even when python2 is installed """ src_dir, node_src_dir, calls = _build_node_from_src( tmpdir, node_version, '/usr/bin/python2') @@ -2949,13 +2948,32 @@ def test_build_node_from_src_current_node_ignores_python2(tmpdir, assert not src_dir.join('tmpbin').exists() -def test_build_node_from_src_old_node_points_python_at_python2(tmpdir): +@pytest.mark.parametrize('node_version', ['13.0.0', '22.11.0']) +def test_build_node_from_src_node_13_does_not_need_python2(tmpdir, + node_version): """ - before 8.5, ./configure only runs on Python 2, so `python` in PATH + since 13, ./configure also runs on Python 3, so a source build works + without a python2 command + """ + src_dir, node_src_dir, calls = _build_node_from_src( + tmpdir, node_version, None) + + assert [c[0][0][0] for c in calls] == [ + './configure', 'make', 'make install'] + for c in calls: + assert not _build_env(c) + assert not src_dir.join('tmpbin').exists() + + +@pytest.mark.parametrize('node_version', ['0.10.25', '8.5.0', '8.7.0']) +def test_build_node_from_src_old_node_points_python_at_python2(tmpdir, + node_version): + """ + before 8.8, gyp and make run `python` by name, so `python` in PATH is pointed at python2 for the build """ src_dir, node_src_dir, calls = _build_node_from_src( - tmpdir, '0.10.25', '/usr/bin/python2') + tmpdir, node_version, '/usr/bin/python2') tmpbin = src_dir.join('tmpbin') assert tmpbin.isdir() @@ -2969,10 +2987,11 @@ def test_build_node_from_src_old_node_points_python_at_python2(tmpdir): assert _build_env(c)['PATH'].startswith(str(tmpbin) + ':') -def test_build_node_from_src_old_node_without_python2(tmpdir): +@pytest.mark.parametrize('node_version', ['0.10.25', '8.7.0', '12.22.12']) +def test_build_node_from_src_old_node_without_python2(tmpdir, node_version): """ - building node.js before 8.5 without python2 fails early with a clear + building node.js before 13 without python2 fails early with a clear error instead of a failed ./configure """ - with pytest.raises(OSError, match='python2.*0.10.25'): - _build_node_from_src(tmpdir, '0.10.25', None) + with pytest.raises(OSError, match='python2.*' + node_version): + _build_node_from_src(tmpdir, node_version, None)