From 96b5ad99d7b659b38bada7e1e60972c26fb0a49c Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 30 Mar 2024 22:39:36 -0700 Subject: [PATCH 1/6] bugfix: undo phantom pkg loading done by roxygen2::roxygenise() --- R/build.R | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/R/build.R b/R/build.R index aee67ce..b86844e 100644 --- a/R/build.R +++ b/R/build.R @@ -94,10 +94,14 @@ package_build <- function(packageName = NULL, .multilog_warn("DataPackageR failed") ) .multilog_trace("Building documentation") - roxygen2::roxygenise(package_path, - clean = TRUE - ) - + local({ + on.exit({ + if (packageName %in% devtools::package_info('attached')$package){ + devtools::unload(packageName) + } + }) + roxygen2::roxygenise(package_path, clean = TRUE) + }) .multilog_trace("Building package") location <- build(package_path, path = dirname(package_path), @@ -105,7 +109,6 @@ package_build <- function(packageName = NULL, ) # try to install and then reload the package in the current session if (install) { - devtools::unload(packageName) install.packages(location, repos = NULL, type = "source", ...) } .next_steps() From 5ea3695542bf4c638eb17430f48a18a7268638a7 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 30 Mar 2024 23:16:49 -0700 Subject: [PATCH 2/6] bugfix: undo phantom pkg loading done by devtools::document() --- R/processData.R | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/R/processData.R b/R/processData.R index d19b90e..4186ce5 100644 --- a/R/processData.R +++ b/R/processData.R @@ -842,14 +842,20 @@ document <- function(path = ".", install = TRUE, ...) { overwrite = TRUE ) .multilog_trace("Rebuilding data package documentation.") - devtools::document(pkg = path) + local({ + on.exit({ + if (basename(path) %in% devtools::package_info('attached')$package){ + devtools::unload(basename(path)) + } + }) + devtools::document(pkg = path) + }) location <- devtools::build( pkg = path, path = dirname(path), vignettes = FALSE, quiet = TRUE ) # try to install and then reload the package in the current session if (install) { - devtools::unload(basename(path)) install.packages(location, repos = NULL, type = "source", ...) } return(TRUE) From 312e48bff411ac28930b297af65a808a9e1fdc23 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sun, 31 Mar 2024 19:01:01 -0700 Subject: [PATCH 3/6] test that phantom pkg loading bug is fixed --- tests/testthat/test-phantom_loading.R | 39 +++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/testthat/test-phantom_loading.R diff --git a/tests/testthat/test-phantom_loading.R b/tests/testthat/test-phantom_loading.R new file mode 100644 index 0000000..1776282 --- /dev/null +++ b/tests/testthat/test-phantom_loading.R @@ -0,0 +1,39 @@ +testthat::test_that( + "no phantom package loading from roxygenise() or associated warnings", + { + # test README.Rmd sequence that led to warnings + processing_code <- system.file( + "extdata", "tests", "subsetCars.Rmd", package = "DataPackageR" + ) + pkg_name <- "mtcars20" + # remove this directory on exit + temp_dir <- withr::local_tempdir() + pkg_path <- file.path(temp_dir, pkg_name) + + datapackage_skeleton( + pkg_name, force = TRUE, + code_files = processing_code, + r_object_names = "cars_over_20", + path = temp_dir) + + expect_no_warning(package_build(pkg_path, install = FALSE)) + # test phantom pkg loading side effect from roxygen2::roxygenise() + expect_false( + res1 <- pkg_name %in% devtools::package_info('attached')$package + ) + + # reset for next test + if (res1) devtools::unload(pkg_name) + + # test phantom pkg loading side effect from roxygen2::roxygenise() + # which is called by devtools::document() + expect_no_warning(document(pkg_path, install = FALSE)) + expect_false( + res2 <- pkg_name %in% devtools::package_info('attached')$package + ) + + # reset and verify attachment state + if (res2) devtools::unload(pkg_name) + expect_false(pkg_name %in% devtools::package_info('attached')$package) + } +) From 6eafacaa6185ffaf728645295952572bd44cdad7 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 30 Mar 2024 22:41:30 -0700 Subject: [PATCH 4/6] Want install = FALSE here b/c we install later --- README.Rmd | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.Rmd b/README.Rmd index 6fcb38c..4f2081b 100644 --- a/README.Rmd +++ b/README.Rmd @@ -191,13 +191,15 @@ datapackage_skeleton( # If the build is run in non-interactive mode, the description will read # "Package built in non-interactive mode". You may update it later. dir.create(file.path(tempdir(),"lib")) -package_build(packageName = file.path(tempdir(),"mtcars20"), install = TRUE, lib = file.path(tempdir(),"lib")) +package_build(packageName = file.path(tempdir(),"mtcars20"), install = FALSE, + lib = file.path(tempdir(),"lib")) # Update the autogenerated roxygen documentation in data-raw/documentation.R. # edit(file.path(tempdir(),"mtcars20","R","mtcars20.R")) # 4. Rebuild the documentation. -document(file.path(tempdir(),"mtcars20"), install = TRUE, lib = file.path(tempdir(),"lib")) +document(file.path(tempdir(),"mtcars20"), install = FALSE, + lib = file.path(tempdir(),"lib")) # Let's use the package we just created. install.packages(file.path(tempdir(),"mtcars20_1.0.tar.gz"), type = "source", repos = NULL) From 589cedb42711fb6e40e977b783bcd605339e88c1 Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Sat, 30 Mar 2024 22:42:34 -0700 Subject: [PATCH 5/6] rebuild README.md wo warnings from fixed phantom loading bugs --- README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 25d46e0..71f4d53 100644 --- a/README.md +++ b/README.md @@ -236,19 +236,18 @@ datapackage_skeleton( # If the build is run in non-interactive mode, the description will read # "Package built in non-interactive mode". You may update it later. dir.create(file.path(tempdir(),"lib")) -package_build(packageName = file.path(tempdir(),"mtcars20"), install = TRUE, lib = file.path(tempdir(),"lib")) -#> Warning: package 'mtcars20' is in use and will not be installed +package_build(packageName = file.path(tempdir(),"mtcars20"), install = FALSE, + lib = file.path(tempdir(),"lib")) # Update the autogenerated roxygen documentation in data-raw/documentation.R. # edit(file.path(tempdir(),"mtcars20","R","mtcars20.R")) # 4. Rebuild the documentation. -document(file.path(tempdir(),"mtcars20"), install = TRUE, lib = file.path(tempdir(),"lib")) -#> Warning: package 'mtcars20' is in use and will not be installed +document(file.path(tempdir(),"mtcars20"), install = FALSE, + lib = file.path(tempdir(),"lib")) # Let's use the package we just created. install.packages(file.path(tempdir(),"mtcars20_1.0.tar.gz"), type = "source", repos = NULL) -#> Warning: package 'mtcars20' is in use and will not be installed library(mtcars20) data("cars_over_20") # load the data cars_over_20 # Now we can use it. From 5da55c9fc3b2e93ee09a212d6b0d73c0efd3529f Mon Sep 17 00:00:00 2001 From: Dave Slager Date: Mon, 1 Apr 2024 10:16:08 -0700 Subject: [PATCH 6/6] better test cleanup --- tests/testthat/test-phantom_loading.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-phantom_loading.R b/tests/testthat/test-phantom_loading.R index 1776282..4b2a17a 100644 --- a/tests/testthat/test-phantom_loading.R +++ b/tests/testthat/test-phantom_loading.R @@ -6,6 +6,11 @@ testthat::test_that( "extdata", "tests", "subsetCars.Rmd", package = "DataPackageR" ) pkg_name <- "mtcars20" + on.exit( + if (pkg_name %in% devtools::package_info('attached')$package){ + devtools::unload(pkg_name) + } + ) # remove this directory on exit temp_dir <- withr::local_tempdir() pkg_path <- file.path(temp_dir, pkg_name) @@ -31,9 +36,5 @@ testthat::test_that( expect_false( res2 <- pkg_name %in% devtools::package_info('attached')$package ) - - # reset and verify attachment state - if (res2) devtools::unload(pkg_name) - expect_false(pkg_name %in% devtools::package_info('attached')$package) } )