Remove dependency on npm?

[adamjstewart]

I'm a developer for the Spack package manager. Spack builds all packages from source, including our sphinx_rtd_theme package. Right now, this requires the user to also install npm, which can be very time consuming to build from source and may not be buildable on all platforms. I'm wondering if it would be possible to handle all pre-processing that requires the use of npm before each release so that npm is only required to build from master/develop.

@j-ogas

[FRidh]

Right now, this requires the user to also install npm, which can be very time consuming to build from source and may not be buildable on all platforms.

In Nixpkgs we build in a sandbox, and define exactly what needs to be fetched where. Fetching stuff from the web like that is not possible and simply bad practice. Introduced in https://github.com/readthedocs/sphinx_rtd_theme/pull/809.

Similar issue here with another package https://github.com/holoviz/panel/issues/1819.

[agjohnson]

There is some background here in a few other issues. We are moving away from storing generated CSS/JS assets in the repository, and instead building assets will be entirely a release procedure. Our releases to PyPI will therefore have the built assets, but if you are trying to use this repository directly, or in your case are building from source, you will need to build the assets. Storing the built assets in this repository has proved to be very problematic for maintenance and for releases.

I'm not sure if there is a good middle ground for your use case, but I'm open to suggestions.

[adamjstewart]

I agree with not storing build assets in the repository, but it would be great to store those build assets in the PyPI tarballs so that you can download a release and build from source without needing npm. Is that possible?

[agjohnson]

Great, yeah if you can use the source tarball, I think there was just an issue with it during the last release, or we didn't configure it correctly -- but it was our intention to have all of the PyPI release artifacts include the compiled assets. We can aim to resolve this in the next release.

[FRidh]

We typically prefer to build entirely from source, but given it is generally hard to deal with packages of multiple ecosystems I think including the assets in the sdist is indeed a good middle ground.

Anyway, I do recommend moving this code out of the setup.py for wheel building, and instead have a separate script that needs to be run first. There already is the MANIFEST.in so that's good.

[stsewd]

A new release is out (0.5.1) https://pypi.org/project/sphinx-rtd-theme/ the static files should be there, let us know if that's what you expect.

[adamjstewart]

@stsewd If I download 0.5.1 from PyPI or GitHub and run:

$ python setup.py build  # or install

it errors out with:

error: [Errno 2] No such file or directory: 'npm'

I see the expected files in sphinx_rtd_theme/static, maybe we just need to change setup.py to only run npm if those files are missing?

[adamjstewart]

If I set the CI environment variable, I'm able to skip the call to npm. Is that advisable?

[stsewd]

I think those commands should be moved to a make file or tox.ini

https://github.com/readthedocs/sphinx_rtd_theme/blob/f6554bfcbfc404db824e11608b1a91c275b3e3d3/setup.py#L16-L83

/cc @agjohnson

[stsewd]

So, looks like we are moving to not have the static files in the repo, but we won't override the default build command, so you can download the source from pypi and install it manually. Users installing from github should/would be warned that they'll need to run npm to generate the assets.

[adamjstewart]

I agree with not having static files in the repo, as long as those files are in release tarballs on GitHub and PyPI. Has the setup.py been modified so that it doesn't run npm if we're building a release and those files are already found?

[stsewd]

@adamjstewart that is being done in https://github.com/readthedocs/sphinx_rtd_theme/pull/1039

[agjohnson]

@adamjstewart also, the other part to this discussion that @stsewd and i had about this change: core team discussed these changes a while back and came to the conclusion that we want to replace installation from git with more frequent development releases. So if you are currently installing from master to obtain recent fixes to the theme, we're aiming to support this installation pattern better with proper releases instead.

[adamjstewart]

Nah, we're not installing from master, but I'm glad to hear that!

[pradyunsg]

In a completely different direction, https://github.com/pradyunsg/sphinx-theme-builder adds an explicit dependency on npm, such that it can be managed by redistributors in a consistent manner across Sphinx themes. It has a clearly defined contract (build-time installation of npm dependencies declared in package.json and runs the build using npm run-script) and works somewhat seamlessly, thanks to nodeenv (+ a fallback allowing redistributors to provide their own node/npm). This generates wheel files that contain compiled assets while the source distribution only contains the source code.

Furo, pydata-sphinx-theme and sphinx-book-theme all use this and (so far) the feedback from the maintainers as well as redistributors has been generally positive.

It doesn't do away with "oh, never access the network" model, that certain redistributors would like to have -- but it does make it such that all the dependencies are declared explicitly and can be provided by the redistributor themselves. If they can't do that today, we can "solve" it in one place for all the Sphinx themes that use a JS-based build pipeline (like they should, given that they're basically web frontend projects).

[FRidh]

In a completely different direction, https://github.com/pradyunsg/sphinx-theme-builder adds an explicit dependency on npm, such that it can be managed by redistributors in a consistent manner across Sphinx themes. It has a clearly defined contract (build-time installation of npm dependencies declared in package.json and runs the build using npm run-script) and works somewhat seamlessly, thanks to nodeenv (+ a fallback allowing redistributors to provide their own node/npm). This generates wheel files that contain compiled assets while the source distribution only contains the source code.

Furo, pydata-sphinx-theme and sphinx-book-theme all use this and (so far) the feedback from the maintainers as well as redistributors has been generally positive.

It doesn't do away with "oh, never access the network" model, that certain redistributors would like to have -- but it does make it such that all the dependencies are declared explicitly and can be provided by the redistributor themselves. If they can't do that today, we can "solve" it in one place for all the Sphinx themes that use a JS-based build pipeline (like they should, given that they're basically web frontend projects).

If I am correct it consists essentially of the following steps:

• create a .nodeenv folder. This is a FHS layout containing npm.
• run npm on package.json to get your node-modules using std npm.
• compile assets using std compile.

This looks good to me.

Is the .nodeenv only used during build-time or will it be included in the installation?

How is the required npm version declared? I imagine this to be project-dependent.

[pradyunsg]

The workflow is typically stb package or python -m build to build the package. stb serve docs/ during development. stb compile if you want to compile just the assets for some reason.

See the pyproject.toml file in any of the themes -- you need to specify a key for declaring the NodeJS version.

The nodeenv is not shipped in any distribution files.

[FRidh]

The workflow is typically stb package or python -m build to build the package. stb serve docs/ during development. stb compile if you want to compile just the assets for some reason.

We need to isolate the part doing the network access hence I was trying to see what steps stb package does.

See the pyproject.toml file in any of the themes -- you need to specify a key for declaring the NodeJS version.

I now see the relevant section, thank you. https://github.com/executablebooks/sphinx-book-theme/blob/af6277a7276112ee915d901984193c49e56d5a76/pyproject.toml#L5

[pradyunsg]

We need to isolate the part doing the network access hence I was trying to see what steps stb package does.

That would be the step that creates the nodeenv and run npm install, as the initial bootstrap -- this would typically be the first run of stb package or stb compile. Subsequent runs will reuse the nodeenv, assuming that the package.json file is not newer than the nodeenv directory (which is what would happen, if a user modifies it during development).

You can tell stb to use a local NodeJS instead of downloading a release when creating the nodeenv, by setting STB_USE_SYSTEM_NODE=true as an environment variable. Note that this will need to match the declared NodeJS version in the theme. If that's problematic, please feel free to file an issue on the sphinx-theme-builder project -- I've started with strict validation of things because it's easier to relax that compared to becoming strict later. :)

[pradyunsg]

I guess, if you want to isolate the network access steps, you can do this:

• Run stb compile --production with network access, to compile the files once.
• Run stb package without network access, to generate the distribution files (this may recompile the files, if the theme isn't using a timestamp aware build system -- using something smarter like webpack would not trigger that).

[pradyunsg]

Right now, this requires the user to also install npm, which can be very time consuming to build from source and may not be buildable on all platforms

With sphinx-theme-builder's use of nodeenv and its use of compiled binaries provided by the NodeJS release team, a build from source would not be required on platforms supported by the NodeJS team. Unsupported platforms will attempt a build, which may fail -- but that's no different from the status quo.

[pradyunsg]

Well, I came here to say that I think sphinx-theme-builder solves the problem that you're trying to solve here, and that I recommend using that. I've suggested it to RTD folks before, so I'm not going to try and push for adoption here.

If they do decide to go ahead and adopt it, I'll be happy to help. :)

[pradyunsg]

@FRidh FYI: I just wrote this today -- https://sphinx-theme-builder.readthedocs.io/en/latest/build-process/ :)

[FRidh]

@FRidh FYI: I just wrote this today -- https://sphinx-theme-builder.readthedocs.io/en/latest/build-process/ :)

Great, thank you!