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

Additional dependencies via Config/Needs/ #642

Open
ms609 opened this issue Aug 16, 2021 · 14 comments
Open

Additional dependencies via Config/Needs/ #642

ms609 opened this issue Aug 16, 2021 · 14 comments
Labels
feature a feature request or enhancement

Comments

@ms609
Copy link
Contributor

ms609 commented Aug 16, 2021

Following on from #459, I am using install_deps('soft', 'Config/Needs/coverage') in a continuous integration workflow.

This installs my package's dependencies, and any package listed in Config/Needs/coverage: but not the dependencies of the latter.

Specifically, my DESCRIPTION includes Config/Needs/coverage: covr, but install_deps does not install packages from "covr"'s imports, such as "rex", so when I run the script I see "package not installed: 'rex'". I believe that this behaviour differs from that of 'soft': dependencies of soft dependencies are installed.

A possible workaround is to list all the "covr" dependencies under "Config/Needs/coverage:", but this would require me to keep this up to date as dependencies change in the future, which seems inelegant.

@jimhester
Copy link
Member

Can you give an example workflow and build log which is failing to install the second order dependencies?

@ms609
Copy link
Contributor Author

ms609 commented Aug 17, 2021

Here we go:

Commit

Workflow file

DESCRIPTION file

Build log

@dragosmg
Copy link

Let me start by thanking you all for the hard work you put into this package.

I have encountered a similar issue. I am currently working on the Arrow R package and wanted to add Config/Needs/website: pkgdown to our DESCRIPTION file.

In this context, my expectation is that using remotes::install_deps(dependencies = "Config/Needs/Website") installs both pkgdown and its dependencies*. However, only pkgdown gets installed. Our CI pipeline building the pkgdown website fails due to, for example, fs or memoise not being available - log can be seen here, but the key part seems to be this one:

> pkgdown::build_site(install = FALSE)
Error in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) : 
  there is no package called 'memoise'
Calls: loadNamespace ... loadNamespace -> withRestarts -> withOneRestart -> doWithOneRestart
Execution halted
1
Error: `docker-compose --file /home/runner/work/arrow/arrow/docker-compose.yml run --rm ubuntu-docs` exited with a non-zero exit code 1, see the process log above.

Somewhat of a reprex (I call this from inside the RStudio project corresponding to the package):

remove.packages(c("pkgdown", "fs", "memoise"))
remotes::install_deps(".", dependencies = "Config/Needs/website")
packageVersion("pkgdown")
packageVersion("fs")
packageVersion("memoise")

It might be worth mentioning that both dependencies = c("hard", "Config/Needs/website")) and dependencies = c("soft", "Config/Needs/website")) do the job, i.e. install pkgdown and its dependencies:

remove.packages(c("pkgdown", "fs", "memoise"))
remotes::install_deps(".", dependencies = c("hard", "Config/Needs/website"))
packageVersion("pkgdown")
packageVersion("fs")
packageVersion("memoise")

but that feels a bit hacky to me as this use case is not clearly stated in the docs and, as a consequence, it makes it a bit difficult to depend on this behaviour.

* I cannot see a scenario in which I would want to install only pkgdown (without its dependencies) as that would mean it's virtually unusable.

@gaborcsardi
Copy link
Member

I suggest you use the v2 tag of r-lib/actions which should not have these issues: https://github.com/r-lib/actions#releases-and-tags

@dragosmg
Copy link

dragosmg commented Jan 10, 2022

Thanks for your reply, Gábor! In this case (Arrow), we use both GHA and other CI workflows - the CI pipelines are a bit more complex. This is only one step in a much bigger picture. I don't think we would be able to switch everything to GHA and use r-lib/actions.

@dragosmg
Copy link

dragosmg commented Jan 10, 2022

A follow-up question: does this mean that, for the time being, the only way to use Config/Needs/... reliably is with pak?

@gaborcsardi
Copy link
Member

If you trust yourself that this is a bug in remotes, then it seems so.

@jimhester
Copy link
Member

I am not sure what the objection to dependencies = c("soft", "Config/Needs/website")) is. Without the soft this is essentially equivalent to install_github("r-lib/pkgdown", dependencies = FALSE), e.g. it would only install pkgdown and not any of its dependencies, which is exactly the behavior you describe. Using c("soft", "Config/Needs/website") is the intended usage.

@dragosmg
Copy link

dragosmg commented Jan 11, 2022

I did not mention this as I'm still trying to figure out if I can (consistently) reproduce an issue I noticed / perceived with "soft" (which directed me towards "hard" and, what I think it was a non-intended use of "hard"). I found "soft" to be slightly different from TRUE as in "soft" is much more recursive. Running install_deps(".", dependencies = c("soft", "Config/Needs/website")) from arrow resulted in the installation of hundreds, if not thousands of (mostly binary) packages over more than 1 hour on a Mac. This made me think I did something wrong and somehow "soft" might not be the behaviour / functionality I was looking for.

@jimhester
Copy link
Member

jimhester commented Jan 11, 2022

so the way TRUE works is it gives you the 1st order soft dependencies and the 2nd order hard dependencies. soft gives you all of the recursive soft dependencies even the N order dependencies, so you are right, you end up basically installing all of CRAN.

So I guess above I should have written 'hard' rather than 'soft'.

@dragosmg
Copy link

@jimhester Thanks for confirming what I noticed. In conclusion, using c("hard", "Config/Needs/website") would not fall under unintended use and, thus, should be appropriate for our use case.
Do you think it might be worth including something similar to your explanation above in the docs for the dependencies argument? As it stands the docs say:

The value "soft" means the same as TRUE,

which I read as "soft" is the equivalent of TRUE.

@jonkeane
Copy link

Also, I'm curious if there is a way to have the same behavior as dependencies = TRUE and install one (or more) of the Config/Needs fields at the same time?

We could do remotes::install_deps(dependencies = TRUE) and remotes:: install_deps(dependencies =c("hard", "Config/Needs/website")) to accomplish that, but I wonder if having a direct way of doing that with one call to install_deps() would be nice?

@dragosmg
Copy link

I ran a small experiment to understand the differences between "soft", "hard" and TRUE a bit better.

Conclusion:

  • TRUE is definitely different from "soft", and
  • the combination we need (TRUE + "Config/Needs/website") cannot be achieved with a single function call. (i.e. it is different from c("hard", "Config/Needs/website")).
Details

Investigation results:

test description number of packages installed
1 dependencies = TRUE 64
2 dependencies = 'hard' 12
3 dependencies = c('hard', 'Config/Needs/website') 61
4 dependencies = 'soft' 1819
5 dependencies = c('soft', 'Config/Needs/website') 1819
6 dependencies = c('Config/Needs/website') 1

I excluded dependencies = "soft" & combinations of "soft" since they are too recursive and end up installing an excessive number of packages.
dependencies = "hard" & dependencies = "Config/Needs/website" are too narrow.
I compared dependencies = TRUE to dependencies = c("hard", "Config/Needs/website"). While the overall number of installed packages looks promising (64 vs 61), there isn't complete overlap.

indicator value
packages exclusive to test 1 21
packages exclusive to test 3 18
packages in common 43

We can safely conclude that dependencies = c("hard", "Config/Needs/website") is not equal to dependencies = TRUE + dependencies = c("Config/Needs/website").
We definitely need "Config/Needs/website" - in our case pkgdown and its dependencies, but in combination with dependencies = "hard" it is too narrow in scope, while with dependencies = "soft" it is much too wide.

How I got there?

  • I removed all installed packages from the system library (with the exception of "base" and "recommended"packages)
  • I installed remotes and rstudioapi in the system library
  • each test installed packages to its own folder (configured via the R_LIBS_USER environment variable) with the R session being restarted for each run
  • I then compared the contents of the 6 libraries

@gaborcsardi
Copy link
Member

Hopefully the situation is better now with pak?

@gaborcsardi gaborcsardi added the feature a feature request or enhancement label Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants