From ae1f32c06c7f6467d58854d16074ab93abf068d1 Mon Sep 17 00:00:00 2001 From: fly1d <309400591+fly1d@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:46:01 +0800 Subject: [PATCH] fix(nodeenv): ignore source trees during prebuilt installs --- CHANGES | 3 ++ nodeenv.py | 6 +-- tests/nodeenv_test.py | 123 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 118 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index c42af9e..fc04c94 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,9 @@ Nodeenv changelog Version [unreleased] -------------------- +- `--prebuilt` installs no longer mistake a cached source tree for a + prebuilt archive, or fail when both trees remain in `src/` + `#443 `_ - `nodeenv -p` no longer puts the python venv's `bin` directory on `PATH` a second time: the venv's own `activate` and `activate.fish` put it there already. Envs without those scripts, such as conda envs, and the git-bash diff --git a/nodeenv.py b/nodeenv.py index c643526..377ec54 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -1013,7 +1013,7 @@ def copy_node_from_prebuilt(env_dir, src_dir, node_version): else: dest = env_dir - src_folder_tpl = src_dir + '/node-v%s*' % node_version + src_folder_tpl = src_dir + '/node-v%s-*' % node_version src_folder, = glob.glob(src_folder_tpl) copytree(src_folder, dest, True) @@ -1141,8 +1141,8 @@ def install_node_wrapped(env_dir, src_dir, args): # if arm64 not found, try x64 node_urls.append(node_url.replace('arm64', 'x64')) - # get src if not downloaded yet - if not os.path.exists(node_src_dir): + # Only source builds can reuse the source directory. + if args.prebuilt or not os.path.exists(node_src_dir): errors = [] # retry outside the handler, so that a second failure is not # reported as "during handling of the above exception" diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index 5ee988b..ab61c7a 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -1841,6 +1841,107 @@ def test_uppercase_machine_amd64(self): assert url == expected +@pytest.mark.parametrize('source_exists', [False, True]) +@pytest.mark.parametrize('prebuilt_exists', [False, True]) +def test_prebuilt_ignores_source_cache( + tmp_path, monkeypatch, source_exists, prebuilt_exists): + args = mock.Mock(node='18.0.0', prebuilt=True, verbose=False) + env_dir = tmp_path / 'env' + src_dir = tmp_path / 'src' + src_dir.mkdir() + source = src_dir / 'node-v18.0.0' + if source_exists: + source.mkdir() + (source / 'source-only').write_text('source', encoding='utf-8') + prebuilt = src_dir / 'node-v18.0.0-linux-x64' + + def write_binary(contents): + (prebuilt / 'bin').mkdir(parents=True, exist_ok=True) + (prebuilt / 'bin' / 'node').write_text(contents, encoding='utf-8') + + if prebuilt_exists: + write_binary('cached prebuilt') + url = 'https://example.com/node-v18.0.0-linux-x64.tar.gz' + monkeypatch.setattr(nodeenv, 'get_node_bin_url', lambda version: url) + monkeypatch.setattr(nodeenv, 'is_WIN', False) + monkeypatch.setattr(nodeenv, 'is_CYGWIN', False) + + def download(url, directory, options): + write_binary('downloaded prebuilt') + + with mock.patch.object( + nodeenv, 'download_node_src', + side_effect=download) as mock_download: + nodeenv.install_node_wrapped(str(env_dir), str(src_dir), args) + + mock_download.assert_called_once_with(url, str(src_dir), args) + assert (env_dir / 'bin' / 'node').read_text() == 'downloaded prebuilt' + assert not (env_dir / 'source-only').exists() + if source_exists: + assert (source / 'source-only').read_text() == 'source' + + +@pytest.mark.parametrize('system, archive_platform, binary', [ + ('posix', 'linux-x64', 'bin/node'), + ('windows', 'win-x64', 'node.exe'), + ('cygwin', 'win-x64', 'node.exe'), +]) +def test_copy_prebuilt_ignores_source_tree( + tmp_path, monkeypatch, system, archive_platform, binary): + env_dir = tmp_path / 'env' + src_dir = tmp_path / 'src' + source = src_dir / 'node-v18.0.1' + source.mkdir(parents=True) + (source / 'source-only').write_text('source', encoding='utf-8') + for version, contents in [('18.0.1', 'prebuilt'), ('18.0.10', 'other')]: + prebuilt = src_dir / ('node-v%s-%s' % (version, archive_platform)) + node = prebuilt / binary + node.parent.mkdir(parents=True) + node.write_text(contents, encoding='utf-8') + monkeypatch.setattr(nodeenv, 'is_WIN', system == 'windows') + monkeypatch.setattr(nodeenv, 'is_CYGWIN', system == 'cygwin') + + nodeenv.copy_node_from_prebuilt(str(env_dir), str(src_dir), '18.0.1') + + if system == 'posix': + dest = env_dir + else: + dest = env_dir / ('Scripts' if system == 'windows' else 'bin') + assert (dest / binary).read_text() == 'prebuilt' + assert not (dest / 'source-only').exists() + assert (source / 'source-only').read_text() == 'source' + + +def test_arm64_fallback_copies_x64_prebuilt(tmp_path, monkeypatch): + args = mock.Mock(node='18.0.0', prebuilt=True, verbose=False) + env_dir = tmp_path / 'env' + src_dir = tmp_path / 'src' + src_dir.mkdir() + arm_url = 'https://example.com/node-v18.0.0-darwin-arm64.tar.gz' + x64_url = arm_url.replace('arm64', 'x64') + monkeypatch.setattr(nodeenv, 'get_node_bin_url', lambda version: arm_url) + monkeypatch.setattr(nodeenv, 'is_WIN', False) + monkeypatch.setattr(nodeenv, 'is_CYGWIN', False) + + def download(url, directory, options): + if url == arm_url: + raise nodeenv.urllib2.HTTPError(url, 404, 'Not Found', None, None) + node = src_dir / 'node-v18.0.0-darwin-x64' / 'bin' / 'node' + node.parent.mkdir(parents=True) + node.write_text('x64 prebuilt', encoding='utf-8') + + with mock.patch.object( + nodeenv, 'download_node_src', + side_effect=download) as mock_download: + nodeenv.install_node_wrapped(str(env_dir), str(src_dir), args) + + assert mock_download.call_args_list == [ + mock.call(arm_url, str(src_dir), args), + mock.call(x64_url, str(src_dir), args), + ] + assert (env_dir / 'bin' / 'node').read_text() == 'x64 prebuilt' + + class TestInstallNode: """Tests for install_node and install_node_wrapped functions""" @@ -2018,11 +2119,11 @@ def download_side_effect(url, src_dir, args): # Verify download was called only once (no fallback for x64) mock_download.assert_called_once() - def test_install_node_wrapped_skips_download_if_exists(self, tmpdir): - """Test that download is skipped if node source dir exists""" + def test_install_node_wrapped_reuses_cached_source(self, tmpdir): + """Source builds reuse an existing source directory.""" args = mock.Mock() args.node = '18.0.0' - args.prebuilt = True + args.prebuilt = False args.verbose = False env_dir = tmpdir.join('env').strpath @@ -2032,28 +2133,28 @@ def test_install_node_wrapped_skips_download_if_exists(self, tmpdir): node_src_dir = os.path.join(src_dir, 'node-v18.0.0') os.makedirs(node_src_dir) - bin_url = ( + src_url = ( 'https://nodejs.org/download/release/v18.0.0/' - 'node-v18.0.0-linux-x64.tar.gz' + 'node-v18.0.0.tar.gz' ) with mock.patch.object( - nodeenv, 'get_node_bin_url', - return_value=bin_url + nodeenv, 'get_node_src_url', + return_value=src_url ), \ mock.patch.object( nodeenv, 'download_node_src' ) as mock_download, \ mock.patch.object( - nodeenv, 'copy_node_from_prebuilt' - ) as mock_copy, \ + nodeenv, 'build_node_from_src' + ) as mock_build, \ mock.patch.object(nodeenv.logger, 'info'): nodeenv.install_node_wrapped(env_dir, src_dir, args) # Verify download was NOT called mock_download.assert_not_called() - # Verify copy was still called - mock_copy.assert_called_once() + mock_build.assert_called_once_with( + env_dir, src_dir, node_src_dir, args) def test_install_node_restores_newline_on_exception(self, tmpdir): """Test that install_node restores newline on exception"""