🧹 Clean up nodeenv and add CI tests (#1561)

.github/workflows/python-ci.yml

@@ -0,0 +1,96 @@
+name: Python CI
+
+on:
+  push:
+    branches: [main, ci-*]
+    paths:
+        - 'packages/mystmd-py/**'
+  pull_request:
+    branches: [main]
+    paths:
+      - 'packages/mystmd-py/**'
+  workflow_dispatch:
+
+jobs:
+  build-package:
+    name: Build Python package
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 2
+          submodules: recursive
+      - uses: ./.github/actions/install
+        with:
+          node: ${{ matrix.node }}
+      - name: Copy necessary files
+        run: |
+          cp packages/mystmd/dist/myst.cjs packages/mystmd-py/src/mystmd_py/
+          cp packages/mystmd/package.json packages/mystmd-py/_package.json
+      - name: Build Python package
+        run: pipx run hatch -- build
+        working-directory: packages/mystmd-py
+      - uses: actions/upload-artifact@v4
+        with:
+          name: package
+          path: packages/mystmd-py/dist/mystmd*.whl
+          if-no-files-found: error
+
+  platform-node:
+    name: Test with Node.js
+    runs-on: ubuntu-latest
+    needs: [build-package]
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+          submodules: recursive
+      - uses: actions/download-artifact@v4
+        with:
+          name: package
+      - name: Install myst from package
+        run: pip install mystmd*.whl
+      - name: Run myst expect success
+        env:
+          MYSTMD_ALLOW_NODEENV: 0
+        run: myst -v
+
+  no-node:
+    name: Test without Node.js
+    runs-on: ubuntu-latest
+    needs: [build-package]
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+          submodules: recursive
+      - uses: actions/download-artifact@v4
+        with:
+          name: package
+      - name: Run myst and expect failure
+        uses: "docker://python:3.12.7-slim-bookworm"
+        with:
+          entrypoint: /bin/bash
+          args: '-c "export MYSTMD_ALLOW_NODEENV=0; pip install mystmd*.whl && ! myst -v"'
+
+  nodeenv-node:
+    name: Test with nodeenv
+    runs-on: ubuntu-latest
+    needs: [build-package]
+    steps:
+      - name: Checkout Repo
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+          submodules: recursive
+      - uses: actions/download-artifact@v4
+        with:
+          name: package
+      - name: Run myst with nodeenv and expect success
+        uses: "docker://python:3.12.7-slim-bookworm"
+        with:
+          entrypoint: /bin/bash
+          args: '-c "export MYSTMD_ALLOW_NODEENV=1; pip install mystmd*.whl && myst -v"'

packages/mystmd-py/src/mystmd_py/main.py

@@ -1,104 +1,45 @@
 import os
 import pathlib
 import platform
-import shutil
 import subprocess
 import sys
 import re
 import textwrap
 
+from .nodeenv import find_any_node, PermissionDeniedError, NodeEnvCreationError
 
-NODEENV_VERSION = "18.0.0"
-INSTALL_NODEENV_KEY = "MYSTMD_ALLOW_NODEENV"
-
-
-class PermissionDeniedError(Exception): ...
-
-
-class NodeEnvCreationError(Exception): ...
-
-
-def is_windows():
-    return platform.system() == "Windows"
-
-
-def find_installed_node():
-    # shutil.which can find things with PATHEXT, but 3.12.0 breaks this by preferring NODE over NODE.EXE on Windows
-    return shutil.which("node.exe") if is_windows() else shutil.which("node")
-
-
-def find_nodeenv_path():
-    # The conda packaging of this package does not need to install node!
-    import platformdirs
-
-    return platformdirs.user_data_path(
-        appname="myst", appauthor=False, version=NODEENV_VERSION
-    )
-
-
-def ask_to_install_node(path):
-    if env_value := os.environ.get(INSTALL_NODEENV_KEY, "").lower():
-        return env_value in {"yes", "true", "1", "y"}
-
-    return input(f"❔ Install Node.js in '{path}'? (y/N): ").lower() == "y"
-
-
-def create_nodeenv(env_path):
-    command = [
-        sys.executable,
-        "-m",
-        "nodeenv",
-        "-v",
-        f"--node={NODEENV_VERSION}",
-        "--prebuilt",
-        "--clean-src",
-        env_path,
-    ]
-    result = subprocess.run(command, capture_output=True, encoding="utf-8")
-    if result.returncode:
-        shutil.rmtree(env_path)
-        raise NodeEnvCreationError(result.stderr)
-    else:
-        return env_path
 
+NODEENV_VERSION = "18.0.0"
 
-def find_any_node(binary_path):
-    node_path = find_installed_node()
-    if node_path is not None:
-        return pathlib.Path(node_path).absolute(), binary_path
 
-    nodeenv_path = find_nodeenv_path()
-    if not nodeenv_path.exists():
-        print("❗ Node.js (node) is required to run MyST, but could not be found`.")
-        if ask_to_install_node(nodeenv_path):
-            print(f"⚙️ Attempting to install Node.js in {nodeenv_path} ...")
-            create_nodeenv(nodeenv_path)
-            print(f"ℹ️ Successfully installed Node.js {NODEENV_VERSION}")
-        else:
-            raise PermissionDeniedError("Node.js installation was not permitted")
+def ensure_valid_version(node_path, node_env):
+    # Check version
+    _version = subprocess.run(
+        [node_path, "-v"], capture_output=True, check=True, text=True, env=node_env
+    ).stdout
+    major_version_match = re.match(r"v(\d+).*", _version)
 
-    # Find the executable path
-    new_node_path = (
-        (nodeenv_path / "Scripts" / "node.exe")
-        if is_windows()
-        else (nodeenv_path / "bin" / "node")
-    )
-    new_path = os.pathsep.join(
-        [*binary_path.split(os.pathsep), str(new_node_path.parent)]
-    )
-    return new_node_path, new_path
+    if major_version_match is None:
+        raise SystemExit(f"MyST could not determine the version of Node.js: {_version}")
+    major_version = int(major_version_match[1])
+    if not (major_version in {18, 20, 22} or major_version > 22):
+        raise SystemExit(
+            f"MyST requires node 18, 20, or 22+; you are running node {major_version}.\n\n"
+            "Please update to the latest LTS release, using your preferred package manager\n"
+            "or following instructions here: https://nodejs.org/en/download"
+        )
 
 
 def main():
     # Find NodeJS (and potential new PATH)
     binary_path = os.environ.get("PATH", os.defpath)
     try:
-        node_path, os_path = find_any_node(binary_path)
+        node_path, os_path = find_any_node(binary_path, nodeenv_version=NODEENV_VERSION)
     except NodeEnvCreationError as err:
         message = textwrap.indent(err.args[0], "    ")
         raise SystemExit(
             "💥 The attempt to install Node.js was unsuccessful.\n"
-            f"🔍 Underlying error:\n{message}\n\n"
+            f"🔍  Underlying error:\n{message}\n\n"
             "ℹ️  We recommend installing the latest LTS release, using your preferred package manager "
             "or following instructions here: https://nodejs.org\n\n"
         ) from err
@@ -112,21 +53,8 @@ def main():
     # Build new env dict
     node_env = {**os.environ, "PATH": os_path}
 
-    # Check version
-    _version = subprocess.run(
-        [node_path, "-v"], capture_output=True, check=True, text=True, env=node_env
-    ).stdout
-    major_version_match = re.match(r"v(\d+).*", _version)
-
-    if major_version_match is None:
-        raise SystemExit(f"MyST could not determine the version of Node.js: {_version}")
-    major_version = int(major_version_match[1])
-    if not (major_version in {18, 20, 22} or major_version > 22):
-        raise SystemExit(
-            f"MyST requires node 18, 20, or 22+; you are running node {major_version}.\n\n"
-            "Please update to the latest LTS release, using your preferred package manager\n"
-            "or following instructions here: https://nodejs.org/en/download"
-        )
+    # Ensure Node.js is compatible
+    ensure_valid_version(node_path, node_env)
 
     # Find path to compiled JS
     js_path = (pathlib.Path(__file__).parent / "myst.cjs").resolve()

packages/mystmd-py/src/mystmd_py/nodeenv.py

@@ -1,11 +1,11 @@
 import os
 import pathlib
+import platform
 import shutil
 import subprocess
 import sys
 
 
-NODEENV_VERSION = "18.0.0"
 INSTALL_NODEENV_KEY = "MYSTMD_ALLOW_NODEENV"
 
 
@@ -15,16 +15,20 @@ class PermissionDeniedError(Exception): ...
 class NodeEnvCreationError(Exception): ...
 
 
+def is_windows():
+    return platform.system() == "Windows"
+
+
 def find_installed_node():
-    return shutil.which("node") or shutil.which("node.exe") or shutil.which("node.cmd")
+    # shutil.which can find things with PATHEXT, but 3.12.0 breaks this by preferring NODE over NODE.EXE on Windows
+    return shutil.which("node.exe") if is_windows() else shutil.which("node")
 
 
-def find_nodeenv_path():
+def find_nodeenv_path(version: str):
     # The conda packaging of this package does not need to install node!
     import platformdirs
-    return platformdirs.user_data_path(
-        appname="myst", appauthor=False, version=NODEENV_VERSION
-    )
+
+    return platformdirs.user_data_path(appname="myst", appauthor=False, version=version)
 
 
 def ask_to_install_node(path):
@@ -34,13 +38,13 @@ def ask_to_install_node(path):
     return input(f"❔ Install Node.js in '{path}'? (y/N): ").lower() == "y"
 
 
-def create_nodeenv(env_path):
+def create_nodeenv(env_path, version):
     command = [
         sys.executable,
         "-m",
         "nodeenv",
         "-v",
-        f"--node={NODEENV_VERSION}",
+        f"--node={version}",
         "--prebuilt",
         "--clean-src",
         env_path,
@@ -53,24 +57,28 @@ def create_nodeenv(env_path):
         return env_path
 
 
-def find_any_node(binary_path):
+def find_any_node(binary_path, nodeenv_version):
     node_path = find_installed_node()
     if node_path is not None:
         return pathlib.Path(node_path).absolute(), binary_path
 
-    nodeenv_path = find_nodeenv_path()
+    nodeenv_path = find_nodeenv_path(nodeenv_version)
     if not nodeenv_path.exists():
         print("❗ Node.js (node) is required to run MyST, but could not be found`.")
         if ask_to_install_node(nodeenv_path):
-            print(f"⚙️ Attempting to install Node.js in {nodeenv_path} ...")
-            create_nodeenv(nodeenv_path)
-            print(f"ℹ️ Successfully installed Node.js {NODEENV_VERSION}")
+            print(f"⚙️  Attempting to install Node.js in {nodeenv_path} ...")
+            create_nodeenv(nodeenv_path, nodeenv_version)
+            print(f"ℹ️  Successfully installed Node.js {nodeenv_version}")
         else:
             raise PermissionDeniedError("Node.js installation was not permitted")
 
+    # Find the executable path
+    new_node_path = (
+        (nodeenv_path / "Scripts" / "node.exe")
+        if is_windows()
+        else (nodeenv_path / "bin" / "node")
+    )
     new_path = os.pathsep.join(
-        [*binary_path.split(os.pathsep), str(nodeenv_path / "bin")]
+        [*binary_path.split(os.pathsep), str(new_node_path.parent)]
     )
-    return nodeenv_path / "bin" / "node", new_path
-
-
+    return new_node_path, new_path