Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧹 Clean up nodeenv and add CI tests #1561

Merged
merged 4 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions .github/workflows/python-ci.yml
Original file line number Diff line number Diff line change
@@ -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"'
114 changes: 21 additions & 93 deletions packages/mystmd-py/src/mystmd_py/main.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
Expand Down
42 changes: 25 additions & 17 deletions packages/mystmd-py/src/mystmd_py/nodeenv.py
Original file line number Diff line number Diff line change
@@ -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"


Expand All @@ -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):
Expand All @@ -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,
Expand All @@ -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
Loading