Skip to content

Commit

Permalink
tolerate parse errors in .ipynb files (closes #2073)
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinushey committed Jan 10, 2025
1 parent cd71641 commit 704e4dd
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 33 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@

# renv 1.1.0 (UNRELEASED)

* Fixed an issue where `renv::snapshot()` could fail if invoked within
a project containing empty or invalid `.ipynb` files. (#2073)

* Fixed an issue where R package installation could fail if the project
depended on a package whose current version available from the configured
package repositories required on a newer version of R than what was currently
Expand Down
62 changes: 34 additions & 28 deletions R/dependencies.R
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ renv_dependencies_impl <- function(

# resolve errors
errors <- match.arg(errors)

# the path to the user .Rprofile is used when discovering dependencies,
# so resolve that eagerly now
renv_scope_binding(
Expand Down Expand Up @@ -502,7 +502,7 @@ renv_dependencies_discover_impl <- function(path) {
signalCondition(warnify(status))
NULL
}

status

}
Expand Down Expand Up @@ -996,7 +996,13 @@ renv_dependencies_discover_chunks_ranges <- function(path, contents, patterns) {

renv_dependencies_discover_ipynb <- function(path) {

json <- renv_json_read(path)
json <- catch(renv_json_read(path))
if (inherits(json, "error")) {
info <- renv_file_info(path)
if (!is.na(info$size) && info$size > 1)
renv_dependencies_error(path, error = json)
}

if (!identical(json$metadata$kernelspec$language, "R"))
return()

Expand Down Expand Up @@ -1051,10 +1057,10 @@ renv_dependencies_discover_r <- function(path = NULL,

if (inherits(expr, "error"))
return(renv_dependencies_error(path, error = expr))

# resolve dev
dev <- dev %||% path == the$paths$r_profile_user

# update current path
state <- renv_dependencies_state()
if (!is.null(state))
Expand Down Expand Up @@ -1107,7 +1113,7 @@ renv_dependencies_discover_r <- function(path = NULL,

renv_dependencies_recurse(expr, callback)
packages <- ls(envir = envir, all.names = TRUE)

# also try to detect knitr::spin() dependencies -- this needs to
# happen outside of the regular dependency discovery machinery
# as it will rely on checking comments in the document
Expand All @@ -1118,7 +1124,7 @@ renv_dependencies_discover_r <- function(path = NULL,
if (length(text) && grepl("^\\s*#'\\s*[-]{3}\\s*$", text[[1L]], perl = TRUE))
packages <- union(c("knitr", "rmarkdown"), packages)
}

renv_dependencies_list(path, packages, dev = dev)
}

Expand Down Expand Up @@ -1235,22 +1241,22 @@ renv_dependencies_discover_r_colon <- function(node, envir) {
}

renv_dependencies_discover_r_citation <- function(node, envir) {

node <- renv_call_expect(node, "utils", "citation")
if (is.null(node))
return(FALSE)

matched <- catch(match.call(utils::citation, node))
if (inherits(matched, "error"))
return(FALSE)

package <- matched[["package"]]
if (!is.character(package) || length(package) != 1L)
return(FALSE)

envir[[package]] <- TRUE
TRUE

}

renv_dependencies_discover_r_pacman <- function(node, envir) {
Expand Down Expand Up @@ -1311,7 +1317,7 @@ renv_dependencies_discover_r_modules <- function(node, envir) {
if (identical(node[[1L]], quote(modules::import))) {
renv_dependencies_discover_r_modules_impl(node, envir)
}

# check for 'import' usages with a module block
node <- renv_call_expect(node, "modules", "module")
if (length(node) >= 2L &&
Expand All @@ -1323,31 +1329,31 @@ renv_dependencies_discover_r_modules <- function(node, envir) {
renv_dependencies_discover_r_modules_impl(node, envir)
})
}

}

renv_dependencies_discover_r_modules_impl <- function(node, envir) {

node <- renv_call_expect(node, "modules", c("import"))
if (is.null(node))
return(FALSE)

# attempt to match the call
prototype <- function(from, ..., attach = TRUE, where = parent.frame()) {}
matched <- catch(match.call(prototype, node, expand.dots = FALSE))
if (inherits(matched, "error"))
return(FALSE)

# extract character vector or symbol from `from`
package <- matched[["from"]]
if (empty(package))
return(FALSE)

# package could be symbols or character so call as.character
# to be safe then mark packages as known
envir[[as.character(package)]] <- TRUE
TRUE

}

renv_dependencies_discover_r_import <- function(node, envir) {
Expand Down Expand Up @@ -1510,7 +1516,7 @@ renv_dependencies_discover_r_testthat <- function(node, envir) {
envir[["xml2"]] <- TRUE
return(TRUE)
}

# check for an R6 class inheriting from a JunitReporter
class <- renv_call_expect(node, "R6", "R6Class")
if (!is.null(class) && identical(class$inherit, quote(JunitReporter))) {
Expand All @@ -1522,7 +1528,7 @@ renv_dependencies_discover_r_testthat <- function(node, envir) {
node <- renv_call_expect(node, "testthat", c("test_package", "test_dir", "test_file"))
if (is.null(node))
return(FALSE)

candidates <- list(
"Junit",
"junit",
Expand All @@ -1545,23 +1551,23 @@ renv_dependencies_discover_r_testthat <- function(node, envir) {
}

renv_dependencies_discover_r_knitr <- function(node, envir) {

matched <- is.call(node) && (
identical(node[[1L]], quote(knitr::opts_chunk$set)) ||
identical(node[[1L]], quote(opts_chunk$set))
)

if (!matched)
return(FALSE)

args <- as.list(node)
if (identical(args[["dev"]], "ragg_png")) {
envir[["ragg"]] <- TRUE
return(TRUE)
}

FALSE

}

renv_dependencies_discover_r_glue_impl <- function(string, node, envir) {
Expand Down Expand Up @@ -1937,10 +1943,10 @@ renv_dependencies_eval <- function(expr) {
}

renv_dependencies_recurse <- function(object, callback) {

if (is.call(object))
callback(object)

if (is.recursive(object))
for (i in seq_along(object))
if (is.call(object[[i]]))
Expand Down
22 changes: 17 additions & 5 deletions tests/testthat/test-snapshot.R
Original file line number Diff line number Diff line change
Expand Up @@ -617,14 +617,26 @@ test_that("standard remotes drop RemoteSha if it's a version", {
})

test_that("we can produce old-style lockfiles if requested", {

skip_on_cran()

renv_scope_options(renv.lockfile.minimal = TRUE)

project <- renv_tests_scope("breakfast")
init()

expect_snapshot(. <- writeLines(readLines("renv.lock")))


})

# # https://github.com/rstudio/renv/issues/2073
test_that("empty .ipynb files are handled gracefully", {

skip_on_cran()
project <- renv_tests_scope("bread")
init()

writeLines("", con = "example.ipynb")
snapshot()

})

0 comments on commit 704e4dd

Please sign in to comment.