STB_USE_SYSTEM_NODE is ignored

[FRidh]

Attempting to package sphinx-theme-builder and pydata-sphinx-theme I noticed that STB_USE_SYSTEM_NODE is ignored.

The Nix environment provides a sandbox. I explicitly provide a build of node and npm

$ find $(nix-build -A nodejs-16_x)/bin
/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin
/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/npm
/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/node
/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/npx
/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/corepack

sphinx-theme-builder requires the version to match. pydata-sphinx-theme version 0.8.1 needs nodejs 16.13.2. Without fixing the entry in pyproject.toml I would expect a nodeenv-version-mismatch but I am not getting that. When I do patch the version I can see it wants to use that version but still it seems to go ahead with trying to download a npm.

Processing /build/pydata_sphinx_theme-0.8.1
  Running command Preparing metadata (pyproject.toml)
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: pydata-sphinx-theme
  Running command Building wheel for pydata-sphinx-theme (pyproject.toml)
  [stb] # nodeenv does not exist.
  [stb] # Generating new nodeenv with NodeJS 16.15.0!
  [stb] $ python -m nodeenv --node=16.15.0 --prebuilt --clean-src
  /build/pydata_sphinx_theme-0.8.1/.nodeenv
   * Install prebuilt node (16.15.0) .

Related to readthedocs/sphinx_rtd_theme#1014 (comment).

[pradyunsg]

Do you have a non-Nix reproducer for the behaviour that you're seeing? A shell script I can run in my machine that reproduces this issue would be great.

[FRidh]

I can't reproduce the issue outside the Nix sandbox

#!/usr/bin/env sh

url="https://nodejs.org/dist/v16.15.1/node-v16.15.1-linux-x64.tar.xz"
package="pydata-sphinx-theme"
version="0.8.1"
testdir="testdir"
download="$testdir/download"
nodeenv="$testdir/nodeenv"
pythonenv="$testdir/pythonenv"

echo "Creating download folder $download"
mkdir -p "$download"
echo "Creating foler with node env $nodeenv"
mkdir -p "$nodeenv"
echo "Creating folder with Python environment $pythonenv"
mkdir -p "$pythonenv"
echo "Downloading nodejs"
wget --no-clobber "$url" -P "$download"
echo "Unpacking nodejs"
tar xf "$download/${url##*/}" -C "$nodeenv" --strip-components 1
echo "Creating venv"
python3 -m venv "$pythonenv"
echo "Active env"
source "$pythonenv/bin/activate"
echo "Update to latest pip"
pip install pip --upgrade
echo "Build wheel of package of interest"
pip wheel --no-binary "$package" "$package==$version" 
echo "Build wheel with Nix provided node of different version, should result in mismatch"
STB_USE_SYSTEM_NODE=true pip wheel --no-binary "$package" "$package==$version"

When I go to the Nix build though. The environment variable is clearly set inside the sandbox but it still goes ahead with trying to fetch a prebuilt node.

Notes for myself. I've tested disabling strictDeps, thinking it might not find node on $PATH but it is there. Tested without pip-build-hook.sh by invoking pip directly, no difference. Regardless, we would have to create the .nodeenv ourselves using node2nix skipping this entire part.

[pradyunsg]

Hmm... it looks like this line was never executed:

sphinx-theme-builder/src/sphinx_theme_builder/_internal/nodejs.py

Line 121 in 01d0fca

node_version = "system"

Based on:

[stb] $ python -m nodeenv --node=16.15.0 --prebuilt --clean-src /build/pydata_sphinx_theme-0.8.1/.nodeenv

It should be --node=system.

Trying STB_USE_SYSTEM_NODE=true stb compile locally, I have:

[stb] $ python -m nodeenv --node=system --prebuilt --clean-src /Users/pradyunsg/Developer/github/furo/.nodeenv

[pradyunsg]

The environment variable is clearly set inside the sandbox but it still goes ahead with trying to fetch a prebuilt node.

I'm fairly sure that the issue here is that STB_USE_SYSTEM_NODE=true is not set, when sphinx-theme-builder is invoked. I'm able to get the --node=system flag even with STB_USE_SYSTEM_NODE=true pip wheel . --no-build-isolation -vv:

  [stb] $ python -m nodeenv --node=system --prebuilt --clean-src
  /Users/pradyunsg/Developer/github/furo/.nodeenv

[FRidh]

The environment variable is clearly set inside the sandbox but it still goes ahead with trying to fetch a prebuilt node.

I'm fairly sure that the issue here is that STB_USE_SYSTEM_NODE=true is not set, when sphinx-theme-builder is invoked. I'm able to get the --node=system flag even with STB_USE_SYSTEM_NODE=true pip wheel . --no-build-isolation -vv:

  [stb] $ python -m nodeenv --node=system --prebuilt --clean-src
  /Users/pradyunsg/Developer/github/furo/.nodeenv

Changing to

def create_nodeenv(nodeenv: Path, node_version: str) -> None:

    import os
    import subprocess
    print("create nodeenv function")
    print(os.environ["STB_USE_SYSTEM_NODE"])
    print(subprocess.run("which node", shell=True, capture_output=True).stdout)
    # exit(1)

    log(
        "[yellow]#[/] [cyan]Generating new [magenta]nodeenv[/] with "
        f"NodeJS [green]{node_version}[/]!"
    )

prints

Building wheels for collected packages: pydata-sphinx-theme
  Running command Building wheel for pydata-sphinx-theme (pyproject.toml)
  [stb] # nodeenv does not exist.
  create nodeenv function
  1
  b'/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/node\n'
  [stb] # Generating new nodeenv with NodeJS 16.15.0!
  [stb] $ python -m nodeenv --node=16.15.0 --prebuilt --clean-src
  /build/pydata_sphinx_theme-0.8.1/.nodeenv
   * Install prebuilt node (16.15.0) .

after patching pyproject.toml to use this version, and when not patching

Building wheels for collected packages: pydata-sphinx-theme
  Running command Building wheel for pydata-sphinx-theme (pyproject.toml)
  [stb] # nodeenv does not exist.
  create nodeenv function
  1
  b'/nix/store/dvzrdz86i15bmjyy869mi7h2bcgl05az-nodejs-16.15.0/bin/node\n'
  [stb] # Generating new nodeenv with NodeJS 16.13.2!
  [stb] $ python -m nodeenv --node=16.13.2 --prebuilt --clean-src
  /build/pydata_sphinx_theme-0.8.1/.nodeenv
   * Install prebuilt node (16.13.2) .

Found the issue. I set STB_USE_SYSTEM_NODE=true in the Nix expression. Booleans are always converted to integer booleans, so this became a 1. A check is done explicitly against the string "true" instead of boolean.

    if os.environ.get("STB_USE_SYSTEM_NODE", "false") != "true":

I recommend using here something like the deprecated https://docs.python.org/3/distutils/apiref.html#distutils.util.strtobool.

[pradyunsg]

Found the issue. I set STB_USE_SYSTEM_NODE=true in the Nix expression. Booleans are always converted to integer booleans, so this became a 1. A check is done explicitly against the string "true" instead of boolean.

Fun. Filed #27 as a follow up on this. :)

[eli-schwartz]

Either checking against a variety of possible values, OR raising an "invalid option value" error if it isn't in {'true', 'false'} would be a good idea. Mandating a consistent style may actually be a good idea though.

The problem here isn't really that passing 1 didn't have the correct effect, the problem is that passing 1 wasn't handled at all.

Personally, I would either do just the latter, or else both.