r-hub / rhub

R-hub API client
https://r-hub.github.io/rhub/
Other
354 stars 53 forks source link

Missed dependencies on GCC13 and GCC14 images #642

Closed gowerc closed 1 month ago

gowerc commented 1 month ago

Hey,

Currently trying to run our tests using Rhub but we appear to be getting issues due to missed dependencies on the GCC13 and GCC14 images (other images such as Clang19 and Atlas run fine)

https://github.com/insightsengineering/rbmi/actions/runs/11362703172/job/31605118630#step:5:1762

In particular it seems to be missing the BH dependency from the lock file despite it being in the LinkingTo: section of rstan which our package has in it's suggests.

I suspect this is more a pak issue but I struggled to work out if the issue was in pak or the pipeline code itself sorry.

gaborcsardi commented 1 month ago

LinkingTo dependencies are only needed when a package is compiled from source. They are not needed when installing binary packages. If rstan needs BH at runtime, i could add it as an Imports or Suggests dependency.

See e.g. on macOS, where all packages have binaries available:

❯ pak::pkg_deps_explain("rstan", "BH", dependencies = TRUE)
x BH

Turning off the binaries:

❯ options(pkg.platforms = "source")
❯ pak::pkg_deps_explain("rstan", "BH", dependencies = TRUE)
rstan -> BH
gowerc commented 1 month ago

Apologies I am slightly confused by

i could add it as an Imports or Suggests dependency.

Wouldn't this be an update on rstan's side ?

gaborcsardi commented 1 month ago

Yes, sorry, typo.

gowerc commented 1 month ago

Ah no worries, sure I'll raise an issue on their page.

I must say I am slightly conflicted though, whilst I agree and understand the logic that LinkingTo isn't needed for binary packages as it lists stuff required at install time I feel this is likely to crop up in the future as it semi-conflicts with base install.packages() which appears to by default install c("Depends", "Imports", "LinkingTo") thus masking / side stepping the issue.

That being said I am confused as to why this issue came up on the 2 GCC images but not the Clang or Atlas one 😕

gaborcsardi commented 1 month ago

I must say I am slightly conflicted though, whilst I agree and understand the logic that LinkingTo isn't needed for binary packages as it lists stuff required at install time I feel this is likely to crop up in the future as it semi-conflicts with base install.packages() which appears to by default install c("Depends", "Imports", "LinkingTo") thus masking / side stepping the issue.

Only if it is not installing binaries:

     In all of these, ‘"LinkingTo"’ is omitted for binary
     packages.

?install.packages

That being said I am confused as to why this issue came up on the 2 GCC images but not the Clang or Atlas one 😕

It depends on which packages we are installing from binaries.

IDK what rstan does with BH, but if people need BH to use rstan, then it is better to at least Suggest it, because otherwise people one some platforms (Windows, macOS) will not have it installed, potentially.

gowerc commented 1 month ago

Interesting... When I just tested this using the Posit Package Manager on a Fedora container it still installed BH, unless I've misunderstood or set this up incorrectly sorry

options(
    "repos" = list("CRAN" = "https://packagemanager.posit.co/cran/__linux__/centos8/latest"),
    "HTTPUserAgent" = sprintf("R/%s R (%s)", getRversion(), paste(getRversion(), R.version["platform"], R.version["arch"], R.version["os"]))
)

.libPaths("./rpkg", include.site = FALSE)

install.packages("rstan")
Installing package into ‘/root/rpkg’
(as ‘lib’ is unspecified)
also installing the dependencies ‘utf8’, ‘generics’, ‘numDeriv’, ‘colorspace’, ‘backports’, ‘abind’, ‘tensorA’, ‘pillar’, ‘distributional’, ‘ps’, ‘farver’, ‘labeling’, ‘munsell’, ‘RColorBrewer’, ‘viridisLite’, ‘fansi’, ‘magrittr’, ‘pkgconfig’, ‘gtable’, ‘checkmate’, ‘matrixStats’, ‘posterior’, ‘callr’, ‘cli’, ‘desc’, ‘processx’, ‘R6’, ‘glue’, ‘isoband’, ‘lifecycle’, ‘rlang’, ‘scales’, ‘tibble’, ‘vctrs’, ‘withr’, ‘StanHeaders’, ‘inline’, ‘gridExtra’, ‘Rcpp’, ‘RcppParallel’, ‘loo’, ‘pkgbuild’, ‘QuickJSR’, ‘ggplot2’, ‘RcppEigen’, ‘BH’

<snip>

* installing *binary* package ‘utf8’ ...
* DONE (utf8)
* installing *binary* package ‘generics’ ...
* DONE (generics)
* installing *binary* package ‘numDeriv’ ...
* DONE (numDeriv)
* installing *binary* package ‘colorspace’ ...
* DONE (colorspace)
* installing *binary* package ‘backports’ ...
* DONE (backports)
* installing *binary* package ‘abind’ ...
* DONE (abind)
* installing *binary* package ‘tensorA’ ...
* DONE (tensorA)
* installing *binary* package ‘ps’ ...
* DONE (ps)
* installing *binary* package ‘farver’ ...
* DONE (farver)
* installing *binary* package ‘labeling’ ...
* DONE (labeling)
* installing *binary* package ‘RColorBrewer’ ...
* DONE (RColorBrewer)
* installing *binary* package ‘viridisLite’ ...
* DONE (viridisLite)
* installing *binary* package ‘fansi’ ...
* DONE (fansi)
* installing *binary* package ‘magrittr’ ...
* DONE (magrittr)
* installing *binary* package ‘pkgconfig’ ...
* DONE (pkgconfig)
* installing *binary* package ‘matrixStats’ ...
* DONE (matrixStats)
* installing *binary* package ‘cli’ ...
* DONE (cli)
* installing *binary* package ‘R6’ ...
* DONE (R6)
* installing *binary* package ‘glue’ ...
* DONE (glue)
* installing *binary* package ‘isoband’ ...
* DONE (isoband)
* installing *binary* package ‘rlang’ ...
* DONE (rlang)
* installing *binary* package ‘withr’ ...
* DONE (withr)
* installing *binary* package ‘inline’ ...
* DONE (inline)
* installing *binary* package ‘Rcpp’ ...
* DONE (Rcpp)
* installing *binary* package ‘RcppParallel’ ...
* DONE (RcppParallel)
* installing *binary* package ‘QuickJSR’ ...
* DONE (QuickJSR)
* installing *binary* package ‘BH’ ...       <--------- HERE
* DONE (BH)
* installing *binary* package ‘munsell’ ...
* DONE (munsell)
* installing *binary* package ‘checkmate’ ...
* DONE (checkmate)
* installing *binary* package ‘desc’ ...
* DONE (desc)
* installing *binary* package ‘processx’ ...
* DONE (processx)
* installing *binary* package ‘lifecycle’ ...
* DONE (lifecycle)
* installing *binary* package ‘RcppEigen’ ...
* DONE (RcppEigen)
* installing *binary* package ‘gtable’ ...
* DONE (gtable)
* installing *binary* package ‘callr’ ...
* DONE (callr)
* installing *binary* package ‘scales’ ...
* DONE (scales)
* installing *binary* package ‘vctrs’ ...
* DONE (vctrs)
* installing *binary* package ‘StanHeaders’ ...
* DONE (StanHeaders)
* installing *binary* package ‘pillar’ ...
* DONE (pillar)
* installing *binary* package ‘gridExtra’ ...
* DONE (gridExtra)
* installing *binary* package ‘pkgbuild’ ...
* DONE (pkgbuild)
* installing *binary* package ‘distributional’ ...
* DONE (distributional)
* installing *binary* package ‘tibble’ ...
* DONE (tibble)
* installing *binary* package ‘posterior’ ...
* DONE (posterior)
* installing *binary* package ‘ggplot2’ ...
* DONE (ggplot2)
* installing *binary* package ‘loo’ ...
* DONE (loo)
* installing *binary* package ‘rstan’ ...
* DONE (rstan)
gowerc commented 1 month ago

IDK what rstan does with BH, but if people need BH to use rstan, then it is better to at least Suggest it, because otherwise people one some platforms (Windows, macOS) will not have it installed, potentially.

Ya agreed, I made a corresponding issue for them to update this on their repo https://github.com/stan-dev/rstan/issues/1140

gaborcsardi commented 1 month ago

Interesting... When I just tested this using the Posit Package Manager on a Fedora container it still installed BH,

install.packages() in this case does not know that it is installing binary Linux packages, that's why. R does not support Linux binary packages natively, so PPM sends them looking like source packages. Try on Windows or macOS or with pak, which (usually) does.

gowerc commented 1 month ago

But yer this is definitely an rstan issue so will close this :)