Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite recursion errors when running test() twice #1341

Closed
lionel- opened this issue Mar 25, 2021 · 1 comment · Fixed by r-lib/pkgload#153
Closed

Infinite recursion errors when running test() twice #1341

lionel- opened this issue Mar 25, 2021 · 1 comment · Fixed by r-lib/pkgload#153

Comments

@lionel-
Copy link
Member

lionel- commented Mar 25, 2021

First time around everything works fine. Second time I get several errors along the lines of:

Error (test-type-list-of.R:49:3): print method gives human friendly output
Error: evaluation nested too deeply: infinite recursion / options(expressions=)?
Backtrace:
    1. testthat::expect_known_output(...) test-type-list-of.R:49:2
    8. pillar:::print.tbl(tibble::tibble(x))
   11. pillar:::format.tbl(x, width = width, ..., n = n, n_extra = n_extra)
   12. pillar::tbl_format_setup(x, width = width, ..., n = n, max_extra_cols = n_extra)
   14. pillar:::tbl_format_setup.tbl(x, width, ..., n = n, max_extra_cols = max_extra_cols)
   17. utils:::head.data.frame(x, n)
   20. tibble:::`[.tbl_df`(x, 1:2, , drop = FALSE)
   21. tibble:::tbl_subset_row(xo, i = i, i_arg)
   22. base::lapply(unclass(x), vec_slice, i = i)
   23. vctrs:::FUN(X[[i]], ...)
       ...
 2728. vctrs::vec_slice(x, i) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/slice.R:210:2
 2731. vctrs::`[.vctrs_vctr`(x = x, i = i)
 2732. vctrs::vec_index(x, i, ...) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/type-vctr.R:188:2
 2733. vctrs::vec_slice(x, i) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/slice.R:210:2
 2736. vctrs::`[.vctrs_vctr`(x = x, i = i)
 2737. vctrs::vec_index(x, i, ...) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/type-vctr.R:188:2
 2738. vctrs::vec_slice(x, i) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/slice.R:210:2
 2741. vctrs::`[.vctrs_vctr`(x = x, i = i)
 2742. vctrs::vec_index(x, i, ...) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/type-vctr.R:188:2
 2743. rlang::maybe_missing(i, TRUE) /Users/lionel/Sync/Projects/R/r-lib/vctrs/R/slice.R:209:2
@lionel-
Copy link
Member Author

lionel- commented Mar 25, 2021

Occurs after two load_all(). Probably a pkgload issue.

# Fine
pkgload::load_all()
pkgload::load_all()
tibble::tibble(list_of(1))
#>   `list_of(1)`
#>    <list<dbl>>
#> 1          [1]

# Bug
pkgload::load_all()
tibble::tibble()
pkgload::load_all()
tibble::tibble(list_of(1))
#> Error: evaluation nested too deeply: infinite recursion / options(expressions=)?
#> Error during wrapup: evaluation nested too deeply: infinite recursion / options(expressions=)?

My guess is that this is because pkgload now unregisters methods when reloading. This makes sense to do it for other packages than the one being reloaded because the method will be registered again right away. However, maybe we shouldn't do it for the package being reloaded because the stale namespace should remain functional for other namespaces that hold references to it. In this case, tibble holds a reference to a stale vctrs namespace whose generics no longer include vctrs methods. cc @jimhester

lionel- added a commit to r-lib/pkgload that referenced this issue Mar 25, 2021
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 1, 2021
# pkgload 1.2.1

* `unload()` no longer unregisters methods for generics of the package being unloaded. This way dangling references to generics defined in the stale namespace still work as expected (r-lib/vctrs#1341).
* `load_all()` will now work for packages that have testthat tests but do not have testthat installed (#151)
* The `pkgbuild` dependency has been moved to `Suggests`, as it is only needed for packages with compiled code.

* `load_all()` will now work for packages that have testthat tests but do not have testthat installed (#151)

* `load_all(warn_conflicts = TRUE)` becomes more narrow and only warns when a *function* in the global environment masks a *function* in the package, consistent with the docs (#125, #143 @jennybc).

* `load_all()` no longer does a comprehensive check on the `DESCRIPTION` file when loading, instead just checking that it exists and starts with Package (#149, @malcolmbarrett)

* `unload()` no longer warns when it can't unload a namespace.

# pkgload 1.2.0

* Fix test failure in R 4.1 with regards to S4 method registration

* `load_all()` now preserves existing namespaces in working order. In
  particular, it doesn't unload the package's shared library and keeps
  it loaded instead. When reloading, a copy of the SO for the new
  namespace is loaded from a temporary location. These temporary SOs
  are only unloaded on GC and deleted from their temporary location
  via a weak reference attached to the namespace.

  This mechanism ensures that lingering references to the namespace
  keep working as expected. Consequently the namespace
  propagation routine that was added to pkgload as a workaround has
  been removed.

  Note that `.Call()` invocations that pass a string symbol rather
  than a structured symbol may keep crashing, because R will look into
  the most recently loaded SO of a given name. Since symbol
  registration is now the norm, we don't expect this to cause much
  trouble.

* `load_all()` no longer forces all bindings of a namespace to avoid
  lazy-load errors. Instead, it removes exported S3 methods from the
  relevant tables.

  - This improves the loading behaviour with packages that define
    objects in their namespaces lazily (e.g. with `delayedAssign()`).

  - This also makes `load_all()` more predictable after a method has
    been removed from the package. It is now actually removed from the
    generic table. It would previously linger until R was restarted.

* If `load_all()` attaches testthat, it automatically suppresses conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant