Skip to content

Set lib path for quarto render #2864

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.

Sign up for GitHub

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

Merged
jayhesselberth merged 4 commits into from
Mar 14, 2025
Merged

Set lib path for quarto render #2864

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5023e8f
Set lib path for quarto render
jayhesselberth Feb 28, 2025
7573d28
Merge branch 'main' into quarto-libpath
jayhesselberth Mar 13, 2025
06eb121
Use `local_libpaths()`
jayhesselberth Mar 14, 2025
04efa01
Better description of behavior
jayhesselberth Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions R/build-quarto-articles.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ quarto_render <- function(pkg, path, quiet = TRUE, frame = caller_env()) {
write_yaml(quarto_format(pkg), metadata_path)

output_dir <- withr::local_tempdir("pkgdown-quarto-", .local_envir = frame)

# set the lib path to the pkgdown installation directory.

@hadley hadley Mar 14, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This explanation makes me wonder if this might actually be a bug in quarto_render(). @cderv it's possible that the quarto package should be doing this to ensure that you use the same library as the current R session.

@jayhesselberth jayhesselberth Mar 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is what I was trying to get at here #2830 (comment).

I would have expected quarto to use the library path of the session it's being called in. Is there a situation where this shouldn't be the case?

@hadley hadley Mar 15, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as a general rule, whenever you run code in a new process I think it should inherit the libpaths of the current process.

@cderv cderv Mar 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now more clearly.

Yes, I think the quarto R package should do that directly. It's using processx::run(), and I should probably do something like callr::r() and inherit .libPaths().

@cderv cderv Mar 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I don't see exactly how using withr::local_libpaths() does change things here. 🤔

  1. first item in .libPaths() is retrieved in lib_path
  2. and then assigned as library path using equivalent of .libPaths(lib_path)

Isn't it a roundtrip ?

So if quarto_render() is not getting .libPaths() info in the first place, how does using withr::local_libpaths() helps here ?

I was thinking I should do something more like callr::r() when I need to assign the main process .libPaths() to environment variable, so that they are passed to new subprocess.

@cderv cderv Mar 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with

---
title: "Untitled"
format: gfm
---

```{r}
#| echo: false
.libPaths()
```

And then

> withr::local_temp_libpaths()
Setting global deferred event(s).
i These will be run:
  * Automatically, when the R session ends.
  * On demand, if you call `withr::deferred_run()`.
i Use `withr::deferred_clear()` to clear them without executing.
> .libPaths()
[1] "C:/Users/chris/AppData/Local/Temp/RtmpK6lR0o/temp_libpathfe84337b7af2" "C:/Users/chris/AppData/Local/R/win-library/4.4"                        "C:/Program Files/R/R-4.4.2/library"                                   
> quarto::quarto_render("test.qmd", quiet = TRUE)
> xfun::file_string("test.md")
# Untitled


    [1] "C:/Users/chris/AppData/Local/R/win-library/4.4"
    [2] "C:/Program Files/R/R-4.4.2/library"

So the temporary library is not see for Quarto rendering.

I think R_LIBS needs to be set so that it works for sub process (that will inherit env var)

> .libPaths()
[1] "C:/Users/chris/AppData/Local/Temp/RtmpK6lR0o/temp_libpathfe84337b7af2" "C:/Users/chris/AppData/Local/R/win-library/4.4"                        "C:/Program Files/R/R-4.4.2/library"                                   
> withr::with_envvar(
+   list(R_LIBS = paste(.libPaths(), collapse = .Platform$path.sep)),
+   quarto::quarto_render("test.qmd", quiet = TRUE)
+ )

> xfun::file_string("test.md")
# Untitled


    [1] "C:/Users/chris/AppData/Local/Temp/RtmpK6lR0o/temp_libpathfe84337b7af2"
    [2] "C:/Users/chris/AppData/Local/R/win-library/4.4"                       
    [3] "C:/Program Files/R/R-4.4.2/library"

So I am not this PR does help. I am looking at making the env var setting in quarto R package directly

@jayhesselberth jayhesselberth Mar 17, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLDR I think setting lib path in quarto via callr::r is the more robust solution.

The intended behavior of this PR:

  1. When pkgdown::build_site(install = FALSE) is called, the standard library path is used (i.e., what you see above).
  2. But when pkgdown::build_site(install = TRUE) is called, the library is installed in a temporary location in a callr session, so we fetch the temporary lib path, which is prepended to .libPaths() (hence the .libPaths[1]).

This was reported to fix the issue, but I could never figure out how to trigger this on GHA, so didn't have a test case.

@cderv cderv Mar 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLDR I think setting lib path in quarto via callr::r is the more robust solution.

I can't use callr directly, but I can mimic it. I tried

But it did not solve the issue I have in quarto for windows build

So I am still trying to understand.

For pkgdown issue I could not reproduce either yet.

so we fetch the temporary lib path, which is prepended to .libPaths() (hence the .libPaths[1])

This is what I understood. But then passing it to withr::local_libpaths() will put this temporary library back in .libPaths(). As shown in example above quarto_render() will call quarto CLI, which will call R directly and this .libPaths() from initial R session will not be passed to it.

@cderv cderv Mar 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For pkgdown issue I could not reproduce either yet.

Ok I got it to reproduce locally now, and I can check that quarto-dev/quarto-r#228 solves it.

I use your repo as an example: https://github.com/jayhesselberth/pkgdown.quarto.issue in a fork. This PR does not solve the problem for me.

I just found another issue from

pkgdown/R/build-quarto-articles.R

Lines 47 to 50 in 7645a47

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}}")
} 

── Building articles ───────────────────────────────────────────────────────────
Writing articles/index.html
Reading vignettes/test-rmarkdown.Rmd
Reading vignettes/articles/test-article.qmd
Reading vignettes/test.qmd
Running `quarto render`
Error: 
! in callr subprocess.
Caused by error in `.f(.x[[i]], .y[[i]], ...)`:
! No built file found for vignettes/articles/test-article.qmd
ℹ See `$stdout` and `$stderr` for standard output and error.
Type .Last.error to see the more details.

articles\test-article.qmd is built in <temp>\pkgdown-quarto-<hash>\articles\test-article.html, but path check above is <temp>\pkgdown-quarto-<hash>\test-article.html which does not exists.

I'll open a new issue about this.

# if build_site(install = TRUE), this will be the temporary installation directory,
# otherwise the default library location
lib_path <- .libPaths()[1]
withr::local_libpaths(lib_path, .local_envir = frame)

quarto::quarto_render(
path,
metadata_file = metadata_path,
Expand Down