Fix #252, Fix #253, devtools::load_all issues

DESCRIPTION

@@ -5,7 +5,7 @@ Description: Simplifies regression tests by comparing objects produced by test
     the tests pass, otherwise execution stops with error details.  If in
     interactive mode, tests can be reviewed through the provided interactive
     environment.
-Version: 1.4.17.9002
+Version: 1.4.17.9003
 Authors@R: c(
     person(
     "Brodie", "Gaslam", email="[email protected]",

NEWS.md

@@ -5,6 +5,12 @@
 * [#288](https://github.com/brodieG/unitizer/issues/288): Prevent upgrade prompt
   in non-interactive mode interrupting result display (regression introduced in
   1.4.15).
+* [#252](https://github.com/brodieG/unitizer/issues/252),
+  [#253](https://github.com/brodieG/unitizer/issues/252): Better documentation
+  of feature incompatibility with `devtools::load_all`, and more graceful
+  recovery from failures caused by the incompatibility.  This only affects
+  `unitizer` sessions run with package search path management enabled (h/t
+  @blset).
 
 ## v1.4.16-17
 

R/global.R

@@ -1,17 +1,17 @@
 # Copyright (C) 2022 Brodie Gaslam
-# 
+#
 # This file is part of "unitizer"
-# 
+#
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation, either version 2 of the License, or
 # (at your option) any later version.
-# 
+#
 # This program is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
-# 
+#
 # Go to <https://www.r-project.org/Licenses/GPL-2> for a copy of the license.
 
 #' @include is.R
@@ -485,17 +485,30 @@ unitizerGlobal <- setRefClass(
       '
       stopifnot(is(to, "unitizerGlobalIndices"))
 
+      success <- TRUE
       if(status@search.path && to@search.path){
-        search_path_update(to@search.path, .self)
+        fail <-
+          inherits(try(search_path_update(to@search.path, .self)), "try-error")
+        success <- success && !fail
       }
-      if(status@namespaces && to@namespaces)
-        namespace_update(to@namespaces, .self)
-      if(status@options && to@options)
-        options_update(tracking@options[[to@options]])
-      if(status@working.directory && to@working.directory)
-        setwd(
-          tracking@working.directory[[to@working.directory]]
+      if(status@namespaces && to@namespaces) {
+        fail <-
+          inherits(try(namespace_update(to@namespaces, .self)), "try-error")
+        success <- success && !fail
+      }
+      if(status@options && to@options) {
+        fail <- inherits(
+          try(options_update(tracking@options[[to@options]])), "try-error"
+        )
+        success <- success && !fail
+      }
+      if(status@working.directory && to@working.directory) {
+        fail <- inherits(
+          try(setwd(tracking@working.directory[[to@working.directory]])),
+          "try-error"
         )
+        success <- success && !fail
+      }
       if(status@random.seed && to@random.seed) {
         if(is.null(tracking@random.seed[[to@random.seed]])) {
           if(exists(".Random.seed", .GlobalEnv, inherits=FALSE))
@@ -504,6 +517,7 @@ unitizerGlobal <- setRefClass(
           assign(
             ".Random.seed", tracking@random.seed[[to@random.seed]], .GlobalEnv
       ) } }
+      if(!success) stop("Global state reset failed, see prior error.")
       indices.last <<- to
       indices.last
     },

R/search.R

@@ -599,8 +599,9 @@ reattach <- function(pos, name, type, data, extra=NULL, global) {
     if(inherits(lib.try, "try-error")) {
       # nocov start
       warning(
-        "Internal Warning: unable to fully restore search path; see prior ",
-        "error. Contact maintainer if this warning persists.",
+        "Warning: unable to fully restore search path; see prior ",
+        "error, and consult `?unitizerState`, searching for \"Caveats\". ",
+        "Contact maintainer if your problem is not covered by documentation.",
         immediate.=TRUE
       )
       global$state()

R/state.R

@@ -35,7 +35,7 @@ NULL
 #' \code{.Rprofile}, although if you intend to do this be sure to read the
 #' \dQuote{CRAN non-compliance} section.
 #'
-#' @section CRAN non-compliance:
+#' @section CRAN Non-Compliance and Other Caveats:
 #'
 #' In the default state management mode, this package fully complies with CRAN
 #' policies.  In order to implement advanced state management features we must
@@ -47,8 +47,21 @@ NULL
 #' Unfortunately this tracing is against CRAN policies, which is why it is
 #' disabled by default.
 #'
+#' Arguably other aspects of state management employed outside of
+#' \code{state="default"} _could_ be considered CRAN non-compliant, but none of
+#' these are deployed unless you explicitly chose to do so.  Additionally,
+#' \code{unitizer} limits state manipulation to the evaluation of its processes
+#' and restores state on exit.  Some exceptional failures may prevent restoring
+#' state fully.
+#'
+#' If state management were to fail fail in an unhandled form, the simplest
+#' work-around is to turn off state management altogether with
+#' \code{state="default"}. If it is a particular aspect of state management
+#' (e.g. search paths with packages attached with \code{devtools::load_all}),
+#' you can disable just that aspect of state (see "Custom Control" section).
+#'
 #' For more details see the reproducible tests vignette with:
-#' \code{vignette(package='unitizer', 'unitizer_reproducible_tests')}
+#' \code{vignette(package='unitizer', 'u4_reproducible-tests')}
 #'
 #' @section Overview:
 #'
@@ -115,7 +128,9 @@ NULL
 #' and re-attaches packages.  This is not always the same as loading a package
 #' into a fresh R session as detaching a package does not necessarily undo every
 #' action that a package takes when it is loaded.  See \code{\link{detach}} for
-#' potential pitfalls of enabling this setting.
+#' potential pitfalls of enabling this setting.  Additionally, packages attached
+#' in non-standard ways (e.g. \code{devtools::load_all}) may not re-attach
+#' properly.
 #'
 #' You can modify what aspects of state are managed by using the \code{state}
 #' parameter to \code{\link{unitize}}.  If you are satisfied with basic default

R/unitize.core.R

@@ -1089,14 +1089,14 @@ reset_and_unshim <- function(global) {
     meta_word_msg(
       "Failed unshimming library/detach/attach; you may want to restart",
       "your R session to reset them to their original values (or you",
-      "can `untrace` them manually)"
+      "can `untrace` them manually)."
     )
   if(!success.release)
     meta_word_msg(
       "Failed releasing global tracking object; you will not be able to",
       "instantiate another `unitizer` session.  This should not happen, ",
       "please contact the maintainer.  In the meantime, restarting your R",
-      "session should restore functionality"
+      "session should restore functionality."
     )
   success.clear && success.unshim
 }

tests/t-search.R

@@ -242,3 +242,24 @@ glb$release()
 unitizer:::unitizerUniqueNames(list(search.path = c(goodbye = "0",
     hello = "1", goodbye = "2", goodbye = "3")))
 
+# - "Fake Package Re-attach" ---------------------------------------------------
+
+# Make sure that aspects of search path management other than search path
+# survive a failure caused by bad search path env (#252, #253).
+
+owd <- getwd()
+test.f <- paste0(tempfile(), ".R")
+writeLines("
+f <- tempfile()
+dir.create(f)
+setwd(f)
+# Package assumed non-existing; R could disallow this in the future
+# which could change the test.
+attach(list(x=42), name='package:adfaadcxuqyojfnkfadsf')
+1 + 1", test.f)
+out <- unitizer:::capture_output(
+  try(unitize(test.f, state='recommended', interactive.mode=FALSE))
+)
+any(grepl("mismatch between actual search path and tracked", out$message))
+identical(owd, getwd())  # confirm working directory restored
+

tests/t-search.Rout.save

@@ -1,12 +1,14 @@
 
-R Under development (unstable) (2021-07-17 r80639) -- "Unsuffered Consequences"
-Copyright (C) 2021 The R Foundation for Statistical Computing
+R Under development (unstable) (2022-02-01 r81609) -- "Unsuffered Consequences"
+Copyright (C) 2022 The R Foundation for Statistical Computing
 Platform: x86_64-apple-darwin17.0 (64-bit)
 
 R is free software and comes with ABSOLUTELY NO WARRANTY.
 You are welcome to redistribute it under certain conditions.
 Type 'license()' or 'licence()' for distribution details.
 
+  Natural language support but running in an English locale
+
 R is a collaborative project with many contributors.
 Type 'contributors()' for more information and
 'citation()' on how to cite R or R packages in publications.
@@ -479,7 +481,30 @@ NULL
 +     hello = "1", goodbye = "2", goodbye = "3")))
 [1] "goodbye"   "hello"     "goodbye.1" "goodbye.2"
 > 
+> # - "Fake Package Re-attach" ---------------------------------------------------
+> 
+> # Make sure that aspects of search path management other than search path
+> # survive a failure caused by bad search path env (#252, #253).
+> 
+> owd <- getwd()
+> test.f <- paste0(tempfile(), ".R")
+> writeLines("
++ f <- tempfile()
++ dir.create(f)
++ setwd(f)
++ # Package assumed non-existing; R could disallow this in the future
++ # which could change the test.
++ attach(list(x=42), name='package:adfaadcxuqyojfnkfadsf')
++ 1 + 1", test.f)
+> out <- unitizer:::capture_output(
++   try(unitize(test.f, state='recommended', interactive.mode=FALSE))
++ )
+> any(grepl("mismatch between actual search path and tracked", out$message))
+[1] TRUE
+> identical(owd, getwd())  # confirm working directory restored
+[1] TRUE
+> 
 > 
 > proc.time()
    user  system elapsed 
-   5.52    1.67    9.49 
+  3.534   0.721   4.428 

vignettes/u1_intro.Rmd

@@ -166,9 +166,10 @@ tracked and managed:
 * Loaded namespaces
 
 State management is turned off by default because it requires tracing some base
-functions which is against CRAN policy.  If you wish to enable this feature use
-`unitize(..., state='suggested')` or `options(unitizer.state='suggested')`.
-For more details see `?unitizerState` and the [reproducible tests
+functions which is against CRAN policy, and generally affects session state in
+uncommon ways.  If you wish to enable this feature use `unitize(...,
+state='suggested')` or `options(unitizer.state='suggested')`.  For more details
+including potential pitfalls see `?unitizerState` and the [reproducible tests
 vignette](u4_reproducible-tests.html).
 
 ### Beware of Force Quitting from `unitizer`

vignettes/u4_reproducible-tests.Rmd

@@ -274,33 +274,32 @@ For example, S3 method registration is not undone when detaching a package, or
 even unloading its namespace.  See discussion in `?detach` and in
 `?unitizerState`.
 
-Another good example of problematic behavior are attaching environments that
-contain references to themselves, as the `tools:rstudio` environment attached 
-by `Rstudio` does.  It contains functions that have for environment the
-`tools:rstudio` environment.  The problem is that once that environment is
-detached from the search path, those functions no longer have access to the
-search path.  Re-attaching the environment to the search path does not solve the
-problem because `attach` attaches a _copy_ of the environment, not the
-environment itself.  This new environment will contain the same objects as the
-original environment, but all the functions therein will have for environment
-the original detached environment, not the copy that is attached to
-the search path.
-
-There are a few possible solutions to this problem.  For now we have adopted the
-simplest which is to keep the `tools:rstudio` environment on the search path
-even search path tracking is enabled (you can over-ride this by changing
-`search.path.keep`, or, if you have environments on your search path with
-similar properties, add their names to `search.path.keep`).  Other options
+One known problem is the use of `devtools::load_all` and similar which place a
+pretend package environment on the search path.  Such packages cannot be
+re-loaded with `library` so the re-attach process will fail (see
+[#252](https://github.com/brodieG/unitizer/issues/252)).
+
+Another issue is attached environments that contain references to themselves, as
+the `tools:rstudio` environment attached by `Rstudio` does.  It contains
+functions that have for environment the `tools:rstudio` environment.  The
+problem is that once that environment is detached from the search path, those
+functions no longer have access to the search path.  Re-attaching the
+environment to the search path does not solve the problem because `attach`
+attaches a _copy_ of the environment, not the environment itself.  This new
+environment will contain the same objects as the original environment, but all
+the functions therein will have for environment the original detached
+environment, not the copy that is attached to the search path.
+
+For the specific `tools::rstudio` problem we work around the issue by keeping it
+on the search path even search path tracking is enabled (you can over-ride this
+by changing `search.path.keep`, or, if you have environments on your search path
+with similar properties, add their names to `search.path.keep`).  Other options
 include re-attaching with `parent.env<-` instead of `attach`, but messing with
 the search path in that way seems to be exactly what R core warns about in
 `?parent.env`:
 
 > The replacement function parent.env<- is extremely dangerous as it can be used to destructively change environments in ways that violate assumptions made by the internal C code. It may be removed in the near future.
 
-Another possibility would be to re-set the environments of functions inside
-detached environments that have for parent the detached environment, but we do
-not do this currently.
-
 ## Global Options
 
 `unitizer` can track and reset global options.  Because many packages set