codecheckers / codecheckers/codecheck
Use up-tree lib_dir (rmarkdown 2.32) instead of rewriting lib paths after render
- Dominant language
- R
- Stars
- 12
- Forks
- 5
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
rmarkdown 2.32 (merged, not yet on CRAN) contains two upstream fixes that let us delete a substantial amount of workaround code in the rendering pipeline:
- **[rstudio/rmarkdown#2632](https://github.com/rstudio/rmarkdown/pull/2632)** — `html_document_base()` no longer errors when `lib_dir` points outside the output directory. An up-tree `lib_dir: "../../libs"` now works and dependencies are referenced with up-tree relative hrefs instead of failing with *"The path <file> does not appear to be a descendant of <dir>"*. See the [maintainer's comment](https://github.com/rstudio/rmarkdown/issues/1859#issuecomment-5431523808) on #1859.
- **[rstudio/rmarkdown#1632](https://github.com/rstudio/rmarkdown/issues/1632)** (also in the 2.32 NEWS) — `render()` in a fork cluster no longer deletes sibling renders' `rmarkdown-str*.html` temp files from the shared `tempdir()`.
The first is the reason the "descendant" error forced our render-locally-then-rewrite approach. The second is the cause of the flaky parallel renders we currently sweep up after.
## What we do today
Every page renders its HTML dependencies into a page-local `libs/`, and we then rewrite the hrefs and delete that directory:
- `R/utils_render_register_htmls.R:23` — `generate_html_document_yml()` hardcodes `lib_dir: libs` (with `self_contained: false`)
- `R/utils_render_register_htmls.R:288` — `edit_html_lib_paths()` computes the depth to `docs/` and regex-rewrites `="(../)*libs/` in the rendered HTML
- `R/utils_render_register_htmls.R:276-281` and `R/utils_render_cert_htmls.R:321-324` — call it, then `unlink()` the `libs/` directory that was just written
On top of that, `R/utils_render_cert_htmls.R:232-256` sweeps up after failed parallel renders: stray `libs/` directories and stray `index_header.html` / `index_prefix.html` / `index_postfix.html` / `html_document.yml` / `temp.md` files. The inline comment names the cause precisely — *"forked processes share /tmp, causing occasional pandoc temp file conflicts"* — which is rmarkdown#1632.
`register/Makefile:49` (`make clean`) additionally has to `find docs/certs -type d -name "libs" -exec rm -rf {} +`.
## Proposed approach
1. Compute the relative depth from the page's output directory to `docs/libs` in `generate_html_document_yml()` and emit `lib_dir: <../ × depth>libs` instead of the fixed `lib_dir: libs`. The depth logic already exists inside `edit_html_lib_paths()` and can move there.
2. Delete `edit_html_lib_paths()` and its two call sites, together with the two `unlink(file.path(output_dir, "libs"))` calls.
3. Remove the parallel-render cleanup block in `R/utils_render_cert_htmls.R:232-256` once a full parallel render is confirmed clean under 2.32.
4. Drop the `libs` removal from `make clean` in the register repository.
Path resolution is unambiguous for us: `temp.md` is written into `output_dir` before rendering, so the input directory and the output directory are the same and a relative `lib_dir` resolves identically either way.
## Open question: concurrent writes into the shared `docs/libs`
Today each parallel worker owns a private `libs/` directory. Afterwards they all copy into the same one, and htmltools' dependency copy is not locked, so two workers can write the same file at the same time. Mitigation options, cheapest first:
- render one certificate serially to populate `docs/libs`, then fan out
- pre-seed `docs/libs` before the parallel section, reusing the machinery in `R/utils_external_libs.R` (`external_library_specs()`, `libs_are_current()`)
This needs to be settled before the cleanup block in step 3 is removed, otherwise we trade one race for another.
## Dependency and Dockerfile
rmarkdown 2.32 is **not on CRAN yet** (CRAN is at 2.31). Until it is released:
- `DESCRIPTION` needs `rmarkdown (>= 2.32)` in `Imports` once we rely on the new behaviour, plus a `Remotes: rstudio/rmarkdown` entry while it is unreleased
- the register's `Dockerfile` installs the package with `remotes::install_github("codecheckers/codecheck")` and `ENV R_REMOTES_UPGRADE="never"`; it must additionally install the development rmarkdown, e.g. an explicit `remotes::install_github("rstudio/rmarkdown")` step before the codecheck install, so the image does not silently fall back to the CRAN 2.31 and fail every render with the descendant error
Both the `Remotes:` entry and the extra Dockerfile step should be removed again once 2.32 reaches CRAN.
## Suggested order of work
Prototype against the GitHub build of rmarkdown and time a full parallel render before committing to it; merge when 2.32 ships.
Contributor guide
Research direction
Start by testing the GitHub build of rmarkdown 2.32 and timing a full parallel render. Read R/utils_render_register_htmls.R, R/utils_render_cert_htmls.R, R/utils_external_libs.R, DESCRIPTION, the register Dockerfile, and register/Makefile. Done means the up-tree docs/libs approach works without path rewriting or cleanup races, dependencies are correctly pinned, and parallel renders complete cleanly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- docker, r
- Domain
- build-system, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100