Browse Source

Revert "Bun as runtime on Mac and Linux (#2138)" (#2153)

Elijah Ahianyo 1 year ago
parent
commit
7a04652a6a

+ 0 - 4
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,

+ 0 - 2
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):

+ 9 - 8
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":

+ 104 - 115
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():
-
-        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)
+    """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()
+
+    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:

+ 7 - 10
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 = {

+ 1 - 31
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()

+ 75 - 2
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()
     )