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

renv::dependencies() incomplete dependency discovery for R Notebook #7935

Closed
jabenninghoff opened this issue Sep 30, 2020 · 10 comments · Fixed by #8022
Closed

renv::dependencies() incomplete dependency discovery for R Notebook #7935

jabenninghoff opened this issue Sep 30, 2020 · 10 comments · Fixed by #8022
Assignees
Milestone

Comments

@jabenninghoff
Copy link

I'm not sure if this is a bug or a feature request, but renv::dependencies() discovers rmarkdown but not Rcpp or rprojroot when using RStudio R Notebooks.

What I'm trying to do

Create an RStudio project that contains R Notebooks, and store package dependencies using renv.

Expected behavior

All packages needed for R Notebooks are written to renv.lock, including: Rcpp, base64enc, digest, evaluate, glue, highr, htmltools, jsonlite, knitr, magrittr, markdown, mime, rmarkdown, rprojroot, stringi, stringr, tinytex, xfun, yaml

Actual behavior

The following packages are not added to renv.lock: backports, Rcpp, rprojroot

How to reproduce

With only system packages installed and renv installed in the site library:

  1. In RStudio, create a new project in a new directory, using renv
  2. In RStudio, create a new R Notebook. Install the required packages when prompted.
  3. Save the new R Notebook.
  4. Run renv::snapshot()
  5. Running renv::clean() will prompt removal of backports, Rcpp, rprojroot
> renv::snapshot()
The following package(s) will be updated in the lockfile:

# CRAN ===============================
- base64enc   [* -> 0.1-3]
- digest      [* -> 0.6.25]
- evaluate    [* -> 0.14]
- glue        [* -> 1.4.2]
- highr       [* -> 0.8]
- htmltools   [* -> 0.5.0]
- jsonlite    [* -> 1.7.1]
- knitr       [* -> 1.30]
- magrittr    [* -> 1.5]
- markdown    [* -> 1.1]
- mime        [* -> 0.9]
- rlang       [* -> 0.4.7]
- rmarkdown   [* -> 2.4]
- stringi     [* -> 1.5.3]
- stringr     [* -> 1.4.0]
- tinytex     [* -> 0.26]
- xfun        [* -> 0.18]
- yaml        [* -> 2.2.1]

Do you want to proceed? [y/N]: y
* Lockfile written to '~/rnotebook-test/renv.lock'.
> renv::clean()
* No stale lockfiles were found.
* No temporary directories were found in the project library.
* No non-system packages were discovered in the system library.
The following packages are installed in the project library,
but appear to be no longer used in your project.

	backports, Rcpp, rprojroot

These packages will be removed.

Do you want to proceed? [y/N]: n
* The project is already clean.

Environment

macOS Mojave 10.14.6
RStudio 1.3.1093
R 4.0.2 (homebrew package)
renv 0.12.0

More info

From the FAQ, I know that renv relies on static analysis. In theory, it should be possible to inspect the header of the .Rmd file and look for the output: html_notebook identifier. The missing dependencies here are really rprojroot and Rcpp, as backports is required by rprojroot.

@jabenninghoff
Copy link
Author

This issue seems related to rstudio/renv#481. Also, as a workaround, is there a way to manually add dependencies to renv.lock?

@kevinushey
Copy link
Contributor

Where are you seeing that those packages are required? For example:

> tools::package_dependencies("rmarkdown", recursive = TRUE)$rmarkdown
 [1] "tools"     "utils"     "knitr"     "yaml"      "htmltools" "evaluate"  "jsonlite"  "mime"      "tinytex"   "xfun"      "methods"   "stringr"   "digest"    "grDevices"
[15] "base64enc" "rlang"     "highr"     "markdown"  "glue"      "magrittr"  "stringi"   "stats"    

In other words, Rcpp, rprojroot and backports are not direct dependencies of rmarkdown, so R Notebooks by themselves aren't enough of a signal for renv to see those packages as dependencies.

@jabenninghoff
Copy link
Author

jabenninghoff commented Oct 1, 2020

RStudio is stating that these are required for R Notebooks. The message I get when trying to create a new R Notebook in a new project is:

Create R Notebook requires updated versions of the following packages: Rcpp, base64enc, digest, evaluate, glue, highr, htmltools, jsonlite, knitr, magrittr, markdown, mime, rmarkdown, rprojroot, stringi, stringr, tinytex, xfun, yaml.

Do you want to install these packages now?

Which is rmarkdown, its dependencies, plus Rcpp, rprojroot and its dependencies (backports)

@jabenninghoff
Copy link
Author

jabenninghoff commented Oct 1, 2020

Interesting, is this an RStudio regression? #4628

The dependencies for R Markdown appear to be stored stored here: https://github.com/rstudio/rstudio/blob/master/src/cpp/session/resources/dependencies/r-packages.json

@kevinushey
Copy link
Contributor

It appears to be a regression indeed! I'm going to move this to the RStudio repository.

@kevinushey kevinushey transferred this issue from rstudio/renv Oct 1, 2020
@kevinushey kevinushey added this to the v1.4 milestone Oct 1, 2020
@jabenninghoff
Copy link
Author

Thanks! To clarify, the issue appears to be that Rcpp and rprojroot are erroneously included under rmarkdown.

        "rmarkdown": {
            "description": "R Markdown",
            "packages": [
                "Rcpp",
                "base64enc",
                "digest",
                "evaluate",
                "glue",
                "highr",
                "htmltools",
                "jsonlite",
                "knitr",
                "magrittr",
                "markdown",
                "mime",
                "rmarkdown",
                "rprojroot",
                "stringi",
                "stringr",
                "tinytex",
                "xfun",
                "yaml"
            ]
        },

@jmcphers
Copy link
Member

jmcphers commented Oct 1, 2020

This was copied from a much older list. Here's the same source in the 1.2 release:

https://github.com/rstudio/rstudio/blob/v1.2-patch/src/gwt/src/org/rstudio/studio/client/common/dependencies/DependencyManager.java#L286-L299

@kevinushey is it possible that Rcpp is a platform specific dependency? (i.e. maybe it's only required on Linux where some of these packages have native code?)

@kevinushey
Copy link
Contributor

I don't think so. Rather, I believe that Rcpp and rprojroot used to be (recursive) dependencies of the rmarkdown package, but that is no longer true.

@ronblum ronblum added the bug label Oct 2, 2020
@jmcphers jmcphers self-assigned this Oct 2, 2020
@jmcphers
Copy link
Member

jmcphers commented Oct 2, 2020

These are heavyweight dependencies, so if we don't need them we should definitely axe them.

@kfeinauer
Copy link
Contributor

Verified fixed on version 1.4.944

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants