From 2d83a13f01d2f7eb707af73fc409208a5b905817 Mon Sep 17 00:00:00 2001 From: infra-bot Date: Sat, 15 Aug 2026 17:01:25 +0200 Subject: [PATCH 1/4] fix: isolate each parallel qmd render in a throwaway project copy Concurrent quarto renders sharing one project race on the shared project state, with two observed failure signatures: - transient 'Unable to read the extension' during format resolution - persistent crossref index corruption ('SyntaxError: Unexpected non-whitespace character after JSON' in crossrefIndexForOutputFile) when several workers interleave writes in .quarto/ Each render now runs in a temporary project where the repository tree is symlinked (here::here() and data paths keep resolving), while _quarto*.yml, _extensions/ and the qmd's chapter are private copies, so every render gets its own fresh .quarto/ state. Outputs (html, extracted media) are copied back to the real chapter. Controlled by option 'squash.isolate_render' (default TRUE) or the new 'isolate' argument. render_single_qmd() now also reports conditionMessage() of the underlying error instead of discarding it. --- R/isolate_qmd_render.R | 151 +++++++++++++++++++++++++++++++++++++++++ R/render_single_qmd.R | 43 ++++++++++-- 2 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 R/isolate_qmd_render.R diff --git a/R/isolate_qmd_render.R b/R/isolate_qmd_render.R new file mode 100644 index 0000000..b9028de --- /dev/null +++ b/R/isolate_qmd_render.R @@ -0,0 +1,151 @@ +#' Build an isolated quarto project to render a single qmd +#' +#' Concurrent `quarto render` processes sharing one quarto project race on the +#' project state (`.quarto/`, `_extensions/` resolution), which surfaces as +#' transient "Unable to read the extension" failures. This helper materializes +#' a throwaway project in `tempdir()` where the render can run alone: +#' the repository tree is exposed through symlinks (so `here::here()` and +#' data paths keep resolving), while the project configuration +#' (`_quarto*.yml`), `_extensions/` and the chapter of the target qmd are +#' real private copies. Each isolated render therefore gets its own fresh +#' `.quarto/` state. +#' +#' @param qmd character. Path to the qmd file to render. +#' +#' @return A list with `iso_qmd` (path of the qmd inside the isolated +#' project), `iso_chapter`, `real_chapter`, `iso_root` and +#' `files_before` (chapter files present before rendering). +#' +#' @noRd +build_isolated_project <- function(qmd) { + qmd <- normalizePath(qmd, mustWork = TRUE) + chapter <- dirname(qmd) + + proj <- chapter + while (!any(file.exists(file.path(proj, c("_quarto.yml", "_quarto.yaml"))))) { + parent <- dirname(proj) + if (identical(parent, proj)) { + stop("No quarto project found above ", qmd) + } + proj <- parent + } + root <- dirname(proj) + + iso <- tempfile(pattern = "squash_iso_") + dir.create(iso) + + # repository level: symlink everything except the quarto project dir + for (entry in list.files(root, all.files = TRUE, no.. = TRUE)) { + if (entry != basename(proj)) { + file.symlink(from = file.path(root, entry), to = file.path(iso, entry)) + } + } + # make sure here::here() anchors on the isolated root + if (!file.exists(file.path(iso, ".here"))) { + file.create(file.path(iso, ".here")) + } + + rel_parts <- strsplit( + substring(chapter, nchar(proj) + 2), + split = "/", + fixed = TRUE + )[[1]] + + # project level: private copies of config and extensions, own .quarto + iso_cur <- file.path(iso, basename(proj)) + dir.create(iso_cur) + for (entry in list.files(proj, all.files = TRUE, no.. = TRUE)) { + if (entry %in% c(".quarto", "_freeze", rel_parts[1])) { + next + } + if (grepl("^_quarto.*\\.ya?ml$", entry) || entry == "_extensions") { + file.copy( + from = file.path(proj, entry), + to = iso_cur, + recursive = TRUE + ) + } else { + file.symlink( + from = file.path(proj, entry), + to = file.path(iso_cur, entry) + ) + } + } + + # descend to the chapter: intermediate levels are real dirs with + # symlinked siblings, the chapter itself is a real copy + real_cur <- proj + for (i in seq_along(rel_parts)) { + real_cur <- file.path(real_cur, rel_parts[i]) + iso_next <- file.path(iso_cur, rel_parts[i]) + dir.create(iso_next) + if (i < length(rel_parts)) { + for (entry in list.files(real_cur, all.files = TRUE, no.. = TRUE)) { + if (entry != rel_parts[i + 1]) { + file.symlink( + from = file.path(real_cur, entry), + to = file.path(iso_next, entry) + ) + } + } + } else { + file.copy( + from = list.files( + real_cur, + full.names = TRUE, + all.files = TRUE, + no.. = TRUE + ), + to = iso_next, + recursive = TRUE + ) + } + iso_cur <- iso_next + } + + files_before <- list.files( + iso_cur, + recursive = TRUE, + all.files = TRUE, + full.names = FALSE + ) + + return( + list( + iso_qmd = file.path(iso_cur, basename(qmd)), + iso_chapter = iso_cur, + real_chapter = chapter, + iso_root = iso, + files_before = files_before + ) + ) +} + +#' Copy files produced by an isolated render back to the real chapter +#' +#' @param isolation list. As returned by [build_isolated_project()]. +#' +#' @return Invisibly, the copied file paths. +#' +#' @noRd +collect_isolated_outputs <- function(isolation) { + files_after <- list.files( + isolation$iso_chapter, + recursive = TRUE, + all.files = TRUE, + full.names = FALSE + ) + new_files <- setdiff(files_after, isolation$files_before) + + for (rel_file in new_files) { + dest <- file.path(isolation$real_chapter, rel_file) + dir.create(dirname(dest), recursive = TRUE, showWarnings = FALSE) + file.copy( + from = file.path(isolation$iso_chapter, rel_file), + to = dest, + overwrite = TRUE + ) + } + + return(invisible(file.path(isolation$real_chapter, new_files))) +} diff --git a/R/render_single_qmd.R b/R/render_single_qmd.R index 7f947ca..1cb9f65 100644 --- a/R/render_single_qmd.R +++ b/R/render_single_qmd.R @@ -6,6 +6,7 @@ #' @param img_root_dir character. Path to the main image folder to extract media to #' @param metadata list. List of metadata to be used for rendering single qmd file #' @param purrr_insistently_rate_backoff function. Function to use to retry rendering qmd files in case of failure. Should be a purrr::rate_backoff function. +#' @param isolate logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state. #' #' @inheritParams compile_qmd_course #' @@ -51,7 +52,8 @@ render_single_qmd <- function( purrr_insistently_rate_backoff = purrr::rate_backoff( pause_base = 0.1, max_times = 5 - ) + ), + isolate = getOption("squash.isolate_render", TRUE) ) { # set image sub-folder name chapter <- dirname(qmd) @@ -66,22 +68,53 @@ render_single_qmd <- function( rate = purrr_insistently_rate_backoff ) + # concurrent renders sharing one quarto project race on the project + # state, so each render runs in its own throwaway copy of the project + isolation <- NULL + if (isTRUE(isolate)) { + isolation <- tryCatch( + expr = { + build_isolated_project(qmd) + }, + error = \(error_message) { + cli_alert_danger( + "Could not isolate {qmd} ({conditionMessage(error_message)}), rendering in place" + ) + NULL + } + ) + } + + input <- if (is.null(isolation)) { + qmd + } else { + isolation$iso_qmd + } + # try rendering qmd and warn user if successful / fail tryCatch( expr = { quarto_render_insistently( - input = qmd, + input = input, metadata = c(metadata, list(`extract-media` = img_dir)), output_format = output_format, quiet = quiet ) + if (!is.null(isolation)) { + collect_isolated_outputs(isolation) + } return(TRUE) }, error = \(error_message) { - cli_alert_danger("Failed to render {qmd}, cleaning and existing") - - # throw an error with original error message + cli_alert_danger( + "Failed to render {qmd} ({conditionMessage(error_message)}), cleaning and exiting" + ) return(FALSE) + }, + finally = { + if (!is.null(isolation)) { + unlink(isolation$iso_root, recursive = TRUE) + } } ) } From 1df3d2428a53f999d996da475a205b4893f22bd7 Mon Sep 17 00:00:00 2001 From: infra-bot Date: Sat, 15 Aug 2026 17:26:13 +0200 Subject: [PATCH 2/4] chore: bump version to 1.2.0.9001 (parallel render isolation) --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 00f9dc6..e07da6a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: squash Title: Several Quarto As Single HTML -Version: 1.2.0 +Version: 1.2.0.9001 Authors@R: c( person("Antoine", "Languillaume", , "antoine@thinkr.fr", role = c("aut", "cre")), person("Colin", "Fay", , "colin@thinkr.fr", role = "aut"), From 4c44a717ed73218a9d4b757b491b7cd6afc8fd72 Mon Sep 17 00:00:00 2001 From: Vincent Guyader Date: Sat, 15 Aug 2026 19:45:40 +0200 Subject: [PATCH 3/4] test: unit tests for isolated render project + docs and NEWS - unit tests for build_isolated_project() (private copies vs symlinks, here anchor, no shared .quarto) and collect_isolated_outputs() - NEWS entry for 1.2.0.9001 - regenerated docs (roxygen2 8.0.0 migrates RoxygenNote to Config/roxygen2/version) --- DESCRIPTION | 2 +- NEWS.md | 5 ++ man/render_single_qmd.Rd | 5 +- man/squash-package.Rd | 1 + tests/testthat/test-isolate_qmd_render.R | 72 ++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/test-isolate_qmd_render.R diff --git a/DESCRIPTION b/DESCRIPTION index e07da6a..beff872 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -39,5 +39,5 @@ Config/fusen/version: 0.6.0 Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.2 SystemRequirements: quarto (>=1.3.0) +Config/roxygen2/version: 8.0.0 diff --git a/NEWS.md b/NEWS.md index d2741be..6943d21 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# squash 1.2.0.9001 (development version) + +* Each qmd is now rendered inside an isolated throwaway copy of the quarto project (option `squash.isolate_render`, `TRUE` by default). Concurrent renders no longer share the project state (`.quarto/` crossref index, `_extensions/` resolution), which removes the random `Failed to render` errors of parallel compilation (quarto-dev/quarto-cli#2749). +* `render_single_qmd()` now reports the underlying error message instead of discarding it. + # squash 1.2.0 * The rendering of the Qmd is now performed insistently with purrr::insistently(), and can be configured in the rendering functions. diff --git a/man/render_single_qmd.Rd b/man/render_single_qmd.Rd index 1d17d00..8014611 100644 --- a/man/render_single_qmd.Rd +++ b/man/render_single_qmd.Rd @@ -10,7 +10,8 @@ render_single_qmd( output_format = "revealjs", metadata = NULL, quiet = TRUE, - purrr_insistently_rate_backoff = purrr::rate_backoff(pause_base = 0.1, max_times = 5) + purrr_insistently_rate_backoff = purrr::rate_backoff(pause_base = 0.1, max_times = 5), + isolate = getOption("squash.isolate_render", TRUE) ) } \arguments{ @@ -25,6 +26,8 @@ render_single_qmd( \item{quiet}{logical. Output info in user console} \item{purrr_insistently_rate_backoff}{function. Function to use to retry rendering qmd files in case of failure. Should be a purrr::rate_backoff function.} + +\item{isolate}{logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state.} } \value{ logical. TRUE if rendering succeeded, FALSE otherwise. Side effect : render qmd as html diff --git a/man/squash-package.Rd b/man/squash-package.Rd index 42f079e..d5f6f73 100644 --- a/man/squash-package.Rd +++ b/man/squash-package.Rd @@ -23,6 +23,7 @@ Useful links: Authors: \itemize{ + \item Antoine Languillaume \email{antoine@thinkr.fr} \item Colin Fay \email{colin@thinkr.fr} \item Swann Floc'hlay \email{swann@thinkr.fr} } diff --git a/tests/testthat/test-isolate_qmd_render.R b/tests/testthat/test-isolate_qmd_render.R new file mode 100644 index 0000000..e683daf --- /dev/null +++ b/tests/testthat/test-isolate_qmd_render.R @@ -0,0 +1,72 @@ +test_that("build_isolated_project creates a private project and keeps the repo reachable", { + # fake repo: root/(data.csv)/courses/(_quarto.yml, _extensions, M01/S01/file.qmd) + root <- tempfile(pattern = "iso_repo") + chapter <- file.path(root, "courses", "M01", "S01") + sibling <- file.path(root, "courses", "M01", "S02") + ext <- file.path(root, "courses", "_extensions", "acme", "theme") + + dir.create(chapter, recursive = TRUE) + dir.create(sibling, recursive = TRUE) + dir.create(ext, recursive = TRUE) + + writeLines("format: revealjs", file.path(root, "courses", "_quarto.yml")) + writeLines("title: theme", file.path(ext, "_extension.yml")) + writeLines("a,b", file.path(root, "data.csv")) + qmd <- file.path(chapter, "doc.qmd") + writeLines("# doc", qmd) + writeLines("img", file.path(chapter, "logo.png")) + writeLines("# other", file.path(sibling, "other.qmd")) + + isolation <- build_isolated_project(qmd) + on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) + + iso_courses <- file.path(isolation$iso_root, "courses") + + # chapter and its content are real copies + expect_true(file.exists(isolation$iso_qmd)) + expect_false(nzchar(Sys.readlink(isolation$iso_chapter))) + expect_true(file.exists(file.path(isolation$iso_chapter, "logo.png"))) + + # project config and extensions are private copies, not symlinks + expect_true(file.exists(file.path(iso_courses, "_quarto.yml"))) + expect_false(nzchar(Sys.readlink(file.path(iso_courses, "_extensions")))) + expect_true( + file.exists(file.path(iso_courses, "_extensions", "acme", "theme", "_extension.yml")) + ) + + # sibling chapter and repo-level files stay reachable through symlinks + expect_true(nzchar(Sys.readlink(file.path(iso_courses, "M01", "S02")))) + expect_true(file.exists(file.path(iso_courses, "M01", "S02", "other.qmd"))) + expect_true(file.exists(file.path(isolation$iso_root, "data.csv"))) + + # here::here() anchor is present at the isolated root + expect_true(file.exists(file.path(isolation$iso_root, ".here"))) + + # no shared .quarto state is exposed in the isolated project + expect_false(dir.exists(file.path(iso_courses, ".quarto"))) +}) + +test_that("collect_isolated_outputs copies rendering outputs back to the real chapter", { + root <- tempfile(pattern = "iso_repo") + chapter <- file.path(root, "courses", "M01", "S01") + dir.create(chapter, recursive = TRUE) + writeLines("format: revealjs", file.path(root, "courses", "_quarto.yml")) + qmd <- file.path(chapter, "doc.qmd") + writeLines("# doc", qmd) + + isolation <- build_isolated_project(qmd) + on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) + + # simulate a render: html + media dir appear in the isolated chapter + writeLines("", file.path(isolation$iso_chapter, "doc.html")) + media <- file.path(isolation$iso_chapter, "out_img", "S01_img") + dir.create(media, recursive = TRUE) + writeLines("img", file.path(media, "fig.png")) + + collect_isolated_outputs(isolation) + + expect_true(file.exists(file.path(chapter, "doc.html"))) + expect_true(file.exists(file.path(chapter, "out_img", "S01_img", "fig.png"))) + # pre-existing files are not duplicated or altered + expect_identical(readLines(file.path(chapter, "doc.qmd")), "# doc") +}) From 8898afde4a63872b640347613ebf5b42ef91bb13 Mon Sep 17 00:00:00 2001 From: Vincent Guyader Date: Sat, 15 Aug 2026 20:24:40 +0200 Subject: [PATCH 4/4] fix: harden isolation after review, release 1.3.0 Review findings addressed (pr-reviewer + Copilot triage): - copy-back now diffs an inventory (path, mtime, size) instead of names only, so re-renders refresh outputs that already existed in the chapter (stale-output bug, reproduced end to end) - every dir.create/file.symlink/file.copy is checked and stops on failure, so the documented render-in-place fallback actually fires (on Windows without the symlink privilege, isolation now degrades cleanly to the previous behavior) - the exposed tree is bounded to the enclosing repository (.git/.here anchor); without such an anchor nothing above the project is exposed - a qmd sitting directly in the project directory gets a real private copy (rel_parts length 0 was unhandled) - projects with a project-level output-dir (websites, books) are refused with a typed condition and fall back to rendering in place - a failed build no longer leaks the partially built isolated tree - messages: no more 'cleaning and exiting' narration; benign fallbacks are info/warning instead of danger - tests: symlink capability probe with skip, fixture leaks fixed, second-render regression test, real quarto integration test (render, edit, re-render, fresh content, no leftover tree) - skip_if_no_chrome() now probes an actual headless print, not just the binary (the devel CI leg failed on a pre-existing test because Chrome is present but headless printing is not functional there) - version 1.3.0, NEWS updated; RoxygenNote churn reverted --- DESCRIPTION | 4 +- NEWS.md | 4 +- R/isolate_qmd_render.R | 310 ++++++++++++++++++----- R/render_single_qmd.R | 17 +- man/render_single_qmd.Rd | 2 +- man/squash-package.Rd | 1 - tests/testthat/test-compile_qmd_course.R | 19 ++ tests/testthat/test-isolate_qmd_render.R | 173 +++++++++++-- 8 files changed, 439 insertions(+), 91 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index beff872..db22c1b 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: squash Title: Several Quarto As Single HTML -Version: 1.2.0.9001 +Version: 1.3.0 Authors@R: c( person("Antoine", "Languillaume", , "antoine@thinkr.fr", role = c("aut", "cre")), person("Colin", "Fay", , "colin@thinkr.fr", role = "aut"), @@ -39,5 +39,5 @@ Config/fusen/version: 0.6.0 Config/testthat/edition: 3 Encoding: UTF-8 Roxygen: list(markdown = TRUE) +RoxygenNote: 7.3.2 SystemRequirements: quarto (>=1.3.0) -Config/roxygen2/version: 8.0.0 diff --git a/NEWS.md b/NEWS.md index 6943d21..92a6a62 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,8 @@ -# squash 1.2.0.9001 (development version) +# squash 1.3.0 * Each qmd is now rendered inside an isolated throwaway copy of the quarto project (option `squash.isolate_render`, `TRUE` by default). Concurrent renders no longer share the project state (`.quarto/` crossref index, `_extensions/` resolution), which removes the random `Failed to render` errors of parallel compilation (quarto-dev/quarto-cli#2749). +* Isolation falls back to rendering in place whenever it is not possible: no quarto project above the qmd, project with a project-level `output-dir` (websites, books), or filesystem without symlink support (e.g. Windows without the symlink privilege). Every filesystem step of the isolation is checked so a partial isolated project can never be rendered silently. +* Outputs are copied back to the real chapter when they are new or updated, so re-renders refresh files left by a previous run. * `render_single_qmd()` now reports the underlying error message instead of discarding it. # squash 1.2.0 diff --git a/R/isolate_qmd_render.R b/R/isolate_qmd_render.R index b9028de..2358c8c 100644 --- a/R/isolate_qmd_render.R +++ b/R/isolate_qmd_render.R @@ -2,19 +2,26 @@ #' #' Concurrent `quarto render` processes sharing one quarto project race on the #' project state (`.quarto/`, `_extensions/` resolution), which surfaces as -#' transient "Unable to read the extension" failures. This helper materializes -#' a throwaway project in `tempdir()` where the render can run alone: -#' the repository tree is exposed through symlinks (so `here::here()` and -#' data paths keep resolving), while the project configuration -#' (`_quarto*.yml`), `_extensions/` and the chapter of the target qmd are -#' real private copies. Each isolated render therefore gets its own fresh -#' `.quarto/` state. +#' transient "Unable to read the extension" failures and corrupted crossref +#' indexes. This helper materializes a throwaway project in `tempdir()` where +#' the render can run alone: the repository tree is exposed through symlinks +#' (so `here::here()` and data paths keep resolving), while the project +#' configuration (`_quarto*.yml`), `_extensions/` and the chapter of the +#' target qmd are real private copies. Each isolated render therefore gets +#' its own fresh `.quarto/` state. +#' +#' Every filesystem step is checked: on any failure (symlinks forbidden, +#' restricted mounts, Windows without the symlink privilege) an error is +#' raised so the caller can fall back to rendering in place. Projects that +#' define a `project: output-dir:` (websites, books) are refused as well, +#' because their outputs land outside the chapter and could not be collected. #' #' @param qmd character. Path to the qmd file to render. #' #' @return A list with `iso_qmd` (path of the qmd inside the isolated #' project), `iso_chapter`, `real_chapter`, `iso_root` and -#' `files_before` (chapter files present before rendering). +#' `files_before` (inventory of the isolated chapter before rendering, +#' with modification time and size). #' #' @noRd build_isolated_project <- function(qmd) { @@ -25,103 +32,143 @@ build_isolated_project <- function(qmd) { while (!any(file.exists(file.path(proj, c("_quarto.yml", "_quarto.yaml"))))) { parent <- dirname(proj) if (identical(parent, proj)) { - stop("No quarto project found above ", qmd) + stop( + errorCondition( + message = paste0("No quarto project found above ", qmd), + class = c("squash_no_project", "error", "condition") + ) + ) } proj <- parent } - root <- dirname(proj) + + refuse_project_with_output_dir(proj = proj) iso <- tempfile(pattern = "squash_iso_") - dir.create(iso) + dir_create_or_stop(iso) + cleanup_armed <- TRUE + on.exit( + { + if (cleanup_armed) { + unlink(iso, recursive = TRUE) + } + }, + add = TRUE + ) - # repository level: symlink everything except the quarto project dir - for (entry in list.files(root, all.files = TRUE, no.. = TRUE)) { - if (entry != basename(proj)) { - file.symlink(from = file.path(root, entry), to = file.path(iso, entry)) + # repository level: symlink everything except the quarto project dir. + # The exposed root is bounded to the enclosing repository (.git / .here + # anchor): without such an anchor, nothing above the project is exposed. + root <- find_repository_root(proj = proj) + if (!is.null(root)) { + rel_proj_parts <- split_path_below(root = root, descendant = proj) + iso_cur <- iso + real_cur <- root + for (i in seq_along(rel_proj_parts)) { + link_siblings_or_stop( + real_dir = real_cur, + iso_dir = iso_cur, + keep_real = rel_proj_parts[i] + ) + real_cur <- file.path(real_cur, rel_proj_parts[i]) + iso_cur <- file.path(iso_cur, rel_proj_parts[i]) + if (i < length(rel_proj_parts)) { + dir_create_or_stop(iso_cur) + } } + iso_proj <- iso_cur + } else { + iso_proj <- file.path(iso, basename(proj)) } + # make sure here::here() anchors on the isolated root if (!file.exists(file.path(iso, ".here"))) { - file.create(file.path(iso, ".here")) + if (!file.create(file.path(iso, ".here"))) { + stop("Could not create ", file.path(iso, ".here")) + } } - rel_parts <- strsplit( - substring(chapter, nchar(proj) + 2), - split = "/", - fixed = TRUE - )[[1]] - # project level: private copies of config and extensions, own .quarto - iso_cur <- file.path(iso, basename(proj)) - dir.create(iso_cur) + dir_create_or_stop(iso_proj) + rel_parts <- split_path_below(root = proj, descendant = chapter) + first_kept <- if (length(rel_parts) > 0) { + rel_parts[1] + } else { + NULL + } for (entry in list.files(proj, all.files = TRUE, no.. = TRUE)) { - if (entry %in% c(".quarto", "_freeze", rel_parts[1])) { + if (entry %in% c(".quarto", "_freeze")) { + next + } + if (identical(entry, first_kept)) { next } - if (grepl("^_quarto.*\\.ya?ml$", entry) || entry == "_extensions") { - file.copy( + is_config <- grepl("^_quarto.*\\.ya?ml$", entry) || entry == "_extensions" + # when the qmd sits directly in the project dir, the project dir IS the + # chapter: plain files must be real private copies as well + is_chapter_file <- length(rel_parts) == 0 && + !dir.exists(file.path(proj, entry)) + if (is_config || is_chapter_file) { + copy_or_stop( from = file.path(proj, entry), - to = iso_cur, + to = iso_proj, recursive = TRUE ) } else { - file.symlink( + link_or_stop( from = file.path(proj, entry), - to = file.path(iso_cur, entry) + to = file.path(iso_proj, entry) ) } } # descend to the chapter: intermediate levels are real dirs with # symlinked siblings, the chapter itself is a real copy + iso_cur <- iso_proj real_cur <- proj for (i in seq_along(rel_parts)) { real_cur <- file.path(real_cur, rel_parts[i]) iso_next <- file.path(iso_cur, rel_parts[i]) - dir.create(iso_next) + dir_create_or_stop(iso_next) if (i < length(rel_parts)) { - for (entry in list.files(real_cur, all.files = TRUE, no.. = TRUE)) { - if (entry != rel_parts[i + 1]) { - file.symlink( - from = file.path(real_cur, entry), - to = file.path(iso_next, entry) - ) - } - } + link_siblings_or_stop( + real_dir = real_cur, + iso_dir = iso_next, + keep_real = rel_parts[i + 1] + ) } else { - file.copy( - from = list.files( - real_cur, - full.names = TRUE, - all.files = TRUE, - no.. = TRUE - ), - to = iso_next, - recursive = TRUE + entries <- list.files( + real_cur, + full.names = TRUE, + all.files = TRUE, + no.. = TRUE ) + if (length(entries) > 0) { + copy_or_stop(from = entries, to = iso_next, recursive = TRUE) + } } iso_cur <- iso_next } - files_before <- list.files( - iso_cur, - recursive = TRUE, - all.files = TRUE, - full.names = FALSE - ) + iso_chapter <- iso_cur + cleanup_armed <- FALSE return( list( - iso_qmd = file.path(iso_cur, basename(qmd)), - iso_chapter = iso_cur, + iso_qmd = file.path(iso_chapter, basename(qmd)), + iso_chapter = iso_chapter, real_chapter = chapter, iso_root = iso, - files_before = files_before + files_before = chapter_inventory(iso_chapter) ) ) } -#' Copy files produced by an isolated render back to the real chapter +#' Copy files produced or updated by an isolated render back to the real chapter +#' +#' Compares the isolated chapter with the inventory captured before rendering: +#' files that are new, or whose size or modification time changed, are copied +#' back (so a re-render refreshes outputs that already existed). #' #' @param isolation list. As returned by [build_isolated_project()]. #' @@ -129,15 +176,16 @@ build_isolated_project <- function(qmd) { #' #' @noRd collect_isolated_outputs <- function(isolation) { - files_after <- list.files( - isolation$iso_chapter, - recursive = TRUE, - all.files = TRUE, - full.names = FALSE - ) - new_files <- setdiff(files_after, isolation$files_before) + after <- chapter_inventory(isolation$iso_chapter) + before <- isolation$files_before + + is_new <- !(after$path %in% before$path) + matched <- match(after$path, before$path) + is_changed <- !is_new & + (after$mtime != before$mtime[matched] | after$size != before$size[matched]) + to_copy <- after$path[is_new | is_changed] - for (rel_file in new_files) { + for (rel_file in to_copy) { dest <- file.path(isolation$real_chapter, rel_file) dir.create(dirname(dest), recursive = TRUE, showWarnings = FALSE) file.copy( @@ -147,5 +195,133 @@ collect_isolated_outputs <- function(isolation) { ) } - return(invisible(file.path(isolation$real_chapter, new_files))) + return(invisible(file.path(isolation$real_chapter, to_copy))) +} + +#' Inventory of a chapter directory: relative path, mtime, size +#' @noRd +chapter_inventory <- function(dir) { + paths <- list.files( + dir, + recursive = TRUE, + all.files = TRUE, + full.names = FALSE + ) + info <- file.info(file.path(dir, paths)) + return( + data.frame( + path = paths, + mtime = as.numeric(info$mtime), + size = info$size, + stringsAsFactors = FALSE + ) + ) +} + +#' Nearest ancestor of the project marking a repository root (.git or .here) +#' +#' Returns NULL when no marker exists above the project: in that case nothing +#' above the project directory is exposed in the isolated tree. +#' +#' @noRd +find_repository_root <- function(proj) { + cur <- dirname(proj) + while (!identical(cur, dirname(cur))) { + if (any(file.exists(file.path(cur, c(".git", ".here"))))) { + return(cur) + } + cur <- dirname(cur) + } + return(NULL) +} + +#' Split the path of `descendant` relative to `root` into components +#' @noRd +split_path_below <- function(root, descendant) { + if (identical(root, descendant)) { + return(character(0)) + } + rel <- substring(descendant, nchar(root) + 2) + return(strsplit(rel, split = "/", fixed = TRUE)[[1]]) +} + +#' Refuse isolation for projects rendering into a project-level output-dir +#' @importFrom yaml read_yaml +#' @noRd +refuse_project_with_output_dir <- function(proj) { + configs <- list.files( + proj, + pattern = "^_quarto.*\\.ya?ml$", + full.names = TRUE + ) + for (config in configs) { + parsed <- tryCatch( + expr = { + read_yaml(config) + }, + error = \(error_message) { + NULL + } + ) + if (!is.null(parsed$project$`output-dir`)) { + stop( + errorCondition( + message = paste0( + "Project ", proj, " renders into an output-dir (", + parsed$project$`output-dir`, + "), its outputs could not be collected from an isolated copy" + ), + class = c("squash_no_isolation", "error", "condition") + ) + ) + } + } + return(invisible(NULL)) +} + +#' dir.create that errors on failure +#' @noRd +dir_create_or_stop <- function(path) { + if (!dir.create(path, showWarnings = FALSE)) { + stop("Could not create directory ", path) + } + return(invisible(path)) +} + +#' file.symlink that errors on failure +#' @noRd +link_or_stop <- function(from, to) { + ok <- suppressWarnings(file.symlink(from = from, to = to)) + if (!isTRUE(all(ok))) { + stop("Could not symlink ", from, " -> ", to) + } + return(invisible(to)) +} + +#' file.copy that errors on failure +#' @noRd +copy_or_stop <- function(from, to, recursive = FALSE) { + ok <- file.copy(from = from, to = to, recursive = recursive) + if (!isTRUE(all(ok))) { + stop( + "Could not copy ", + paste(from[!ok], collapse = ", "), + " into ", to + ) + } + return(invisible(to)) +} + +#' Symlink every entry of real_dir into iso_dir, except keep_real +#' @noRd +link_siblings_or_stop <- function(real_dir, iso_dir, keep_real) { + for (entry in list.files(real_dir, all.files = TRUE, no.. = TRUE)) { + if (!identical(entry, keep_real)) { + link_or_stop( + from = file.path(real_dir, entry), + to = file.path(iso_dir, entry) + ) + } + } + return(invisible(iso_dir)) } diff --git a/R/render_single_qmd.R b/R/render_single_qmd.R index 1cb9f65..8d42e1e 100644 --- a/R/render_single_qmd.R +++ b/R/render_single_qmd.R @@ -6,7 +6,7 @@ #' @param img_root_dir character. Path to the main image folder to extract media to #' @param metadata list. List of metadata to be used for rendering single qmd file #' @param purrr_insistently_rate_backoff function. Function to use to retry rendering qmd files in case of failure. Should be a purrr::rate_backoff function. -#' @param isolate logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state. +#' @param isolate logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state. Outputs are copied back to the real chapter afterwards. When isolation is not possible (no quarto project above the qmd, project with a project-level output-dir, filesystem without symlink support), the qmd is rendered in place as before. #' #' @inheritParams compile_qmd_course #' @@ -76,8 +76,19 @@ render_single_qmd <- function( expr = { build_isolated_project(qmd) }, + squash_no_project = \(condition) { + # no quarto project above the qmd: there is no shared project + # state to isolate from, rendering in place is the normal path + NULL + }, + squash_no_isolation = \(condition) { + cli_alert_info( + "{conditionMessage(condition)}, rendering in place" + ) + NULL + }, error = \(error_message) { - cli_alert_danger( + cli_alert_warning( "Could not isolate {qmd} ({conditionMessage(error_message)}), rendering in place" ) NULL @@ -107,7 +118,7 @@ render_single_qmd <- function( }, error = \(error_message) { cli_alert_danger( - "Failed to render {qmd} ({conditionMessage(error_message)}), cleaning and exiting" + "Failed to render {qmd} ({conditionMessage(error_message)})" ) return(FALSE) }, diff --git a/man/render_single_qmd.Rd b/man/render_single_qmd.Rd index 8014611..281c811 100644 --- a/man/render_single_qmd.Rd +++ b/man/render_single_qmd.Rd @@ -27,7 +27,7 @@ render_single_qmd( \item{purrr_insistently_rate_backoff}{function. Function to use to retry rendering qmd files in case of failure. Should be a purrr::rate_backoff function.} -\item{isolate}{logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state.} +\item{isolate}{logical. If TRUE (default, controlled by option "squash.isolate_render"), render inside a throwaway copy of the quarto project so concurrent renders do not share project state. Outputs are copied back to the real chapter afterwards. When isolation is not possible (no quarto project above the qmd, project with a project-level output-dir, filesystem without symlink support), the qmd is rendered in place as before.} } \value{ logical. TRUE if rendering succeeded, FALSE otherwise. Side effect : render qmd as html diff --git a/man/squash-package.Rd b/man/squash-package.Rd index d5f6f73..42f079e 100644 --- a/man/squash-package.Rd +++ b/man/squash-package.Rd @@ -23,7 +23,6 @@ Useful links: Authors: \itemize{ - \item Antoine Languillaume \email{antoine@thinkr.fr} \item Colin Fay \email{colin@thinkr.fr} \item Swann Floc'hlay \email{swann@thinkr.fr} } diff --git a/tests/testthat/test-compile_qmd_course.R b/tests/testthat/test-compile_qmd_course.R index d744540..41bd816 100644 --- a/tests/testthat/test-compile_qmd_course.R +++ b/tests/testthat/test-compile_qmd_course.R @@ -375,6 +375,25 @@ skip_if_no_chrome <- function() { if (inherits(has_chrome, "try-error")) { skip("chrome not installed") } + # finding the binary is not enough: probe that a headless session + # can actually be established (fails on some CI runners) + probe_html <- tempfile(fileext = ".html") + writeLines("probe", probe_html) + probe_pdf <- try( + { + pagedown::chrome_print( + input = probe_html, + output = tempfile(fileext = ".pdf"), + timeout = 30 + ) + }, + silent = TRUE + ) + unlink(probe_html) + if (inherits(probe_pdf, "try-error")) { + skip("chrome is installed but headless printing is not functional here") + } + unlink(probe_pdf) } run_with_multiple_quarto <- function(expr) { diff --git a/tests/testthat/test-isolate_qmd_render.R b/tests/testthat/test-isolate_qmd_render.R index e683daf..1716228 100644 --- a/tests/testthat/test-isolate_qmd_render.R +++ b/tests/testthat/test-isolate_qmd_render.R @@ -1,5 +1,16 @@ -test_that("build_isolated_project creates a private project and keeps the repo reachable", { - # fake repo: root/(data.csv)/courses/(_quarto.yml, _extensions, M01/S01/file.qmd) +symlinks_supported <- function() { + probe_dir <- tempfile(pattern = "symlink_probe") + dir.create(probe_dir) + on.exit(unlink(probe_dir, recursive = TRUE), add = TRUE) + target <- file.path(probe_dir, "target") + file.create(target) + ok <- suppressWarnings( + file.symlink(from = target, to = file.path(probe_dir, "link")) + ) + isTRUE(ok) && nzchar(Sys.readlink(file.path(probe_dir, "link"))) +} + +make_fake_repo <- function() { root <- tempfile(pattern = "iso_repo") chapter <- file.path(root, "courses", "M01", "S01") sibling <- file.path(root, "courses", "M01", "S02") @@ -8,6 +19,7 @@ test_that("build_isolated_project creates a private project and keeps the repo r dir.create(chapter, recursive = TRUE) dir.create(sibling, recursive = TRUE) dir.create(ext, recursive = TRUE) + file.create(file.path(root, ".here")) writeLines("format: revealjs", file.path(root, "courses", "_quarto.yml")) writeLines("title: theme", file.path(ext, "_extension.yml")) @@ -17,7 +29,16 @@ test_that("build_isolated_project creates a private project and keeps the repo r writeLines("img", file.path(chapter, "logo.png")) writeLines("# other", file.path(sibling, "other.qmd")) - isolation <- build_isolated_project(qmd) + list(root = root, chapter = chapter, qmd = qmd) +} + +test_that("build_isolated_project creates a private project and keeps the repo reachable", { + skip_if_not(symlinks_supported(), "symlinks are not supported here") + + repo <- make_fake_repo() + on.exit(unlink(repo$root, recursive = TRUE), add = TRUE) + + isolation <- build_isolated_project(repo$qmd) on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) iso_courses <- file.path(isolation$iso_root, "courses") @@ -46,27 +67,147 @@ test_that("build_isolated_project creates a private project and keeps the repo r expect_false(dir.exists(file.path(iso_courses, ".quarto"))) }) -test_that("collect_isolated_outputs copies rendering outputs back to the real chapter", { +test_that("collect_isolated_outputs copies new and updated outputs back", { + skip_if_not(symlinks_supported(), "symlinks are not supported here") + + repo <- make_fake_repo() + on.exit(unlink(repo$root, recursive = TRUE), add = TRUE) + + # a stale output from a previous render already sits in the real chapter + writeLines("OLD", file.path(repo$chapter, "doc.html")) + + isolation <- build_isolated_project(repo$qmd) + on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) + + # simulate a render: the html is REGENERATED and media appear + Sys.sleep(1.1) + writeLines("NEW", file.path(isolation$iso_chapter, "doc.html")) + media <- file.path(isolation$iso_chapter, "out_img", "S01_img") + dir.create(media, recursive = TRUE) + writeLines("img", file.path(media, "fig.png")) + + collect_isolated_outputs(isolation) + + # updated pre-existing output is refreshed, new files are brought back + expect_identical( + readLines(file.path(repo$chapter, "doc.html")), + "NEW" + ) + expect_true(file.exists(file.path(repo$chapter, "out_img", "S01_img", "fig.png"))) + # untouched inputs are not altered + expect_identical(readLines(file.path(repo$chapter, "doc.qmd")), "# doc") +}) + +test_that("a qmd directly in the project dir gets a real private copy", { + skip_if_not(symlinks_supported(), "symlinks are not supported here") + root <- tempfile(pattern = "iso_repo") - chapter <- file.path(root, "courses", "M01", "S01") + proj <- file.path(root, "courses") + dir.create(proj, recursive = TRUE) + file.create(file.path(root, ".here")) + writeLines("format: revealjs", file.path(proj, "_quarto.yml")) + qmd <- file.path(proj, "doc.qmd") + writeLines("# doc", qmd) + on.exit(unlink(root, recursive = TRUE), add = TRUE) + + isolation <- build_isolated_project(qmd) + on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) + + expect_true(file.exists(isolation$iso_qmd)) + expect_false(nzchar(Sys.readlink(isolation$iso_qmd))) +}) + +test_that("nothing above the project is exposed without a repository marker", { + skip_if_not(symlinks_supported(), "symlinks are not supported here") + + base <- tempfile(pattern = "iso_norepo") + proj <- file.path(base, "courses") + chapter <- file.path(proj, "M01") dir.create(chapter, recursive = TRUE) - writeLines("format: revealjs", file.path(root, "courses", "_quarto.yml")) + # a sibling of the project that must NOT leak into the isolated tree + dir.create(file.path(base, "unrelated")) + writeLines("format: revealjs", file.path(proj, "_quarto.yml")) qmd <- file.path(chapter, "doc.qmd") writeLines("# doc", qmd) + on.exit(unlink(base, recursive = TRUE), add = TRUE) isolation <- build_isolated_project(qmd) on.exit(unlink(isolation$iso_root, recursive = TRUE), add = TRUE) - # simulate a render: html + media dir appear in the isolated chapter - writeLines("", file.path(isolation$iso_chapter, "doc.html")) - media <- file.path(isolation$iso_chapter, "out_img", "S01_img") - dir.create(media, recursive = TRUE) - writeLines("img", file.path(media, "fig.png")) + expect_false(file.exists(file.path(isolation$iso_root, "unrelated"))) + expect_true(file.exists(isolation$iso_qmd)) +}) - collect_isolated_outputs(isolation) +test_that("projects with a project-level output-dir are refused", { + root <- tempfile(pattern = "iso_site") + proj <- file.path(root, "site") + dir.create(proj, recursive = TRUE) + writeLines( + c("project:", " type: website", " output-dir: _site"), + file.path(proj, "_quarto.yml") + ) + qmd <- file.path(proj, "index.qmd") + writeLines("# home", qmd) + on.exit(unlink(root, recursive = TRUE), add = TRUE) + + expect_error( + build_isolated_project(qmd), + class = "squash_no_isolation" + ) +}) + +test_that("an isolated render produces up-to-date output in the real chapter", { + skip_if_not(symlinks_supported(), "symlinks are not supported here") + skip_if_not(quarto::quarto_available(), "quarto is not available") - expect_true(file.exists(file.path(chapter, "doc.html"))) - expect_true(file.exists(file.path(chapter, "out_img", "S01_img", "fig.png"))) - # pre-existing files are not duplicated or altered - expect_identical(readLines(file.path(chapter, "doc.qmd")), "# doc") + # dedicated fixture: a real render requires a well-formed project + # (the fake _extensions of make_fake_repo() would make quarto error) + root <- tempfile(pattern = "iso_render") + chapter <- file.path(root, "courses", "M01", "S01") + dir.create(chapter, recursive = TRUE) + file.create(file.path(root, ".here")) + writeLines("format: revealjs", file.path(root, "courses", "_quarto.yml")) + repo <- list( + root = root, + chapter = chapter, + qmd = file.path(chapter, "doc.qmd") + ) + on.exit(unlink(repo$root, recursive = TRUE), add = TRUE) + writeLines( + c("---", "title: v1", "---", "", "# FIRST-VERSION"), + repo$qmd + ) + + expect_true( + render_single_qmd( + qmd = repo$qmd, + img_root_dir = "iso_img", + output_format = "revealjs" + ) + ) + html <- file.path(repo$chapter, "doc.html") + expect_true(file.exists(html)) + expect_true(any(grepl("FIRST-VERSION", readLines(html, warn = FALSE)))) + + # re-render with modified content: the real chapter must be refreshed + writeLines( + c("---", "title: v2", "---", "", "# SECOND-VERSION"), + repo$qmd + ) + expect_true( + render_single_qmd( + qmd = repo$qmd, + img_root_dir = "iso_img", + output_format = "revealjs" + ) + ) + expect_true(any(grepl("SECOND-VERSION", readLines(html, warn = FALSE)))) + + # no isolated project tree is left behind + leftovers <- list.files( + tempdir(), + pattern = "^squash_iso_", + full.names = TRUE + ) + expect_length(leftovers, 0) })