Merge pull request #150 from ropensci/fix_render_root

Normalize path for render_root in yml to fix relative paths

R/processData.R

@@ -1,18 +1,15 @@
 .validate_render_root <- function(x) {
-  # catch an error if it doesn't exist
-  render_root <-
-    try(normalizePath(x,
-      mustWork = TRUE,
-      winslash = "/"
-    ), silent = TRUE)
-  if (inherits(render_root, "try-error")) {
-    .multilog_error(paste0("render_root  = ", x, " doesn't exist."))
-    # try creating, even if it's an old temp dir.
-    # This isn't ideal. Would like to rather say it's a temporary
-    # directory and use the current one..
-    return(FALSE)
+  # catch an error if it doesn't exist, otherwise return normalized path
+  # important for handling relative paths in a rmarkdown::render() context
+  if (! dir.exists(x)){
+    .multilog_error(paste0("render_root = ", x, " doesn't exist"))
+    stop(paste0("render_root = ", x, " doesn't exist"))
   }
-  return(TRUE)
+  normalizePath(x, winslash = "/")
+  # old comments below have been retained:
+  # try creating, even if it's an old temp dir.
+  # This isn't ideal. Would like to rather say it's a temporary
+  # directory and use the current one..
 }
 
 
@@ -62,7 +59,7 @@ DataPackageR <- function(arg = NULL, deps = TRUE) {
   # get vector of R and Rmd files from validated YAML
   r_files <- file.path(pkg_dir, 'data-raw', get_yml_r_files(ymlconf))
   objects_to_keep <- get_yml_objects(ymlconf)
-  render_root <- .get_render_root(ymlconf)
+  render_root <- .validate_render_root(.get_render_root(ymlconf))
 
   # The test for a valid DESCRIPTION here is no longer needed since
   # we use proj_set().
@@ -263,13 +260,7 @@ validate_yml <- function(pkg_dir){
     stop(err_msg, call. = FALSE)
   }
   render_root <- .get_render_root(ymlconf)
-  if (!.validate_render_root(render_root)) {
-    .multilog_fatal(paste0(
-      "Can't create, or render_root = ",
-      render_root, " doesn't exist"
-    ))
-    stop("error", call. = FALSE)
-  }
+  .validate_render_root(render_root)
   if (length(get_yml_objects(ymlconf)) == 0) {
     .multilog_fatal("You must specify at least one data object.")
     stop("exiting", call. = FALSE)

tests/testthat/test-build-locations.R

@@ -42,3 +42,32 @@ test_that("Error on data pkg dirname different from data pkg name", {
                    "name of the data package directory")
   expect_error(package_build(file.path(td, not_sn)), err_msg)
 })
+
+test_that("properly handle relative render_root path from yaml config", {
+  # A lightly modified version of Jason's reprex
+  withr::with_tempdir({
+    datapackage_skeleton("new")
+
+    utils::write.csv(data.frame(x=1:10),
+              file.path('new', 'inst', 'extdata', 'ext.csv'),
+              row.names=F)
+
+    x <- "x <- read.csv(file.path('inst', 'extdata', 'ext.csv'))"
+    writeLines(x, file.path('new', 'data-raw', 'x.R'))
+
+    config <- yml_add_files("new", "x.R")
+    config <- yml_add_objects(config, "x")
+    config <- yml_write(config, "new")
+
+    yml <- yaml::read_yaml(file.path("new", "datapackager.yml"))
+    yml$configuration$render_root$tmp <- NULL
+    yml$configuration$render_root <- "./"
+    yaml::write_yaml(yml, file.path("new", "datapackager.yml"))
+
+    expect_error(package_build())
+
+    withr::with_dir('new', {
+      expect_no_error(package_build())
+    })
+  })
+})

tests/testthat/test-edge-cases.R

@@ -298,7 +298,7 @@ test_that("local edge case block 8", {
          recursive = TRUE
   )
   suppressWarnings(expect_error(yml_list_objects(td_foo)))
-  expect_false(DataPackageR:::.validate_render_root(file.path(td, 'foobar')))
+  expect_error(DataPackageR:::.validate_render_root(file.path(td, 'foobar')))
   suppressWarnings(expect_error(
     DataPackageR:::yml_add_files("subsetCars", "foo.rmd")
   ))