Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quarto articles, in the vignettes/articles/* sense, don't seem to work #2821

Closed
jennybc opened this issue Nov 23, 2024 · 7 comments · Fixed by #2826
Closed

Quarto articles, in the vignettes/articles/* sense, don't seem to work #2821

jennybc opened this issue Nov 23, 2024 · 7 comments · Fixed by #2826
Labels

Comments

@jennybc
Copy link
Member

jennybc commented Nov 23, 2024

I'm working on making usethis::use_vignette() and usethis::use_article() work for .qmd. In that PR (r-lib/usethis#2085), I feel like I've got the right files going in the right places, etc., and .qmd vignettes (vignettes/*.qmd) seem fine. But when it comes time to build the site, I see problems with anything like vignettes/articles/*.qmd:

> pkgdown::build_site()
── Installing package abcde into temporary library ──────────────────────────────────────────────────────────────────────────
── Building pkgdown site for package abcde ─────────────────────────────────────
Reading from: /Users/jenny/tmp/abcde
Writing to: /Users/jenny/tmp/abcde/docs
── Sitrep ──────────────────────────────────────────────────────────────────────
✔ URLs ok.
✔ Favicons ok.
✔ Open graph metadata ok.
✔ Articles metadata ok.
✔ Reference metadata ok.
── Initialising site ───────────────────────────────────────────────────────────
── Building home ───────────────────────────────────────────────────────────────
Writing authors.html
Reading LICENSE.md
Writing LICENSE.html
Writing LICENSE-text.html
Reading DESCRIPTION
Writing index.html
Writing 404.html
── Building function reference ─────────────────────────────────────────────────
Writing reference/index.html
── Building articles ───────────────────────────────────────────────────────────
Writing articles/index.html
Reading vignettes/articles/rmarkdown-article.Rmd
Writing articles/rmarkdown-article.html
Reading vignettes/rmarkdown-vignette.Rmd
Writing articles/rmarkdown-vignette.html
Reading vignettes/articles/quarto-article.qmd
Reading vignettes/quarto-vignette.qmd
Running `quarto render`
Run `rlang::last_trace()` to see where the error occurred.
Error:
! ! in callr subprocess.
Caused by error in `.f(.x[[i]], .y[[i]], ...)`:
! No built file found for vignettes/articles/quarto-article.qmd
Hide Traceback
    ▆
 1. └─pkgdown::build_site()
 2.   └─pkgdown:::build_site_external(...)
 3.     └─callr::r(...)
 4.       └─callr:::get_result(output = out, options)
 5.         └─throw(callr_remote_error(remerr, output), parent = fix_msg(remerr[[3]]))

Trying to just build articles also falls over:

> pkgdown::build_articles()
── Building articles ────────────────────────────────────────────────────────────────────────────────────────────────────────
Reading vignettes/articles/quarto-article.qmd
Reading vignettes/quarto-vignette.qmd
Running `quarto render`
Run `rlang::last_trace()` to see where the error occurred.
Error in `.f()`:
! No built file found for vignettes/articles/quarto-article.qmd
Hide Traceback
     ▆
  1. └─pkgdown::build_articles()
  2.   └─pkgdown:::build_quarto_articles(pkg, quiet = quiet)
  3.     ├─pkgdown:::unwrap_purrr_error(...)
  4.     │ └─base::withCallingHandlers(...)
  5.     └─purrr::walk2(...)
  6.       └─purrr::map2(.x, .y, .f, ..., .progress = .progress)
  7.         └─purrr:::map2_("list", .x, .y, .f, ..., .progress = .progress)
  8.           ├─purrr:::with_indexed_errors(...)
  9.           │ └─base::withCallingHandlers(...)
 10.           ├─purrr:::call_with_cleanup(...)
 11.           └─pkgdown (local) .f(.x[[i]], .y[[i]], ...)
 12.             └─cli::cli_abort("No built file found for {.file {input_file}}")
 13.               └─rlang::abort(...)

The toy package I'm working with: https://github.com/jennybc/abcde

In sleuthing around for packages that actually have vignettes/articles/*.qmd, I came across someone using their own fork and branch for qmd article rendering that I suspect is a workaround for what I'm seeing? 🤷‍♀️

https://github.com/ethanbass/pkgdown/tree/qmd_articles
ethanbass@8b10575

I poked around a couple of other hits and so far haven't come across any where it looks like things are "just working" for anyone with vignettes/articles/*.qmd*.

@jennybc
Copy link
Member Author

jennybc commented Nov 23, 2024

Yes I can personally attest to success with pkgdown::build_site() if I install pkgdown from the fork and branch mentioned above. They are definitely onto something promising.

@jennybc
Copy link
Member Author

jennybc commented Nov 23, 2024

Here's another toy package, that contains only a qmd article, also not able to build a pkgdown site: https://github.com/jennybc/fghij

@jennybc
Copy link
Member Author

jennybc commented Nov 26, 2024

usethis 3.1.0 is on CRAN now (the version that adds .qmd support to use_vignette() and use_article()).

jayhesselberth added a commit that referenced this issue Nov 27, 2024
@jayhesselberth
Copy link
Collaborator

The above workaround works, but is sort of clunky as it copies everything from the vignettes/articles directory.

It would be better to figure out why the rendered quarto document and resources aren't built in the correct place from the get-go. I think we may just need a better way to specifiy a destination directory in build_quarto_articles().

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Nov 28, 2024

Interestingly, this works if you call pkgdown::build_article("articles/quarto-article") directly, but fails if built via pkgdown::build_articles().

bastienchassagnol added a commit to bastienchassagnol/DrosophiliaDeconv that referenced this issue Nov 29, 2024
@venpopov
Copy link

venpopov commented Dec 7, 2024

The problem is due to the following combination of things:

build_quarto_articles() without a specific article argument, it calls quarto_render() on a temporary quarto project, in which the file structure of articles in an articles subfolder is preserved. The rendering itself is successful and these are the contents of the temporary dir

├── articles
│   ├── quarto_article.html
│   └── quarto_article_files
├── quarto_vignette.html
└── quarto_vignette_files

The problem is that in these lines

unwrap_purrr_error(purrr::walk2(qmds$file_in, qmds$file_out, function(input_file, output_file) {
built_path <- path(output_dir, path_rel(output_file, "articles"))
if (!file_exists(built_path)) {
cli::cli_abort("No built file found for {.file {input_file}}")
}

output_file refers to a path entry file_out in pkg constructed by the package_vignettes function:

pkgdown/R/package.R

Lines 344 to 350 in dfa28be

# Vignettes will be written to /articles/ with path relative to vignettes/
# *except* for vignettes in vignettes/articles, which are moved up a level
file_in <- path("vignettes", vig_path)
file_out <- path_ext_set(vig_path, ext)
file_out[!path_has_parent(file_out, "articles")] <- path(
"articles", file_out[!path_has_parent(file_out, "articles")]
)

so you end up with a pkg$vignettes table like this:

pkg <- local_pkgdown_site()
pkg <- pkg_add_file(pkg, "vignettes/articles/quarto_article.qmd")
pkg <- pkg_add_file(pkg, "vignettes/quarto_vignette.qmd")
options(width = 200)
pkg$vignettes[,1:4]
# A tibble: 2 × 4
  name                    type  file_in                               file_out                     
  <chr>                   <chr> <chr>                                 <chr>                        
1 articles/quarto_article qmd   vignettes/articles/quarto_article.qmd articles/quarto_article.html 
2 quarto_vignette         qmd   vignettes/quarto_vignette.qmd         articles/quarto_vignette.html

built_path takes the file_out argument and strips the articles folder. This works for vignettes, but for articles it fails because quarto renders them in a nested folder (as this is the project structure).

The solution in the fork @jennybc linked works because it moves the nested rendered articles out of the articles folder. I'm not sure why you think this is clunky as the only things in the articles subdir are things that you do want to move.

This was never a problem for .Rmd files because they are rendered one by one and given file_out as the target. But the current code renders quarto files as a project, which respects the folder structure. The alternative solution is to fix file_out to depend on the file_in (instead of collapsing the folder structure), but that would have to be done just for .qmd files.

@venpopov
Copy link

venpopov commented Dec 7, 2024

The reason this wasn't caught is that there are no tests for quarto articles, just quarto vignettes. This test fails:

test_that("can build quarto articles in articles folder", {
  skip_if_no_quarto()

  pkg <- local_pkgdown_site()
  pkg <- pkg_add_file(pkg, "vignettes/articles/vig1.qmd")
  pkg <- pkg_add_file(pkg, "vignettes/vig2.qmd")
  pkg <- pkg_add_file(pkg, "vignettes/articles/vig3.rmd")
  pkg <- pkg_add_file(pkg, "vignettes/vig4.rmd")

  suppressMessages(build_articles(pkg))

  expect_true(file_exists(path(pkg$dst_path, "articles/vig1.html")))
  expect_true(file_exists(path(pkg$dst_path, "articles/vig2.html")))
  expect_true(file_exists(path(pkg$dst_path, "articles/vig3.html")))
  expect_true(file_exists(path(pkg$dst_path, "articles/vig4.html")))
})

but it passes for the fork @jennybc linked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants