Skip to content

docs(cleanup): making small changes for cleaning up manual linting#147

Open
benschwaller wants to merge 1 commit into
charmed-hpc:mainfrom
benschwaller:docs_lint_fixes
Open

docs(cleanup): making small changes for cleaning up manual linting#147
benschwaller wants to merge 1 commit into
charmed-hpc:mainfrom
benschwaller:docs_lint_fixes

Conversation

@benschwaller
Copy link
Copy Markdown
Contributor

Pre-submission checklist

  • I read and followed the CONTRIBUTING guidelines.
  • I have ensured that the documentation tests complete successfully.
  • I have added a metadata description at the top of any new page.

Summary of Changes

//: Added LTS to a version number in performance.md to satisfy vale.
//: Added proxy-agent to pa11y install for correct functionality.
//: Added node_modules to excludes for passing pa11y.

Related Issues, PRs, and Discussions

@benschwaller benschwaller requested a review from a team as a code owner June 2, 2026 21:17
@AshleyCliff
Copy link
Copy Markdown
Contributor

@benschwaller do you have any error logs for the proxy-agent piece? I'm trying to get a better idea of what's requiring it to see if it needs to be added to the Sphinx Stack makefile template upstream.

It's already in .sphinx/node_modules for me locally but isn't in the makefile anywhere so I'm not sure where/when/why it's already installed.

@benschwaller
Copy link
Copy Markdown
Contributor Author

benschwaller commented Jun 3, 2026

I needed it for specifically running pa11y. When I did a fresh make pa11y in I did not have proxy-agent npm installed. If I uninstall it and wipe my .sphinx/venv now though I don't have the issue. I am trying to get a fresh multipass instance to get a better view (seems like a good usecase for workshop but I want to get you an answer sooner).

@NucciTheBoss
Copy link
Copy Markdown
Member

I concur with Ashley's comment here. When I ran make pa11y inside a fresh workshop instance, I didn't need to install proxy-agent for it to work correctly. That said, just because I couldn't easily reproduce the issue doesn't necessarily preclude that some potential funkiness is going on here. Hopefully your second test will produce some logs that we can analyze.

@benschwaller
Copy link
Copy Markdown
Contributor Author

So I spun up a fresh multipass. Never came across the proxy-agent issue, so let's count that as a one off. Here's new findings though from a freshly born ubuntu 24.04 that I needed to get everything to work.

git clone https://github.com/charmed-hpc/docs.git
sudo apt install make
sudo apt install npm (for pa11y) 
sudo apt install python3.12-venv

If python 3.12-venv is not installed before make, you need to do a rm -rf .sphinx/venv otherwise you get

|.sphinx/venv/bin/activate; .sphinx/venv/bin/sphinx-autobuild -b dirhtml --host 127.0.0.1 --port 8000 "." "_build" -c . -d .sphinx/.doctrees -j auto
/bin/sh: 1: .: cannot open .sphinx/venv/bin/activate: No such file
make: *** [Makefile:82: run] Error 2

make run then works, I run make pa11y and:

Error: Failed to launch the browser process:  Code: 127

stderr:
/home/ubuntu/.cache/puppeteer/chrome/linux-148.0.7778.97/chrome-linux64/chrome: error while loading shared libraries: libatk-1.0.so.0: cannot open shared object file: No such file or directory

TROUBLESHOOTING: https://pptr.dev/troubleshooting

    at ChildProcess.onClose (/home/ubuntu/docs/.sphinx/node_modules/@puppeteer/browsers/lib/cjs/launch.js:350:24)
    at ChildProcess.emit (node:events:529:35)
    at ChildProcess._handle.onexit (node:internal/child_process:292:12)

ldd shows missing packages for puppeteer which can then be added to make pa11y happy.

ldd /home/ubuntu/.cache/puppeteer/chrome/linux-148.0.7778.97/chrome-linux64/chrome | grep "not found"
        libatk-1.0.so.0 => not found
        libatk-bridge-2.0.so.0 => not found
        libcups.so.2 => not found
        libasound.so.2 => not found
        libcairo.so.2 => not found
        libpango-1.0.so.0 => not found
        libXdamage.so.1 => not found
        libatspi.so.0 => not found
        
sudo apt-get install -y \
  libatk1.0-0 \
  libatk-bridge2.0-0 \
  libcups2t64 \
  libasound2t64 \
  libcairo2 \
  libpango-1.0-0 \
  libxdamage1 \
  libatspi2.0-0

What would be best to change / edit this commit and then also to address the reqs I needed to follow?

@benschwaller
Copy link
Copy Markdown
Contributor Author

Testing this with workshop now too for getting familiar with that

@benschwaller
Copy link
Copy Markdown
Contributor Author

In workshop with minimal 24.04, this is all I needed to add these apt installs for puppeteer. The other issues held.

sudo apt update && sudo apt install -y
libgbm1
libxcomposite1
libxfixes3
libxrandr2

@benschwaller
Copy link
Copy Markdown
Contributor Author

@AshleyCliff @NucciTheBoss just a gentle ping, how should I proceed with this commit? I am leaning towards:

  • Closing it and then reopening without the proxy-agent add
  • Adding the apt installs for puppeteer for pa11y in a similar manner to REQPDFPACKS in the makefile
  • Add some lines to the documentation to mention the basic installs and note on the sphinx/venv.

Thoughts?

@AshleyCliff
Copy link
Copy Markdown
Contributor

AshleyCliff commented Jun 5, 2026

It's probably worth comparing against the current Sphinx Stack first repo, docs since the plan is to update to the latest version within the next few weeks. Anything that's likely to get solved with those updates might as well wait.

@benschwaller
Copy link
Copy Markdown
Contributor Author

AI summarization below of what to do now with 2.0 migration on sphinx in mind. TLDR just some names would change for 2.0 but the installs needed for puppeteer would still be present, make a reqpa11y and do things similarly to pdf. I didn't see any reason we would wait on this given the name changes will need to be done for several other bits of the makefile anyway.

  1. Add the package variable (after Makefile:17, next to REQPDFPACKS)
    REQPA11YPACKS = ca-certificates fonts-liberation libasound2 libatk-bridge2.0-0 libatk1.0-0 libc6 libcairo2 libcups2 libdbus-1-3 libexpat1 libfontconfig1 libgbm1 libgcc1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libpangocairo-1.0-0 libstdc++6 libx11-6 libx11-xcb1 libxcb1 libxcomposite1 libxcursor1 libxdamage1 libxext6 libxfixes3 libxi6 libxrandr2 libxrender1 libxss1 libxtst6 lsb-release wget xdg-utils
    I deviated from the single-line REQPDFPACKS style and kept it on one line anyway — splitting 36 packages with \ continuations would be more readable, but matching the existing convention matters more than my preference here. If you want it split, easy change.

  2. Update .PHONY (line 47–50)
    .PHONY: help full‑help html epub pdf linkcheck spelling spellcheck woke
    vale pa11y run serve install pa11y-install
    pa11y-deps pa11y-deps-force
    vale‑install pdf‑prep pdf‑prep‑force clean clean-doc allmetrics
    update lint-md
    Added pa11y-deps pa11y-deps-force on the same line as pa11y-install to mirror how pdf-prep pdf-prep-force is grouped.

  3. Make pa11y-install depend on pa11y-deps (line 69)
    pa11y-install: pa11y-deps
    @command -v $(PA11Y) >/dev/null || {
    echo "Installing "pa11y" from npm..."; echo;
    mkdir -p $(SPHINXDIR)/node_modules/ ;
    npm install --prefix $(SPHINXDIR) pa11y;
    }
    The pa11y target (line 109) is pa11y-install html, so it transitively picks up the check too. No change needed there.

  4. Add the two new targets (after line 74, before pymarkdownlnt-install)
    pa11y-deps:
    @for packageName in $(REQPA11YPACKS); do (dpkg-query -W -f='$${Status}' $$packageName 2>/dev/null |
    grep -c "ok installed" >/dev/null && echo "Package $$packageName is installed") && continue ||
    (echo; echo "pa11y requires the installation of the following packages: $(REQPA11YPACKS)" &&
    echo "" && echo "Run 'sudo make pa11y-deps-force' to install these packages" && echo "" && echo
    "Please be aware these packages will be installed to your system") && exit 1 ; done

pa11y-deps-force:
apt-get update
apt-get install --no-install-recommends -y $(REQPA11YPACKS)
I copied the pdf-prep / pdf-prep-force recipe format verbatim, then dropped apt-get upgrade -y from the force target.

2.0 translation (for when you upgrade)
When you cut over to sphinx-stack 2.0, the same additions need these renames:

v1.x v2.0
REQPA11YPACKS DOCS_PA11YPACKAGES
$(SPHINXDIR) $(DEV_DIR)
$(PA11Y) $(PA11Y_CMD)
$(VENVDIR) (only used in prereqs, not here) $(DOCS_VENVDIR)
The body of the two new targets is byte-for-byte the same. Also remember: 2.0 drops allmetrics, so if you use it, sort that out before/after the rename in a separate change.

Reasoning per decision

Why a separate -deps / -deps-force split, mirroring pdf-prep. That pattern exists in the existing Makefile specifically because the Makefile shouldn't be silently making system-wide changes. The check target emits a helpful message and exits 1; the force target is opt-in via sudo. Pa11y needs the same courtesy. CI images bake the deps in at build time; dev machines run sudo make pa11y-deps-force once; day-to-day make pa11y is silent.

Why prereq on pa11y-install, not pa11y. pa11y-install is also a public target someone could run on its own (e.g. to populate the node_modules cache). Putting the check there means any code path that actually launches pa11y (which depends on pa11y-install for the binary) gets the system check. pa11y itself stays untouched. This matches the pdf: pdf-prep indirection.

Why no apt-get upgrade -y in pa11y-deps-force. The existing pdf-prep-force has it, but that's actually a footgun: on a CI runner it can pull in unrelated security updates mid-build and obscure failures, and on a dev machine it's destructive in a way the user probably didn't consent to. I left pdf-prep-force alone (don't expand scope of this change), but the new target should be the cleaner pattern. If you want, drop apt-get upgrade -y from pdf-prep-force too in a follow-up.

Why the trailing \ on the install line. Matches the existing pdf-prep-force cosmetic. It's a no-op continuation to a blank line and doesn't break anything. Keeping it means a future diff against sphinx-stack stays clean.

Why 36 packages and not the 10 I had before. The full list is the official Puppeteer troubleshooting list (https://pptr.dev/troubleshooting) for the Chromium binary that pa11y 9's bundled Puppeteer 24 actually downloads. I had a partial and partially-wrong list in my earlier message; this one is verbatim from the upstream source, which is what you want as a source of truth.

Why npm is NOT in REQPA11YPACKS. The Puppeteer list is the system-deps floor for the Chromium binary. npm is a project-wide prereq (you need it for make install in many JS projects), and some users install it via nvm or NodeSource instead of apt. Mixing it in would be overreach. If your environment gets pa11y working without apt install npm, leave it out. If your docs already say sudo apt install npm as a prereq, keep that documentation as-is.

Why the variable name is REQPA11YPACKS and not DOCS_PA11YPACKAGES. Matches your existing REQPDFPACKS (v1.x style). When you upgrade to 2.0, you'd rename it then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants