From a76c4d27675cf1ceb81aa4ad8f03b5d475a9fc99 Mon Sep 17 00:00:00 2001 From: jihtsan <15151999611@163.com> Date: Fri, 16 Oct 2026 11:03:42 +0800 Subject: [PATCH 1/3] fix: preserve Python command identity in legacy source builds Closes #435. --- CHANGES | 6 +++- nodeenv.py | 21 +++++++---- tests/nodeenv_test.py | 10 ++---- tests/test_build_python.py | 74 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 tests/test_build_python.py diff --git a/CHANGES b/CHANGES index 59b4974..faadfa2 100644 --- a/CHANGES +++ b/CHANGES @@ -4,9 +4,13 @@ Nodeenv changelog Version [unreleased] -------------------- +- Legacy source builds also find ``python2.7`` when ``python2`` is absent. + The temporary ``python`` launcher preserves shim command names and + relative PATH entries, and refreshes stale or dangling links on retries + `#435 `_ - `--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 + own, so the `python` to python2 shim 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 `_ diff --git a/nodeenv.py b/nodeenv.py index b33829a..cbc1083 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -1082,22 +1082,29 @@ def build_node_from_src(env_dir, src_dir, node_src_dir, args): 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') + python2_path = (shutil.which('python2') or + shutil.which('python2.7')) if not python2_path: raise OSError( - 'no python2 command (required for building ' + 'no python2 or python2.7 command (required for building ' 'node.js %s) was found' % args.node ) 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 + # not point it at Python 2 itself. Invoke the selected command + # by its own name so pyenv shims do not dispatch to `python`. logger.debug(' * Temporarily pointing python to %s', python2_path) - node_tmpbin_dir = join(src_dir, 'tmpbin') - node_tmpbin_link = join(node_tmpbin_dir, 'python') + node_tmpbin_dir = abspath(join(src_dir, 'tmpbin')) + node_tmpbin_python = 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]) + # Remove old links without following them, including dangling + # ones left by a failed build. Refresh the launcher on retries. + if os.path.islink(node_tmpbin_python): + os.unlink(node_tmpbin_python) + writefile(node_tmpbin_python, '#!/bin/sh\nexec %s "$@"\n' % + _quote(abspath(python2_path))) + make_executable(node_tmpbin_python) env['PATH'] = '{}:{}'.format(node_tmpbin_dir, os.environ.get('PATH', '')) diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index 0badd52..c7e75c8 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -2926,10 +2926,6 @@ def _build_env(call): 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.8.0', '12.22.12', '22.11.0']) def test_build_node_from_src_current_node_does_not_link_python(tmpdir, node_version): @@ -2977,9 +2973,9 @@ def test_build_node_from_src_old_node_points_python_at_python2(tmpdir, 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 tmpbin.join('python').read() == ( + '#!/bin/sh\nexec /usr/bin/python2 "$@"\n') + build = calls assert [c[0][0][0] for c in build] == [ './configure', 'make', 'make install'] for c in build: diff --git a/tests/test_build_python.py b/tests/test_build_python.py new file mode 100644 index 0000000..23d6396 --- /dev/null +++ b/tests/test_build_python.py @@ -0,0 +1,74 @@ +import os +from types import SimpleNamespace + +import pytest + +import nodeenv + + +pytestmark = pytest.mark.skipif( + nodeenv.is_WIN, reason='source builds use POSIX configure scripts') + + +def _script(path, contents): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text('#!/bin/sh\n' + contents) + path.chmod(0o755) + return path + + +def _build(tmp_path, node='8.7.0'): + source = tmp_path / 'src' / ('node-v' + node) + _script(source / 'configure', + 'exec python configure-argument "argument with spaces"\n') + make = _script(tmp_path / 'make', 'exit 0\n') + args = SimpleNamespace( + node=node, load_average=None, jobs='2', without_ssl=False, + debug=False, profile=False, make_path=str(make), verbose=False) + nodeenv.build_node_from_src( + str(tmp_path / 'env'), str(source.parent), str(source), args) + + +@pytest.mark.parametrize('case', [ + 'python2.7', 'dangling-link', 'stale-link', 'shim', 'relative-path', +]) +def test_legacy_build_selects_python_without_reusing_a_link( + tmp_path, monkeypatch, case): + tools = tmp_path / 'selected tools' + name = 'python2.7' if case == 'python2.7' else 'python2' + python = _script( + tools / name, + '[ "${0##*/}" = "' + name + '" ] || exit 19\n' + 'printf "%s\\n" "$0" "$@" >> "$PYTHON_LOG"\n') + log = tmp_path / 'python.log' + monkeypatch.setenv('PYTHON_LOG', str(log)) + monkeypatch.chdir(tmp_path) + path = str(tools) if case != 'relative-path' else tools.name + monkeypatch.setenv('PATH', path + os.pathsep + '/usr/bin:/bin') + + launcher = tmp_path / 'src' / 'tmpbin' / 'python' + old = tmp_path / 'old' / 'python2' + if case in ('stale-link', 'dangling-link'): + launcher.parent.mkdir(parents=True) + if case == 'stale-link': + _script(old, 'exit 23\n') + launcher.symlink_to(old) + + _build(tmp_path) + + assert log.read_text().splitlines() == [ + str(python), 'configure-argument', 'argument with spaces'] + if case == 'stale-link': + assert old.read_text() == '#!/bin/sh\nexit 23\n' + + +def test_legacy_build_refreshes_launcher_on_retry(tmp_path, monkeypatch): + log = tmp_path / 'python.log' + monkeypatch.setenv('PYTHON_LOG', str(log)) + for label in ('first', 'second'): + tools = tmp_path / label + _script(tools / 'python2', + 'printf "%s\\n" "' + label + '" >> "$PYTHON_LOG"\n') + monkeypatch.setenv('PATH', str(tools) + os.pathsep + '/usr/bin:/bin') + _build(tmp_path) + assert log.read_text().splitlines() == ['first', 'second'] From d91e720c2648bf4d9cedc5831769a0a6c45dfbfa Mon Sep 17 00:00:00 2001 From: jihtsan <15151999611@163.com> Date: Fri, 16 Oct 2026 12:08:11 +0800 Subject: [PATCH 2/3] Make launcher assertions portable across host paths --- tests/nodeenv_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index c7e75c8..db08de1 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -13,6 +13,7 @@ import os.path import pathlib import shutil +import shlex import socket import subprocess import sys @@ -2968,13 +2969,15 @@ def test_build_node_from_src_old_node_points_python_at_python2(tmpdir, before 8.8, gyp and make run `python` by name, so `python` in PATH is pointed at python2 for the build """ + python2_path = str(tmpdir.join('selected tools', 'python2')) src_dir, node_src_dir, calls = _build_node_from_src( - tmpdir, node_version, '/usr/bin/python2') + tmpdir, node_version, python2_path) tmpbin = src_dir.join('tmpbin') assert tmpbin.isdir() - assert tmpbin.join('python').read() == ( - '#!/bin/sh\nexec /usr/bin/python2 "$@"\n') + shebang, command = tmpbin.join('python').read().splitlines() + assert shebang == '#!/bin/sh' + assert shlex.split(command) == ['exec', python2_path, '$@'] build = calls assert [c[0][0][0] for c in build] == [ './configure', 'make', 'make install'] From 6e5cf06cfacd6a333dc7b85866c76e5052f7630c Mon Sep 17 00:00:00 2001 From: jihtsan <15151999611@163.com> Date: Mon, 26 Oct 2026 09:22:24 +0800 Subject: [PATCH 3/3] test(nodeenv): isolate legacy Python selection tests for #435 --- CHANGES | 11 +++++++---- tests/test_build_python.py | 5 ++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index faadfa2..f40c337 100644 --- a/CHANGES +++ b/CHANGES @@ -4,13 +4,16 @@ Nodeenv changelog Version [unreleased] -------------------- -- Legacy source builds also find ``python2.7`` when ``python2`` is absent. - The temporary ``python`` launcher preserves shim command names and - relative PATH entries, and refreshes stale or dangling links on retries +- `--source` builds of node.js before 13 also accept `python2.7` when there + is no `python2`. For node.js before 8.8, `python` in `PATH` is now a small + launcher script instead of a link: it runs the selected python2 by its + absolute path and its own name, so a pyenv shim or a relative `PATH` entry + works, and a stale or dangling link left by an earlier run is replaced + instead of reused `#435 `_ - `--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 shim in `PATH` is only set up for older + own, so the `python` to python2 launcher 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 `_ diff --git a/tests/test_build_python.py b/tests/test_build_python.py index 23d6396..0ae26c4 100644 --- a/tests/test_build_python.py +++ b/tests/test_build_python.py @@ -1,4 +1,3 @@ -import os from types import SimpleNamespace import pytest @@ -44,7 +43,7 @@ def test_legacy_build_selects_python_without_reusing_a_link( monkeypatch.setenv('PYTHON_LOG', str(log)) monkeypatch.chdir(tmp_path) path = str(tools) if case != 'relative-path' else tools.name - monkeypatch.setenv('PATH', path + os.pathsep + '/usr/bin:/bin') + monkeypatch.setenv('PATH', path) launcher = tmp_path / 'src' / 'tmpbin' / 'python' old = tmp_path / 'old' / 'python2' @@ -69,6 +68,6 @@ def test_legacy_build_refreshes_launcher_on_retry(tmp_path, monkeypatch): tools = tmp_path / label _script(tools / 'python2', 'printf "%s\\n" "' + label + '" >> "$PYTHON_LOG"\n') - monkeypatch.setenv('PATH', str(tools) + os.pathsep + '/usr/bin:/bin') + monkeypatch.setenv('PATH', str(tools)) _build(tmp_path) assert log.read_text().splitlines() == ['first', 'second']