Unable to build articles with custom rmarkdown output formats

[gadenbuie]

This is a new error that has just popped up recently and that might not be entirely pkgdown's fault (but I haven't been able to track down a smaller reprex).

I have a package (cleanrmd) that provides alternative vignette output formats, which I use for vignettes in several packages.

output:
  cleanrmd::html_document_clean:
    theme: new.css
    toc: true
    toc_depth: 2

I just started noticing failures building the pkgdown site with the following error

-- RMarkdown error -------------------------------------------------------------------
Error in `map()`:
ℹ In index: 1.
Caused by error in `render_rmarkdown()` at pkgdown/R/build-articles.R:280:2:
! Failed to render RMarkdown
Caused by error:
! in callr subprocess.
Caused by error:
! 'arg' should be one of “default”, “bootstrap”, “cerulean”, “cosmo”, “darkly”, “flatly”, “journal”, “lumen”, “paper”, “readable”, “sandstone”, “simplex”, “spacelab”, “united”, “yeti”

I've put together a repo with a reprex. Within that repo, building the articles with pkgdown::build_articles() will work for the standard rmarkdown::html_vignette, but will fail for the custom method.

One (probably very relevant detail) is that removing the theme value from html_document_clean() by commenting out that line will cause the articles to build correctly. (But IIUC the custom document format should be ignored entirely during the rendering.)

output:
  cleanrmd::html_document_clean:
    # theme: new.css
    toc: true
    toc_depth: 2

[gadenbuie]

My pkgdown CI run ran correctly about a week ago, it seems like the most likely culprit is the recent rmarkdown 2.22 release, but I don't immediately see anything relevant in the changes between 2.21 and 2.22.

Here's the diff between session info results: https://www.diffchecker.com/mNuGIyyo/

[maelle]

Thanks for the reprex and investigation!

From a quick thus not thorough exploration, it seems that rmarkdown gets some information from the YAML directly

https://github.com/rstudio/rmarkdown/blob/58ca4df16a3a5c14f277b1878f9ff5b842fe1306/R/html_resources.R#LL212C3-L212C30 (I found this commit "promising")

pkgdown passes a special output format (docs) but at that stage it seems the one that rmarkdown uses, is the one in the document. rmarkdown:::resolve_theme("new.css") gets the error you've noticed.

Maybe it's actually a rmarkdown bug then? 🤔

[maelle]

what I don't get is why the vignette can be rendered from RStudio IDE then?

[cderv]

Thanks a lot @maelle

This is indeed one of the latest fix at rstudio/rmarkdown#2486 that I believe trigger the issue. It is kind of side effect because

• pkgdown runs its own format, and ignoring the YAML
• but rmarkdown will use the YAML frontmatter of a document in its discover_rmd_resources() process
• We recently added (or at least tried) to make the process more clever regarding which resource to copy when there is a bslib theme
• cleanrmd::html_document_clean() has a theme argument which does not behave like a html_document_base() one, so the bslib related functions that was added a while ago to support new feature are not handling this case.

I just need to confirm this as I need to understand why exactly this happens when running in pkgdown and not just with rmarkdown. Something is triggered somehow

Anyway, I'll fix this in rmarkdown - not a pkgdown issue IMO. @gadenbuie I have ideas, but happy to discuss with you on this for possibly broader / better support for all this (bslib theme, custom formats own arguments, ...)

[maelle]

Thank you @cderv 🙏

[cderv]

I just need to confirm this as I need to understand why exactly this happens when running in pkgdown and not just with rmarkdown. Something is triggered somehow

I believe this is because pkgdown is explictly calling for find_external_resources() that the issue is triggered. Otherwise, with usual rendering discover_rmd_resources() is not called. So no error shown.

pkgdown/R/rmarkdown.R

Lines 62 to 70 in 39083f3

	if (copy_images) {
	ext_src <- rmarkdown::find_external_resources(input_path)
	# temporarily copy the rendered html into the input path directory and scan
	# again for additional external resources that may be been included by R code
	tempfile_in_input_dir <- file_temp(ext = "html", tmp_dir = path_dir(input_path))
	file_copy(path, tempfile_in_input_dir)
	withr::defer(unlink(tempfile_in_input_dir))
	ext_post <- rmarkdown::find_external_resources(tempfile_in_input_dir)

find_external_resources() will consider the YAML front matter. This is not new https://github.com/rstudio/rmarkdown/blob/10863429024e236389a48f40ab1485620d53d311/R/html_resources.R#L202-L203

  # parse the YAML front matter to discover resources named there
  front_matter <- yaml_front_matter(md_file)

And so some of the resources will be discovered and handled based on the YAML of the Rmd file

output:
  cleanrmd::html_document_clean:
    theme: new.css
    toc: true
    toc_depth: 2

which I understand pkgdown ignores anyway. It does not seem to have a been an issue until now... So is this an underlying issue to think about ?

We have the why this is triggered. About the actual error with cleanrmd, this is because of the above and the fact that resolve_theme(output_format[["theme"]])) is checked, but resolve_theme() does not handle the case where theme can be from another custom format. This is rmarkdown improvement for sure.

[maelle]

So is any change needed on pkgdown side? Finding external resources is useful 😁

[cderv]

I guess this is working now, as not other issue reported, but I believe you are finding more resources that you need too because finding resources includes the YAML front matter content. But output YAML key is not used IIUC.

Only change to prevent "more files to be copied" (which is probably not an issue most of the time), would be to remove output key from YAML as not used at all by pkgdown. This would prevent in rmarkdown process on it.

So it depends the pkgdown point of view on this.

[gadenbuie]

Just to confirm: rstudio/rmarkdown#2493 does indeed solve the problem for me, both in the reprex repo and in my original repo. Thanks @maelle and @cderv!