Assign .Depends to package environment to mimic more closely to library()

[yiufung]

• Change introduced

In load_all, assign .Depends to package environment, following the behavior in R attachNamespace(), as shown in R-3-4-3/src/library/base/R/namespace.R#L165.

• Problems & Previous Workaround

It caused some issues that depend on this behavior, for example, using data.table within test_that.

Previously, the workarounds are:

1. add .datatable.aware = TRUE (or other code) to package source file to make it data.table-aware. This is because data.table#cedta check failed at L32 and we had to assign manually to make L37 work. See data.table#2053, devtools#192, dplyr#548,
2. manually change both DESCRIPTION/Imports and NAMESPACE. Not a preferred way as we all use roxygen2 to auto-generate NAMESPACE now, and to avoid that we had to add #' @import data.table directive in R code instead, which is no more convenient than 1).

The pain point is that we cannot use data.table within our package during dev phase. @mattdowle proposed 2 ways at answer. The ii) way ( 2) above) works for dev phase, but it requires source code changed. The i) way works only after package development is finished and when user uses library() to attach the package, so it's not convenient during development.

This commit enables:

1. Only DESCRIPTION/Depends declaration is required, no R code change needed, keeping package source clean
2. Works during package development phase, so devtools::test() works
3. Mimicks even more closely to native library() behavior

• Reproducible example
  As suggested by @hadley at devtools#1472, a minimal package to reproduce the error is created at skeleton_pkg:

With current pkgload:

With modified pkgload:

R version 3.4.3 (2017-11-30)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows >= 8 x64 (build 9200)

Matrix products: default

locale:
[1] LC_COLLATE=Chinese (Traditional)_Hong Kong SAR.950  LC_CTYPE=Chinese (Traditional)_Hong Kong SAR.950   
[3] LC_MONETARY=Chinese (Traditional)_Hong Kong SAR.950 LC_NUMERIC=C                                       
[5] LC_TIME=Chinese (Traditional)_Hong Kong SAR.950    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] pkgload_0.0.0.9000

loaded via a namespace (and not attached):
 [1] compiler_3.4.3        backports_1.1.2       magrittr_1.5          assertthat_0.2.0      R6_2.2.2              rprojroot_1.3-2      
 [7] tools_3.4.3           withr_2.1.1           rstudioapi_0.7.0-9000 yaml_2.1.16           crayon_1.3.4          desc_1.1.1           
[13] testthat_2.0.0        rlang_0.1.6.9002     

[jimhester]

Thanks, this seems like a good improvement! I think we should include your skeleton package in the pkgload tests to prevent regressions.

Also if you could and note to NEWS.md describing the change, mentioning this issue number and your GitHub username.

Thanks again for the PR

[yiufung]

It seems that library() reads from Meta/package.rds(link) to get Dependency info, which, when being packaged, actually ignores useless R dependency (see here), so I just add one more commit to follow the convention.

Result

[yiufung]

Tests added without including the whole skeleton package, just to avoid it becoming bulky. I think this is preventive enough. NEWS.md updated as well.

Travis Build failed twice due to poor network.. I'll do another PR close & reopen to re-trigger the build, which might make the thread a little messy. Hope you don't mind.

[yiufung]

Any other concerns I could address?

[jimhester]

No, it is good, thanks again!

[yiufung]

:P thanks