rstudio / htmltools

Tools for HTML generation and output
https://rstudio.github.io/htmltools/
215 stars 68 forks source link

copyDependencyToDir() should create outputDir recursively #331

Closed cderv closed 1 year ago

cderv commented 2 years ago

I believe the change in https://github.com/rstudio/htmltools/pull/276 and new 0.5.3 version broke Quarto rendering with HTML dependencies.

Using latest CRAN version, inside a new project with this document test.qmd

---
title: "Untitled"
format: 
  html:
    self-contained: false
---

## Title

```{r}
mtcars |> 
  kableExtra::kbl()

I got this 

output file: test.knit.md

Error in normalizePath(): ! path[1]="test_files\libs/kePrint-0.0.1": Le chemin d’accès spécifié est introuvable Backtrace: ▆

  1. └─global .main()
  2. └─execute(...)
  3. └─pandoc_includes(...)
  4. └─dependencies_from_render(input, files_dir, knit_meta, format)
  5. └─html_dependencies_as_string(extras$dependencies, files_dir)
  6. └─base::lapply(dependencies, htmltools::copyDependencyToDir, files_dir)
  7. └─htmltools (local) FUN(X[[i]], ...)
  8. └─base::normalizePath(target_dir, "/", TRUE) Messages d'avis : 1: Dans dir.create(outputDir) : impossible de créer le répertoire 'test_files\libs', à cause de 'No such file or directory' 2: Dans dir.create(target_dir) : impossible de créer le répertoire 'test_files\libs\kePrint-0.0.1', à cause de 'No such file or directory' Exécution arrêtée

Quarto is passing test_files\\libs to outputDir in copyDependencyToDir() and with the recent change, if test_files does not exist, it will error this way. Creating test_files folder before rendering, or installing htmltools 0.5.2 will make this example works.

For the change in #276, I think there could have been some side effect where https://github.com/rstudio/htmltools/blob/6a03c3f35fbe6bfd0f91ba0607808a2b9127c5e5/R/html_dependency.R#L336-L337 was throwing a warning anyway as recursive = TRUE is not set. With 0.5.2 I am seeing

#> Warning messages:
#> 1: In dir.create(outputDir) :
#>   cannot create dir 'test_files\libs', reason 'No such file or directory'
#> 2: In dir.create(target_dir) :
#>   cannot create dir 'test_files\libs\kePrint-0.0.1', reason 'No such file or directory'

But the document is still rendering correctly.

Could we set recursive = TRUE for outputDir creation maybe ?

Or should outputDir not be set to a path, but just a folder name ? In that case this would be to change in Quarto. The fact I see warning in both 0.5.2 and 0.5.3 makes me wonder. it is just working with dependencies copied in 0.5.2 but erroring with 0.5.3

or maybe I am looking in the wrong place. I am not convinced yet, so still look, but sharing now with you in case it can help

cc @gadenbuie as you made the changed. Seems like some side effect here going on.

cderv commented 2 years ago

I am still unsure what was the change that breaks Quarto, but as this was really problem, quarto is now ensuring that the directory exists before calling the htmltools function. https://github.com/quarto-dev/quarto-cli/commit/2e9a92cdc9bacb6751515b24e0e1582ed8150f1c

cderv commented 2 years ago

or maybe I am looking in the wrong place. I am not convinced yet, so still look, but sharing now with you in case it can help

A git bisect confirmed me that #276 merge is what created the issue.

In fact the issue is :

That is what is causing the issue with 0.5.3 where it was working with 0.5.2

Hope it helps know what to do.

Reprex with R Markdown by changing lib_dir (easier to debug than quarto)

---
title: "Untitled"
output: 
  html_document:
    self_contained: false
    lib_dir: test_files/libs
---

## Title

```{r}
mtcars |> 
  kableExtra::kbl()
gadenbuie commented 2 years ago

Thanks for the analysis @cderv. I think that copyDependencyToDir() should create outputDir recursively, I added a fix for this in #332.

cderv commented 2 years ago

Thanks @gadenbuie for the PR !