rstudio / packrat

Packrat is a dependency management system for R
http://rstudio.github.io/packrat/
402 stars 90 forks source link

packrat (renv) dependency lookup inadvertently ignores projects in directories prefixed with `/data/`. #684

Closed jimhester closed 2 years ago

jimhester commented 2 years ago

We have a situation where our system temp directory is actually a symlink to /data/tmp, and the renv default exclusion rules cause all inferred packages to be ignored when trying to deploy content to RStudio Connect.

While in our case /data is the first directory after the root, it seems this will happen if a directory is called data anywhere in the tree. I think ideally these exclusions would match only child directories of the project root. This should be a relatively minimal example.

dir.create("data/foo", recursive = TRUE)
writeLines("library(glue)", "data/foo/test.R")
options(renv.renvignore.exclude = packrat:::ignoresForRenv(packrat:::opts$ignored.directories()))
path <- normalizePath("data/foo")
packrat:::renv$dependencies(path, root = path)
#> Finding R package dependencies ... Done!
#> [1] Source  Package Require Version Dev    
#> <0 rows> (or 0-length row.names)

Created on 2022-08-11 by the reprex package (v2.0.1)

cc @aronatkins, who I believe added this code in https://github.com/rstudio/packrat/pull/647

A workaround for us is to set options(packrat.dependency.discovery.renv = FALSE), but ideally users wouldn't need to resort to this :)

aronatkins commented 2 years ago

Thanks for filing this.

Our intent is to do as you describe - ignore data only as a child of the project root. I'm guessing that something is treating the current directory as the project root rather than root, but we'll need to dig in to be sure.

kevinushey commented 2 years ago

Part of the reason is probably the asis = TRUE here:

https://github.com/rstudio/packrat/blob/28e4a1e0ddb87c6e206c1c73c0f3807b0d09a967/R/dependencies.R#L145

Compare e.g.

> options(renv.renvignore.exclude = c("/data/"))
> renv:::renv_renvignore_pattern_extra("exclude", "/root")
[1] "^\\Q/root/\\E\\Qdata/\\E$"
> 
> options(renv.renvignore.exclude = structure("/data/", asis = TRUE))
> renv:::renv_renvignore_pattern_extra("exclude", "/root")
[1] "/data/"
attr(,"asis")
[1] TRUE

I think we had some motivation to use asis = TRUE here but I don't recall the details...

aronatkins commented 2 years ago

Capturing more history:

The "asis" support was added to renv here: https://github.com/rstudio/renv/pull/866

> packrat::opts$ignored.directories()
[1] "data" "inst"

The intent was to rewrite the Packrat set of ignored directories into renv-equivalents so they would apply only at the project root. The "asis" attribute was meant to avoid any parsing of the rule by renv.

The filtering in Packrat (given a recursive list.files): https://github.com/rstudio/packrat/blob/28e4a1e0ddb87c6e206c1c73c0f3807b0d09a967/R/dependencies.R#L203-L223