Include patched install2.r script for non-latest images

[eitsupi]

Changes suggested in #181 (comment) and #267 (comment).

The install2.r command currently used in rocker/r-ver is from the littler package, so it is taken from CRAN snapshots and will not be updated in the non-latest images.

This PR adds install2.r scripts to rocker_scripts to allow older images to use the latest install2.r commands and to add new features.
First, add a new --skipmissing option to allow #267 to be resolved.

$ /rocker_scripts/bin/install2.r --error --skipmissing --skipinstalled -n -1 foo

In the future, I hope to be able to set up a GitHub Actions workflow to capture upstream changes automatically, but currently we have to edit them manually.

ToDo

• Add a new script patch_install_command.sh to rewrite the /usr/local/bin/install2.r simlink on rocker/r-ver.
• Merge the latest upstream install2.r (check if multiple packages fail with parallel install eddelbuettel/littler#91).
• Update NEWS.md

[eitsupi]

If possible, I would like to submit 24b2589 changes to the littler repo and set up GitHub Actions in this repository to monitor install2.r in the littler repo and automatically create a PR when changes are detected.
@eddelbuettel Do you think the changes in 24b2589 are worth including in littler?

[eddelbuettel]

I haven't had a chance to look but littler is generally quite open to non-breaking clean changes (and I just merged two distinct PRs to it this week....)

We could also do what I often do when I write new scripts for myself and bang on them locally for a bit, commit incrementally and only make a release once I reckon it's "stable" (or "tested" or "worthwhile" putting out or ...).

So you could just run it for a bit 'just here' as a copy, and then PR over so that we aim to be in sync thereafter.

[eitsupi]

Thank you for your prompt reply.
Since this change is obviously niche, I think it makes sense that it be used only within this repository first.

[cboettig]

LGTM

[cboettig]

@eitsupi nice work!

One question, I think it should be preferable not to have to add patch_install2.r explicitly in each of the downstream Dockerfiles? This should be inherited by r-ver, so shouldn't be necessary, right?

I would imagine just adding this to the r-ver Dockerfile, or even calling it from within the install_R.sh recipe where the initial symlink to install2.r is first set up. This way, it also only a one-line change if this version of install2.r winds up in littler and we decide to go back to symlinking that.

I guess once this is in place we can start phasing this in, to avoid scripts breaking when packages are removed from CRAN (like the impending geospatial shift). Part of me wonders if this is for internal use only, if we shouldn't just make the default for skipmissing be TRUE? (ironically, that would be a bit closer to the default behavior of install.packages() and friends, though obviously a deviation from install2.r)?

[eitsupi]

One question, I think it should be preferable not to have to add patch_install2.r explicitly in each of the downstream Dockerfiles? This should be inherited by r-ver, so shouldn't be necessary, right?
I would imagine just adding this to the r-ver Dockerfile, or even calling it from within the install_R.sh recipe where the initial symlink to install2.r is first set up. This way, it also only a one-line change if this version of install2.r winds up in littler and we decide to go back to symlinking that.

My concern was that install_R.sh, which used to work independently, would now be changed to assume the existence of an external file.
Although it is possible to continue executing install_R.sh on its own by checking the existence of /rocker_scripts/bin/install2.r and then reconfiguring the symbolic link, I felt it would be easier to explicitly execute another script and reconfigure the symbolic link.
Would it be better to handle symbolic links in install_R.sh?

I guess once this is in place we can start phasing this in, to avoid scripts breaking when packages are removed from CRAN (like the impending geospatial shift). Part of me wonders if this is for internal use only, if we shouldn't just make the default for skipmissing be TRUE? (ironically, that would be a bit closer to the default behavior of install.packages() and friends, though obviously a deviation from install2.r)?

Since this change may affect many Dockerfiles that have rocker/r-ver set as the base image, it is better not to change the default behavior so that unexpected package removal can be detected.

Incidentally, I am wondering if the default for the --ncpu option could be set to -1. (It would affect a lot of users, but I imagine there's not much downside)

[cboettig]

My concern was that install_R.sh, which used to work independently, would now be changed to assume the existence of an external file.

I think it may make the most sense to include the patch-install.r on the r-ver Dockerfile (as you have, rather than in install_R.sh), but to just let the downstream Dockerfiles inherit it? That would mean install_R.sh would not become less portable. It's true that our downstream install scripts would not be portable if they started using the extra option (though they wouldn't have to if the default of skipmissing was TRUE). The downstream images mostly assume implicitly that they are building on r-ver already, (i.e. that install2.r is available, etc). So I think we gain relatively little by making sure each downstream Dockerfile re-copies the patch-install.r over explicitly?

[eitsupi]

Since install_R.sh can be run without dependencies, if we want to install R on an Ubuntu-based image that is not used in this repository, we can easily install R from source by copying and running install_R.sh only.
Since I used to do that, I thought it would be better not to compromise the portability of install_R.sh considering the possibility that there are other such users.

Another reason is that modifying install_R.sh would make debugging more difficult in terms of runtime. Why don't you run it in a separate layer like this PR first, and then integrate it into install_R.sh when the behavior is considered stable enough?

[cboettig]

@eitsupi I'm with you 100%, we don't need to modify install_R.sh, sorry! I was just confused by the additional changes to the cuda Dockerfiles because I forgot they now also run install_R.sh instead of inheriting from r-ver! that's what I get for reviewing the PR from my phone. this looks good.

[eitsupi]

@cboettig Thank you for merging it.

Since #278 did not include the changes in this PR, a diff change (add patch_install_command.sh to the R4.1.2 images) was created another PR #279, so you seems need to merge that one as well.