Revise quarto argument (#828)

Replace path specification (needed for historical reasons) with `TRUE`/`FALSE`/`NA`. Fixes #658
rstudio · May 2, 2023 · d974d04 · d974d04
1 parent 0918f7f
commit d974d04
Show file tree

Hide file tree

Showing 14 changed files with 320 additions and 302 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,10 @@
 # rsconnect (development version)
 
+* `deployApp()`'s `quarto` argument now takes values `TRUE`, `FALSE` or 
+  `NA`. The previous value (a path to a quarto binary) is now deprecated,
+  and instead we automatically figure out the packge from `QUARTO_PATH` and
+  `PATH` env vars (#658).
+
 * `deployApp()` gains a new `envVars` argument which takes a vector of the 
   names of environment variables that should be securely copied to the server. 
   The names (not values) of these environment variables are also saved in the

diff --git a/R/appMetadata-quarto.R b/R/appMetadata-quarto.R
@@ -0,0 +1,69 @@
+inferQuartoInfo <- function(metadata, appDir, appPrimaryDoc) {
+  if (hasQuartoMetadata(metadata)) {
+    return(list(
+      version = metadata[["quarto_version"]],
+      engines = metadata[["quarto_engines"]]
+    ))
+  }
+
+  # If we don't yet have Quarto details, run quarto inspect ourselves
+  inspect <- quartoInspect(
+    appDir = appDir,
+    appPrimaryDoc = appPrimaryDoc
+  )
+  if (is.null(inspect)) {
+    return(NULL)
+  }
+
+  list(
+    version = inspect[["quarto"]][["version"]],
+    engines = I(inspect[["engines"]])
+  )
+}
+
+hasQuartoMetadata <- function(x) {
+  !is.null(x$quarto_version)
+}
+
+# Run "quarto inspect" on the target and returns its output as a parsed object.
+quartoInspect <- function(appDir = NULL, appPrimaryDoc = NULL) {
+  # If "quarto inspect appDir" fails, we will try "quarto inspect
+  # appPrimaryDoc", so that we can support single files as well as projects.
+  quarto <- quarto_path()
+  if (is.null(quarto)) {
+    cli::cli_abort(c(
+      "`quarto` not found.",
+      i = "Check that it is installed and available on your {.envvar PATH}."
+    ))
+  }
+
+  paths <- c(appDir, file.path(appDir, appPrimaryDoc))
+
+  for (path in paths) {
+    args <- c("inspect", path.expand(path))
+    inspect <- tryCatch(
+      {
+        json <- suppressWarnings(system2(quarto, args, stdout = TRUE, stderr = TRUE))
+        parsed <- jsonlite::fromJSON(json)
+        return(parsed)
+      },
+      error = function(e) NULL
+    )
+  }
+  return(NULL)
+}
+
+# inlined from quarto::quarto_path()
+quarto_path <- function() {
+  path_env <- Sys.getenv("QUARTO_PATH", unset = NA)
+  if (is.na(path_env)) {
+    path <- unname(Sys.which("quarto"))
+    if (nzchar(path)) {
+      path
+    } else {
+      NULL
+    }
+  } else {
+    path_env
+  }
+}
diff --git a/R/appMetadata.R b/R/appMetadata.R
@@ -1,18 +1,31 @@
 appMetadata <- function(appDir,
                         appFiles = NULL,
                         appPrimaryDoc = NULL,
-                        quarto = NULL,
+                        quarto = NA,
                         contentCategory = NULL,
                         isCloudServer = FALSE,
                         metadata = list()) {
 
   appFiles <- listDeploymentFiles(appDir, appFiles)
   checkAppLayout(appDir, appPrimaryDoc)
 
-  # User has supplied quarto path or quarto package/IDE has supplied metadata
+  if (is_string(quarto)) {
+    lifecycle::deprecate_warn(
+      when = "0.9.0",
+      what = "deployApp(quarto = 'can no longer be a path')",
+      with = I("quarto = `TRUE` instead")
+    )
+    quarto <- TRUE
+  } else {
+    check_bool(quarto, allow_na = TRUE)
+  }
+
+  # If quarto package/IDE has supplied metadata, always use quarto
   # https://github.com/quarto-dev/quarto-r/blob/08caf0f42504e7/R/publish.R#L117-L121
   # https://github.com/rstudio/rstudio/blob/3d45a20307f650/src/cpp/session/modules/SessionRSConnect.cpp#L81-L123
-  hasQuarto <- !is.null(quarto) || hasQuartoMetadata(metadata)
+  if (hasQuartoMetadata(metadata)) {
+    quarto <- TRUE
+  }
 
   # Generally we want to infer appPrimaryDoc from appMode, but there's one
   # special case
@@ -24,7 +37,7 @@ appMetadata <- function(appDir,
     rootFiles <- appFiles[dirname(appFiles) == "."]
     appMode <- inferAppMode(
       file.path(appDir, appFiles),
-      hasQuarto = hasQuarto,
+      usesQuarto = quarto,
       isCloudServer = isCloudServer
     )
   }
@@ -44,12 +57,16 @@ appMetadata <- function(appDir,
     appDir = appDir,
     files = appFiles
   )
-  quartoInfo <- inferQuartoInfo(
-    appDir = appDir,
-    appPrimaryDoc = appPrimaryDoc,
-    quarto = quarto,
-    metadata = metadata
-  )
+
+  if (appIsQuartoDocument(appMode)) {
+    quartoInfo <- inferQuartoInfo(
+      metadata = metadata,
+      appDir = appDir,
+      appPrimaryDoc = appPrimaryDoc
+    )
+  } else {
+    quartoInfo <- NULL
+  }
 
   list(
     appMode = appMode,
@@ -99,10 +116,9 @@ checkAppLayout <- function(appDir, appPrimaryDoc = NULL) {
   ))
 }
 
-
 # infer the mode of the application from files in the root dir
 inferAppMode <- function(absoluteAppFiles,
-                         hasQuarto = FALSE,
+                         usesQuarto = NA,
                          isCloudServer = FALSE) {
 
   matchingNames <- function(paths, pattern) {
@@ -125,22 +141,13 @@ inferAppMode <- function(absoluteAppFiles,
   rmdFiles <- matchingNames(absoluteAppFiles, "\\.rmd$")
   qmdFiles <- matchingNames(absoluteAppFiles, "\\.qmd$")
 
-  # We make Quarto requirement conditional on the presence of files that Quarto
-  # can render and _quarto.yml, because keying off the presence of qmds
-  # *or* _quarto.yml was causing deployment failures in static content.
-  # https://github.com/rstudio/rstudio/issues/11444
-  quartoYml <- matchingNames(absoluteAppFiles, "^_quarto.y(a)?ml$")
-  hasQuartoYaml <- length(quartoYml) > 0
-  hasQuartoCompatibleFiles <- length(qmdFiles) > 0 || length(rmdFiles > 0)
-  requiresQuarto <- (hasQuartoCompatibleFiles && hasQuartoYaml) || length(qmdFiles) > 0
-
-  # We gate the deployment of content that appears to be Quarto behind the
-  # presence of Quarto metadata. Rmd files can still be deployed as Quarto
-  if (requiresQuarto && !hasQuarto) {
-    cli::cli_abort(c(
-      "Can't deploy Quarto content when {.arg quarto} is {.code NULL}.",
-      i = "Please supply a path to a quarto binary in {.arg quarto}."
-    ))
+  if (is.na(usesQuarto)) {
+    # Can't use _quarto.yml alone because it causes deployment failures for
+    # static content: https://github.com/rstudio/rstudio/issues/11444
+    quartoYml <- matchingNames(absoluteAppFiles, "^_quarto.y(a)?ml$")
+
+    usesQuarto <- length(qmdFiles) > 0 ||
+      (length(quartoYml) > 0 && length(rmdFiles > 0))
   }
 
   # Documents with "server: shiny" in their YAML front matter need shiny too
@@ -150,7 +157,7 @@ inferAppMode <- function(absoluteAppFiles,
   if (hasShinyQmd) {
     return("quarto-shiny")
   } else if (hasShinyRmd) {
-    if (hasQuarto) {
+    if (usesQuarto) {
       return("quarto-shiny")
     } else {
       return("rmd-shiny")
@@ -168,7 +175,7 @@ inferAppMode <- function(absoluteAppFiles,
   # Any non-Shiny R Markdown or Quarto documents are rendered content and get
   # rmd-static or quarto-static.
   if (length(rmdFiles) > 0 || length(qmdFiles) > 0) {
-    if (hasQuarto) {
+    if (usesQuarto) {
       return("quarto-static")
     } else {
       # For Shinyapps and posit.cloud, treat "rmd-static" app mode as "rmd-shiny" so that
@@ -275,6 +282,14 @@ appIsDocument <- function(appMode) {
   )
 }
 
+appIsQuartoDocument <- function(appMode) {
+  appMode %in% c(
+    "quarto-static",
+    "quarto-shiny"
+  )
+}
+
+
 appHasParameters <- function(appDir, appPrimaryDoc, appMode, contentCategory = NULL) {
   # Only Rmd deployments are marked as having parameters. Shiny applications
   # may distribute an Rmd alongside app.R, but that does not cause the
@@ -321,55 +336,3 @@ documentHasPythonChunk <- function(filename) {
   matches <- grep("`{python", lines, fixed = TRUE)
   return(length(matches) > 0)
 }
-
-inferQuartoInfo <- function(appDir, appPrimaryDoc, quarto, metadata) {
-  if (hasQuartoMetadata(metadata)) {
-    return(list(
-      version = metadata[["quarto_version"]],
-      engines = metadata[["quarto_engines"]]
-    ))
-  }
-
-  if (is.null(quarto)) {
-    return(NULL)
-  }
-
-  # If we don't yet have Quarto details, run quarto inspect ourselves
-  inspect <- quartoInspect(
-    quarto = quarto,
-    appDir = appDir,
-    appPrimaryDoc = appPrimaryDoc
-  )
-  if (is.null(inspect)) {
-    return(NULL)
-  }
-
-  list(
-    version = inspect[["quarto"]][["version"]],
-    engines = I(inspect[["engines"]])
-  )
-}
-
-hasQuartoMetadata <- function(x) {
-  !is.null(x$quarto_version)
-}
-
-# Run "quarto inspect" on the target and returns its output as a parsed object.
-quartoInspect <- function(quarto, appDir = NULL, appPrimaryDoc = NULL) {
-  # If "quarto inspect appDir" fails, we will try "quarto inspect
-  # appPrimaryDoc", so that we can support single files as well as projects.
-  paths <- c(appDir, file.path(appDir, appPrimaryDoc))
-
-  for (path in paths) {
-    args <- c("inspect", path.expand(path))
-    inspect <- tryCatch(
-      {
-        json <- suppressWarnings(system2(quarto, args, stdout = TRUE, stderr = TRUE))
-        parsed <- jsonlite::fromJSON(json)
-        return(parsed)
-      },
-      error = function(e) NULL
-    )
-  }
-  return(NULL)
-}
diff --git a/R/deployApp.R b/R/deployApp.R
@@ -122,9 +122,13 @@
 #' @param forceGeneratePythonEnvironment Optional. If an existing
 #'   `requirements.txt` file is found, it will be overwritten when this argument
 #'   is `TRUE`.
-#' @param quarto Optional. Full path to a Quarto binary for use deploying Quarto
-#'   content. The provided Quarto binary will be used to run `quarto inspect`
-#'   to gather information about the content.
+#' @param quarto Should the deployed content be built by quarto?
+#'   (`TRUE`, `FALSE`, or `NA`). The default, `NA`, will use quarto if
+#'   there are `.qmd` files in the bundle, or if there is a
+#'   `_quarto.yml` and `.Rmd` files.
+#'
+#'   (This option is ignored and quarto will always be used if the
+#'   `metadata` contains `quarto_version` and `quarto_engines` fields.)
 #' @param appVisibility One of `NULL`, `"private"`, or `"public"`; the
 #'   visibility of the deployment. When `NULL`, no change to visibility is
 #'   made. Currently has an effect only on deployments to shinyapps.io.
@@ -155,7 +159,7 @@
 #'
 #' # deploy a Quarto website, using the quarto package to
 #' # find the Quarto binary
-#' deployApp("~/projects/quarto/site1", quarto = quarto::quarto_path())
+#' deployApp("~/projects/quarto/site1")
 #' }
 #' @seealso [applications()], [terminateApp()], and [restartApp()]
 #' @family Deployment functions
@@ -183,7 +187,7 @@ deployApp <- function(appDir = getwd(),
                       forceUpdate = NULL,
                       python = NULL,
                       forceGeneratePythonEnvironment = FALSE,
-                      quarto = NULL,
+                      quarto = NA,
                       appVisibility = NULL,
                       image = NULL
                       ) {

diff --git a/R/writeManifest.R b/R/writeManifest.R
@@ -22,7 +22,7 @@ writeManifest <- function(appDir = getwd(),
                           contentCategory = NULL,
                           python = NULL,
                           forceGeneratePythonEnvironment = FALSE,
-                          quarto = NULL,
+                          quarto = NA,
                           image = NULL,
                           verbose = FALSE) {
   appFiles <- listDeploymentFiles(

diff --git a/man/deployApp.Rd b/man/deployApp.Rd
diff --git a/man/writeManifest.Rd b/man/writeManifest.Rd
diff --git a/tests/testthat/_snaps/appMetadata-quarto.md b/tests/testthat/_snaps/appMetadata-quarto.md
@@ -0,0 +1,9 @@
+# quartoInspect requires quarto
+
+    Code
+      quartoInspect()
+    Condition
+      Error in `quartoInspect()`:
+      ! `quarto` not found.
+      i Check that it is installed and available on your `PATH`.
+