Running covr for httr/R6 lead to cyclic namespace dependency failure

[maksymiuks]

Hi!

Recently, while working on the validated package repository, we discovered that it is no longer possible to calculate coverage for R6 and httr packages, as the calculation returns NaN coverage and an error message.

> library('httr')
Error in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) : 
  cyclic namespace dependency detected when loading 'httr', already loading 'covr', 'httr'

We have investigated the issue, only for httr and R6, but it will, in fact, affect any package that covr imports via NAMESPACE and was introduced in 1dbc608 by importingFrom httr package, which inherently, importsFrom R6.

library(covr)
sessionInfo()
(...)
other attached packages:
[1] covr_3.6.4.9007

loaded via a namespace (and not attached):
[1] httr_1.4.7     compiler_4.5.0 lazyeval_0.2.2 R6_2.6.1       rex_1.2.1      tools_4.5.0   

rex, on the other hand, which is loaded because it is used in .onLoad, does not result in a cyclic namespace because it is not, in fact, imported via it.

More specifically, the cyclic dependency is caused by hooks added to the package:

covr/R/covr.R

Line 792 in 5609fb8

"setHook(packageEvent(pkg, \"onLoad\"), function(...) covr:::trace_environment(ns))",

Upon loading R6 (let's use this package as an example for simplicity), the triple colon operator in covr:::trace_environment tries to load covr NAMESPACE, which loads httr, which loads R6, resulting in a loop.

I'd like to ask whether it is something you would consider implementing/accepting a fix for. Right now, it affects only two packages, but it makes it possible that in the future, even more packages will be excluded from calculating coverage.

If you decide to fix it and the fix isn't to remove these imports, I was doing some experiments with custom code coverage calculation that might be useful. If we skip attaching the package and proceed directly to custom code in:

covr/R/covr.R

Lines 757 to 759 in 5609fb8

	writeLines(c(
	paste0("library('", pkg$package, "')"),
	commands), con = outfile)

We could first attach covr from the main library (and deps), then force detach of the package that we want calculated coverage of (like R6) and reload it from the covr custom library with hooks attached. Then we don't enter the loop as the covr would already be loaded.

Let me know what you think about this problem. I can help with implementing/further elaboration if you think it's worth pursuing.