From 7a04652a6af9010c639631d5475d1868e84ff8a6 Mon Sep 17 00:00:00 2001 From: Elijah Ahianyo Date: Thu, 9 Nov 2023 21:01:48 +0000 Subject: [PATCH] Revert "Bun as runtime on Mac and Linux (#2138)" (#2153) --- reflex/constants/__init__.py | 4 - reflex/constants/base.py | 2 - reflex/utils/exec.py | 17 +-- reflex/utils/prerequisites.py | 215 ++++++++++++++++------------------ reflex/utils/processes.py | 17 ++- tests/test_prerequisites.py | 32 +---- tests/utils/test_utils.py | 77 +++++++++++- 7 files changed, 194 insertions(+), 170 deletions(-) diff --git a/reflex/constants/__init__.py b/reflex/constants/__init__.py index a467d2322..628f7511b 100644 --- a/reflex/constants/__init__.py +++ b/reflex/constants/__init__.py @@ -2,8 +2,6 @@ from .base import ( COOKIES, - IS_LINUX, - IS_LINUX_OR_MAC, IS_WINDOWS, LOCAL_STORAGE, POLLING_MAX_HTTP_BUFFER_SIZE, @@ -71,8 +69,6 @@ __ALL__ = [ Fnm, GitIgnore, RequirementsTxt, - IS_LINUX_OR_MAC, - IS_LINUX, IS_WINDOWS, LOCAL_STORAGE, LogLevel, diff --git a/reflex/constants/base.py b/reflex/constants/base.py index ad32fa23b..8957edfb7 100644 --- a/reflex/constants/base.py +++ b/reflex/constants/base.py @@ -11,8 +11,6 @@ from types import SimpleNamespace from platformdirs import PlatformDirs IS_WINDOWS = platform.system() == "Windows" -IS_LINUX_OR_MAC = platform.system() in ["Linux", "Darwin"] -IS_LINUX = platform.system() == "Linux" class Dirs(SimpleNamespace): diff --git a/reflex/utils/exec.py b/reflex/utils/exec.py index 24f6ce631..e63f1e718 100644 --- a/reflex/utils/exec.py +++ b/reflex/utils/exec.py @@ -116,7 +116,7 @@ def run_frontend(root: Path, port: str): # Start watching asset folder. start_watching_assets_folder(root) # validate dependencies before run - prerequisites.validate_frontend_dependencies() + prerequisites.validate_frontend_dependencies(init=False) # Run the frontend in development mode. console.rule("[bold green]App Running") @@ -134,7 +134,7 @@ def run_frontend_prod(root: Path, port: str): # Set the port. os.environ["PORT"] = str(get_config().frontend_port if port is None else port) # validate dependencies before run - prerequisites.validate_frontend_dependencies() + prerequisites.validate_frontend_dependencies(init=False) # Run the frontend in production mode. console.rule("[bold green]App Running") run_process_and_launch_url([prerequisites.get_package_manager(), "run", "prod"]) # type: ignore @@ -239,20 +239,21 @@ def output_system_info(): dependencies = [ f"[Reflex {constants.Reflex.VERSION} with Python {platform.python_version()} (PATH: {sys.executable})]", + f"[Node {prerequisites.get_node_version()} (Expected: {constants.Node.VERSION}) (PATH:{path_ops.get_node_path()})]", ] system = platform.system() - if system == "Windows" or constants.IS_LINUX and not prerequisites.is_valid_linux(): + + if system != "Windows": dependencies.extend( [ - f"[Node {prerequisites.get_node_version()} (Expected: {constants.Node.VERSION}) (PATH:{path_ops.get_node_path()})]", f"[FNM {prerequisites.get_fnm_version()} (Expected: {constants.Fnm.VERSION}) (PATH: {constants.Fnm.EXE})]", - ] + f"[Bun {prerequisites.get_bun_version()} (Expected: {constants.Bun.VERSION}) (PATH: {config.bun_path})]", + ], ) - - if system != "Windows" or constants.IS_LINUX and not prerequisites.is_valid_linux(): + else: dependencies.append( - f"[Bun {prerequisites.get_bun_version()} (Expected: {constants.Bun.VERSION}) (PATH: {config.bun_path})]", + f"[FNM {prerequisites.get_fnm_version()} (Expected: {constants.Fnm.VERSION}) (PATH: {constants.Fnm.EXE})]", ) if system == "Linux": diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index efab097fa..ec7d81ee1 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -29,6 +29,23 @@ from reflex.config import Config, get_config from reflex.utils import console, path_ops, processes +def check_node_version() -> bool: + """Check the version of Node.js. + + Returns: + Whether the version of Node.js is valid. + """ + current_version = get_node_version() + if current_version: + # Compare the version numbers + return ( + current_version >= version.parse(constants.Node.MIN_VERSION) + if constants.IS_WINDOWS + else current_version == version.parse(constants.Node.VERSION) + ) + return False + + def get_node_version() -> version.Version | None: """Get the version of node. @@ -70,35 +87,24 @@ def get_bun_version() -> version.Version | None: return None -def get_package_manager() -> str | None: +def get_install_package_manager() -> str | None: """Get the package manager executable for installation. Currently on unix systems, bun is used for installation only. Returns: The path to the package manager. """ - # On Windows or lower linux kernels(WSL1), we use npm instead of bun. - if constants.IS_WINDOWS or constants.IS_LINUX and not is_valid_linux(): - return get_npm_package_manager() + # On Windows, we use npm instead of bun. + if constants.IS_WINDOWS: + return get_package_manager() # On other platforms, we use bun. return get_config().bun_path -def get_install_package_manager() -> str | None: - """Get package manager to install dependencies. - - Returns: - Path to install package manager. - """ - if constants.IS_WINDOWS: - return get_npm_package_manager() - return get_config().bun_path - - -def get_npm_package_manager() -> str | None: - """Get the npm package manager executable for installing and running app - on windows. +def get_package_manager() -> str | None: + """Get the package manager executable for running app. + Currently on unix systems, npm is used for running the app only. Returns: The path to the package manager. @@ -354,6 +360,13 @@ def update_next_config(next_config: str, config: Config) -> str: return next_config +def remove_existing_bun_installation(): + """Remove existing bun installation.""" + console.debug("Removing existing bun installation.") + if os.path.exists(get_config().bun_path): + path_ops.rm(constants.Bun.ROOT_PATH) + + def download_and_run(url: str, *args, show_status: bool = False, **env): """Download and run a script. @@ -415,38 +428,52 @@ def download_and_extract_fnm_zip(): def install_node(): - """Install fnm and nodejs for use by Reflex.""" - if constants.IS_WINDOWS or constants.IS_LINUX and not is_valid_linux(): + """Install fnm and nodejs for use by Reflex. + Independent of any existing system installations. + """ + if not constants.Fnm.FILENAME: + # fnm only support Linux, macOS and Windows distros. + console.debug("") + return - path_ops.mkdir(constants.Fnm.DIR) - if not os.path.exists(constants.Fnm.EXE): - download_and_extract_fnm_zip() + path_ops.mkdir(constants.Fnm.DIR) + if not os.path.exists(constants.Fnm.EXE): + download_and_extract_fnm_zip() - if constants.IS_WINDOWS: - # Install node - fnm_exe = Path(constants.Fnm.EXE).resolve() - fnm_dir = Path(constants.Fnm.DIR).resolve() - process = processes.new_process( - [ - "powershell", - "-Command", - f'& "{fnm_exe}" install {constants.Node.VERSION} --fnm-dir "{fnm_dir}"', - ], - ) - else: # All other platforms (Linux, WSL1). - # Add execute permissions to fnm executable. - os.chmod(constants.Fnm.EXE, stat.S_IXUSR) - # Install node. - process = processes.new_process( - [ - constants.Fnm.EXE, - "install", - constants.Node.VERSION, - "--fnm-dir", - constants.Fnm.DIR, - ], - ) - processes.show_status("Installing node", process) + if constants.IS_WINDOWS: + # Install node + fnm_exe = Path(constants.Fnm.EXE).resolve() + fnm_dir = Path(constants.Fnm.DIR).resolve() + process = processes.new_process( + [ + "powershell", + "-Command", + f'& "{fnm_exe}" install {constants.Node.VERSION} --fnm-dir "{fnm_dir}"', + ], + ) + else: # All other platforms (Linux, MacOS). + # TODO we can skip installation if check_node_version() checks out + # Add execute permissions to fnm executable. + os.chmod(constants.Fnm.EXE, stat.S_IXUSR) + # Install node. + # Specify arm64 arch explicitly for M1s and M2s. + architecture_arg = ( + ["--arch=arm64"] + if platform.system() == "Darwin" and platform.machine() == "arm64" + else [] + ) + + process = processes.new_process( + [ + constants.Fnm.EXE, + "install", + *architecture_arg, + constants.Node.VERSION, + "--fnm-dir", + constants.Fnm.DIR, + ], + ) + processes.show_status("Installing node", process) def install_bun(): @@ -597,88 +624,50 @@ def validate_bun(): raise typer.Exit(1) -def validate_node(): - """Check the version of Node.js is correct. - - Raises: - Exit: If the version of Node.js is incorrect. - """ - current_version = get_node_version() - - # Check if Node is installed. - if not current_version: - console.error( - "Failed to obtain node version. Make sure node is installed and in your PATH." - ) - raise typer.Exit(1) - - # Check if the version of Node is correct. - if current_version < version.parse(constants.Node.MIN_VERSION): - console.error( - f"Reflex requires node version {constants.Node.MIN_VERSION} or higher to run, but the detected version is {current_version}." - ) - raise typer.Exit(1) - - -def remove_existing_fnm_dir(): - """Remove existing fnm directory on linux and mac.""" - if os.path.exists(constants.Fnm.DIR): - console.debug("Removing existing fnm installation.") - path_ops.rm(constants.Fnm.DIR) - - -def validate_frontend_dependencies(): - """Validate frontend dependencies to ensure they meet requirements.""" - # Bun only supports linux and Mac. For Non-linux-or-mac, we use node. - validate_bun() if constants.IS_LINUX_OR_MAC else validate_node() - - -def parse_non_semver_version(version_string: str) -> version.Version | None: - """Parse unsemantic version string and produce - a clean version that confirms to packaging.version. +def validate_frontend_dependencies(init=True): + """Validate frontend dependencies to ensure they meet requirements. Args: - version_string: The version string - - Returns: - A cleaned semantic packaging.version object. + init: whether running `reflex init` + Raises: + Exit: If the package manager is invalid. """ - # Remove non-numeric characters from the version string - cleaned_version_string = re.sub(r"[^\d.]+", "", version_string) - try: - parsed_version = version.parse(cleaned_version_string) - return parsed_version - except version.InvalidVersion: - console.debug(f"could not parse version: {version_string}") - return None + if not init: + # we only need to validate the package manager when running app. + # `reflex init` will install the deps anyway(if applied). + package_manager = get_package_manager() + if not package_manager: + console.error( + "Could not find NPM package manager. Make sure you have node installed." + ) + raise typer.Exit(1) + if not check_node_version(): + node_version = get_node_version() + console.error( + f"Reflex requires node version {constants.Node.MIN_VERSION} or higher to run, but the detected version is {node_version}", + ) + raise typer.Exit(1) -def is_valid_linux() -> bool: - """Check if the linux kernel version is valid enough to use bun. - This is typically used run npm at runtime for WSL 1 or lower linux versions. + if constants.IS_WINDOWS: + return - Returns: - If linux kernel version is valid enough. - """ - if not constants.IS_LINUX: - return False - kernel_string = platform.release() - kv = parse_non_semver_version(kernel_string) - return kv.major > 5 or (kv.major == 5 and kv.minor >= 10) if kv else False + if init: + # we only need bun for package install on `reflex init`. + validate_bun() def initialize_frontend_dependencies(): """Initialize all the frontend dependencies.""" # Create the reflex directory. path_ops.mkdir(constants.Reflex.DIR) + # validate dependencies before install + validate_frontend_dependencies() # Install the frontend dependencies. processes.run_concurrently(install_node, install_bun) # Set up the web directory. initialize_web_directory() - # remove existing fnm dir on linux and mac - if constants.IS_LINUX_OR_MAC and is_valid_linux(): - remove_existing_fnm_dir() def check_db_initialized() -> bool: diff --git a/reflex/utils/processes.py b/reflex/utils/processes.py index e385a7276..6c04d12d6 100644 --- a/reflex/utils/processes.py +++ b/reflex/utils/processes.py @@ -13,7 +13,6 @@ from typing import Callable, Generator, List, Optional, Tuple, Union import psutil import typer -from reflex import constants from reflex.utils import console, path_ops, prerequisites @@ -126,18 +125,16 @@ def new_process(args, run: bool = False, show_logs: bool = False, **kwargs): Returns: Execute a child program in a new process. """ - node_bin_path = "" - if not constants.IS_LINUX_OR_MAC or not prerequisites.is_valid_linux(): - node_bin_path = path_ops.get_node_bin_path() or "" - if not node_bin_path: - console.warn( - "The path to the Node binary could not be found. Please ensure that Node is properly " - "installed and added to your system's PATH environment variable." - ) + node_bin_path = path_ops.get_node_bin_path() + if not node_bin_path: + console.warn( + "The path to the Node binary could not be found. Please ensure that Node is properly " + "installed and added to your system's PATH environment variable." + ) # Add the node bin path to the PATH environment variable. env = { **os.environ, - "PATH": os.pathsep.join([node_bin_path, os.environ["PATH"]]), # type: ignore + "PATH": os.pathsep.join([node_bin_path if node_bin_path else "", os.environ["PATH"]]), # type: ignore **kwargs.pop("env", {}), } kwargs = { diff --git a/tests/test_prerequisites.py b/tests/test_prerequisites.py index 3d4047ea7..593129b7d 100644 --- a/tests/test_prerequisites.py +++ b/tests/test_prerequisites.py @@ -4,11 +4,7 @@ import pytest from reflex import constants from reflex.config import Config -from reflex.utils.prerequisites import ( - initialize_requirements_txt, - install_node, - update_next_config, -) +from reflex.utils.prerequisites import initialize_requirements_txt, update_next_config @pytest.mark.parametrize( @@ -146,29 +142,3 @@ def test_initialize_requirements_txt_not_exist(mocker): open_mock().write.call_args[0][0] == f"\n{constants.RequirementsTxt.DEFAULTS_STUB}{constants.Reflex.VERSION}\n" ) - - -@pytest.mark.parametrize( - "is_windows, is_linux, release, expected", - [ - (True, False, "10.0.20348", True), - (False, True, "6.2.0-1015-azure", False), - (False, True, "4.4.0-17763-Microsoft", True), - (False, False, "21.6.0", False), - ], -) -def test_install_node(is_windows, is_linux, release, expected, mocker): - mocker.patch("reflex.utils.prerequisites.constants.IS_WINDOWS", is_windows) - mocker.patch("reflex.utils.prerequisites.constants.IS_LINUX", is_linux) - mocker.patch("reflex.utils.prerequisites.platform.release", return_value=release) - mocker.patch("reflex.utils.prerequisites.download_and_extract_fnm_zip") - mocker.patch("reflex.utils.prerequisites.processes.new_process") - mocker.patch("reflex.utils.prerequisites.processes.show_status") - mocker.patch("reflex.utils.prerequisites.os.chmod") - - path_ops = mocker.patch("reflex.utils.prerequisites.path_ops.mkdir") - install_node() - if expected: - path_ops.assert_called_once() - else: - path_ops.assert_not_called() diff --git a/tests/utils/test_utils.py b/tests/utils/test_utils.py index e6cfd21c2..364c6e719 100644 --- a/tests/utils/test_utils.py +++ b/tests/utils/test_utils.py @@ -121,6 +121,19 @@ def test_validate_bun_path_incompatible_version(mocker): prerequisites.validate_bun() +def test_remove_existing_bun_installation(mocker): + """Test that existing bun installation is removed. + + Args: + mocker: Pytest mocker. + """ + mocker.patch("reflex.utils.prerequisites.os.path.exists", return_value=True) + rm = mocker.patch("reflex.utils.prerequisites.path_ops.rm", mocker.Mock()) + + prerequisites.remove_existing_bun_installation() + rm.assert_called_once() + + def test_setup_frontend(tmp_path, mocker): """Test checking if assets content have been copied into the .web/public folder. @@ -351,6 +364,65 @@ def test_node_install_windows(tmp_path, mocker): download.assert_called_once() +@pytest.mark.parametrize( + "machine, system", + [ + ("x64", "Darwin"), + ("arm64", "Darwin"), + ("x64", "Windows"), + ("arm64", "Windows"), + ("armv7", "Linux"), + ("armv8-a", "Linux"), + ("armv8.1-a", "Linux"), + ("armv8.2-a", "Linux"), + ("armv8.3-a", "Linux"), + ("armv8.4-a", "Linux"), + ("aarch64", "Linux"), + ("aarch32", "Linux"), + ], +) +def test_node_install_unix(tmp_path, mocker, machine, system): + fnm_root_path = tmp_path / "reflex" / "fnm" + fnm_exe = fnm_root_path / "fnm" + + mocker.patch("reflex.utils.prerequisites.constants.Fnm.DIR", fnm_root_path) + mocker.patch("reflex.utils.prerequisites.constants.Fnm.EXE", fnm_exe) + mocker.patch("reflex.utils.prerequisites.constants.IS_WINDOWS", False) + mocker.patch("reflex.utils.prerequisites.platform.machine", return_value=machine) + mocker.patch("reflex.utils.prerequisites.platform.system", return_value=system) + + class Resp(Base): + status_code = 200 + text = "test" + + mocker.patch("httpx.stream", return_value=Resp()) + download = mocker.patch("reflex.utils.prerequisites.download_and_extract_fnm_zip") + process = mocker.patch("reflex.utils.processes.new_process") + chmod = mocker.patch("reflex.utils.prerequisites.os.chmod") + mocker.patch("reflex.utils.processes.stream_logs") + + prerequisites.install_node() + + assert fnm_root_path.exists() + download.assert_called_once() + if system == "Darwin" and machine == "arm64": + process.assert_called_with( + [ + fnm_exe, + "install", + "--arch=arm64", + constants.Node.VERSION, + "--fnm-dir", + fnm_root_path, + ] + ) + else: + process.assert_called_with( + [fnm_exe, "install", constants.Node.VERSION, "--fnm-dir", fnm_root_path] + ) + chmod.assert_called_once() + + def test_bun_install_without_unzip(mocker): """Test that an error is thrown when installing bun with unzip not installed. @@ -375,9 +447,10 @@ def test_create_reflex_dir(mocker, is_windows): is_windows: Whether platform is windows. """ mocker.patch("reflex.utils.prerequisites.constants.IS_WINDOWS", is_windows) - mocker.patch("reflex.utils.prerequisites.install_bun", mocker.Mock()) - mocker.patch("reflex.utils.prerequisites.install_node", mocker.Mock()) + mocker.patch("reflex.utils.prerequisites.processes.run_concurrently", mocker.Mock()) mocker.patch("reflex.utils.prerequisites.initialize_web_directory", mocker.Mock()) + mocker.patch("reflex.utils.processes.run_concurrently") + mocker.patch("reflex.utils.prerequisites.validate_bun") create_cmd = mocker.patch( "reflex.utils.prerequisites.path_ops.mkdir", mocker.Mock() )