Skip to content

ignore packages when recursively enumerating dependencies#654

Merged
aronatkins merged 2 commits intomasterfrom
aron-recursive-ignore
Feb 14, 2022
Merged

ignore packages when recursively enumerating dependencies#654
aronatkins merged 2 commits intomasterfrom
aron-recursive-ignore

Conversation

@aronatkins
Copy link
Copy Markdown
Contributor

previously, we ignored only direct dependencies

Given a Shiny application having:

library(shiny)
library(emo) # hadley/emo

ui <- fluidPage("So much", emo::ji("dancing"))
server <- function(input, output) {}
shinyApp(ui = ui, server = server)

The default set of dependencies:

> packrat:::appDependencies(".")
 [1] "R6"          "Rcpp"        "assertthat"  "base64enc"   "bslib"       "cachem"      "commonmark" 
 [8] "crayon"      "digest"      "ellipsis"    "emo"         "fastmap"     "fontawesome" "fs"         
[15] "generics"    "glue"        "htmltools"   "httpuv"      "jquerylib"   "jsonlite"    "later"      
[22] "lifecycle"   "lubridate"   "magrittr"    "mime"        "packrat"     "promises"    "purrr"      
[29] "rappdirs"    "rlang"       "sass"        "shiny"       "sourcetools" "stringi"     "stringr"    
[36] "withr"       "xtable"     

Prior to this change, with "emo" and "Rcpp" as ignores, only the top-level "emo" is ignored:

> packrat::set_opts(ignored.packages = c("emo","Rcpp"), persist = FALSE)
> packrat:::appDependencies(".")
 [1] "R6"          "Rcpp"        "base64enc"   "bslib"       "cachem"      "commonmark"  "crayon"     
 [8] "digest"      "ellipsis"    "fastmap"     "fontawesome" "fs"          "glue"        "htmltools"  
[15] "httpuv"      "jquerylib"   "jsonlite"    "later"       "lifecycle"   "magrittr"    "mime"       
[22] "packrat"     "promises"    "rappdirs"    "rlang"       "sass"        "shiny"       "sourcetools"
[29] "withr"       "xtable"     

After this change, both "emo" and "Rcpp" are ignored:

> packrat::set_opts(ignored.packages = c("emo","Rcpp"), persist = FALSE)
> packrat:::appDependencies(".")
 [1] "R6"          "base64enc"   "bslib"       "cachem"      "commonmark"  "crayon"      "digest"     
 [8] "ellipsis"    "fastmap"     "fontawesome" "fs"          "glue"        "htmltools"   "httpuv"     
[15] "jquerylib"   "jsonlite"    "later"       "lifecycle"   "magrittr"    "mime"        "packrat"    
[22] "promises"    "rappdirs"    "rlang"       "sass"        "shiny"       "sourcetools" "withr"      
[29] "xtable"     

If we were to choose a dependency with substantial dependencies, we'll see those dependencies disappear, as well; notice that "stringi" is not included as a result of ignoring "stringr"; neither of these are direct project dependencies:

> packrat::set_opts(ignored.packages = c("stringr"), persist = FALSE)
> packrat:::appDependencies(".")
 [1] "R6"          "Rcpp"        "assertthat"  "base64enc"   "bslib"       "cachem"      "commonmark" 
 [8] "crayon"      "digest"      "ellipsis"    "emo"         "fastmap"     "fontawesome" "fs"         
[15] "generics"    "glue"        "htmltools"   "httpuv"      "jquerylib"   "jsonlite"    "later"      
[22] "lifecycle"   "lubridate"   "magrittr"    "mime"        "packrat"     "promises"    "purrr"      
[29] "rappdirs"    "rlang"       "sass"        "shiny"       "sourcetools" "withr"       "xtable"     

@aronatkins aronatkins requested a review from kevinushey January 31, 2022 14:36
Copy link
Copy Markdown
Contributor

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@toph-allen
Copy link
Copy Markdown
Contributor

What I did

I smoke tested this and #656 in a merged branch.

I tested the changes by:

  • initializing Packrat in a test repo (using external.packages = "packrat", which I assume is a requirement for working with local branches);
  • running packrat:::appDependencies(".") and reconnect::writeManifest() before and after running packrat::set_opts(ignored.packages = c("emo", "Rcpp", "stringr"));
  • running a few other packrat functions.

Results

In both cases, all expected packages were ignored. I didn't find any other issues using basic Packrat functions related to these changes.

Running packrat::snapshot(), I got an error Error: Unable to retrieve package records for the following packages: - 'packrat', which I'm assuming is due to packrat being listed in external.packages and/or being built from a local branch.

@aronatkins
Copy link
Copy Markdown
Contributor Author

@toph-allen - I think you're right about the packrat dependency. The .snapshotImpl function accepts an implicit.packrat.dependency argument to drop the packrat dependency, but I don't know how best to avoid the snapshot error with a local packrat build. @kevinushey - any testing advice on this?

@kevinushey
Copy link
Copy Markdown
Contributor

For Packrat's own tests, we use a dummy repository with a mock version of packrat available:

# Force Packrat tests to believe the currently installed / tested
# version of Packrat is on CRAN.
cat("Repository: CRAN",
file = "packrat/DESCRIPTION",
sep = "\n",
append = TRUE)

Could something similar be done for testing here? You could also try appending

Repository: CRAN

to the DESCRIPTION file of the external copy of packrat as well, to convince packrat that packrat is available on a CRAN-like repository.

If not, it might suffice to just add some code to packrat to check whether tests are running, and allow for packrat to be missing while running tests.

@aronatkins aronatkins force-pushed the aron-recursive-ignore branch from eeebb26 to 29733b2 Compare February 14, 2022 14:23
@aronatkins aronatkins merged commit 7118074 into master Feb 14, 2022
@aronatkins aronatkins deleted the aron-recursive-ignore branch February 14, 2022 15:28
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 this pull request may close these issues.

3 participants