feat(dock_from_desc): add strict_install parameter (default TRUE)#105
Merged
VincentGuyader merged 1 commit intomasterfrom May 6, 2026
Merged
feat(dock_from_desc): add strict_install parameter (default TRUE)#105VincentGuyader merged 1 commit intomasterfrom
VincentGuyader merged 1 commit intomasterfrom
Conversation
4 tasks
There was a problem hiding this comment.
Pull request overview
This PR introduces a new strict_install parameter to dock_from_desc() to make Docker image builds fail fast when R package installation emits warnings (by prepending options(warn = 2); to install RUN commands), addressing missing/archived/unavailable packages that would otherwise only surface at runtime.
Changes:
- Add
strict_installargument (defaultTRUE) todock_from_desc()and injectoptions(warn = 2);into all generated install RUN commands. - Add an internal helper
.r_strict_prefix()to centralize the strict-install prefix logic. - Add tests for strict vs non-strict behavior and document the behavior change in NEWS + generated Rd.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
R/dock_from_desc.R |
Adds strict_install parameter and applies strict warning-to-error prefix across install RUN lines. |
R/utils.R |
Introduces .r_strict_prefix() helper used by Dockerfile generation. |
tests/testthat/test-dock_from_desc.R |
Adds coverage for strict/non-strict modes and asserts default is strict. |
NEWS.md |
Documents new strict_install feature and behavior change (default strict). |
man/dockerfiles.Rd |
Updates generated documentation to include strict_install parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
265
to
272
| pong <- mapply( | ||
| function(dock, ver, nm) { | ||
| function(dock, ver, nm, strict_prefix) { | ||
| res <- dock$RUN( | ||
| sprintf( | ||
| "%sRscript -e 'remotes::install_github(\"%s\")'", | ||
| "%sRscript -e '%sremotes::install_github(\"%s\")'", | ||
| .github_pat_run_prefix(github_pat), | ||
| strict_prefix, | ||
| ver |
VincentGuyader
force-pushed
the
feat/strict-install-9
branch
from
b999fe6 to
21171f4
Compare
Comment on lines
+84
to
+89
| .r_strict_prefix <- function(strict_install) { | ||
| if (isTRUE(strict_install)) { | ||
| "options(warn = 2); " | ||
| } else { | ||
| "" | ||
| } |
|
|
||
| test_that("dock_from_desc(strict_install = TRUE) prepends options(warn = 2) to every install RUN", { | ||
| skip_if(is_rdevel, "skip on R-devel") | ||
| skip_on_os("mac") |
|
|
||
| test_that("dock_from_desc(strict_install = FALSE) does not prepend options(warn = 2)", { | ||
| skip_if(is_rdevel, "skip on R-devel") | ||
| skip_on_os("mac") |
Closes #9. The previous behaviour of `R -e 'remotes::install_*(...)'` lets a docker build succeed even when an install emitted a warning that silently masked a real failure: "package was archived from CRAN", "partial download, retrying", "no installation candidate". The docker user only finds out the package is missing at container runtime. Add a new `strict_install` parameter, default `TRUE`. When `TRUE`, every install RUN is prefixed with `options(warn = 2);` so any R warning during install becomes a hard error and aborts the build. The opt-out (`strict_install = FALSE`) restores the previous tolerant behaviour for build environments that emit benign warnings (locale defaulting, NTP time-verification, ABI-version notices) the caller does not want to fail the build. The injection covers the five install RUN sites in `dock_from_desc()`: - `R -e 'install.packages("remotes")'` - `Rscript -e 'remotes::install_version(...)'` per CRAN dep - `Rscript -e 'remotes::install_github(...)'` per non-CRAN dep - `R -e 'remotes::install_local("/app.tar.gz")'` (build_from_source = FALSE) - `R -e 'remotes::install_local()'` (build_from_source = TRUE) `dock_from_renv()` is not covered by this PR; it has its own install RUN family (`renv::restore()`, `install.packages("renv")`, `install_version("renv", ...)`) and a separate POC will extend the same `options(warn = 2);` injection there. R CMD check --as-cran: 0 errors / 0 warnings / 0 notes.
VincentGuyader
force-pushed
the
feat/strict-install-9
branch
from
21171f4 to
c658211
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #9.
R -e 'remotes::install_*(...)'lets the docker build succeed evenwhen an install emitted a warning that silently masked a real
failure: "package was archived from CRAN", "partial download,
retrying", "no installation candidate". The docker user only finds
out the package is missing at container runtime.
Add a new
strict_installparameter, defaultTRUE. WhenTRUE,every install RUN is prefixed with
options(warn = 2);so any Rwarning during install becomes a hard error and aborts the build.
The opt-out (
strict_install = FALSE) restores the previoustolerant behaviour for build environments that emit benign
warnings (locale defaulting, NTP time-verification, ABI-version
notices) the caller does not want to fail the build.
Behaviour change
This is a behaviour change for users regenerating their Dockerfile.
Each install RUN now emits
instead of the bare form. To restore the previous behaviour:
Scope
The injection covers the five install RUN sites in
dock_from_desc():R -e 'install.packages("remotes")'Rscript -e 'remotes::install_version(...)'per CRAN depRscript -e 'remotes::install_github(...)'per non-CRAN depR -e 'remotes::install_local("/app.tar.gz")'(build_from_source = FALSE)R -e 'remotes::install_local()'(build_from_source = TRUE)dock_from_renv()is not covered by this PR; it has its owninstall RUN family (
renv::restore(),install.packages("renv"),install_version("renv", ...)) and a follow-up will extend thesame
options(warn = 2);injection there.Test plan
backward-compat-default test.
devtools::test(filter = "dock_from_desc")-> [ FAIL 0 | PASS 69 ].## New featuresdocuments the behaviourchange and the opt-out.
Context
Tier A item from the post-CRAN-prep roadmap (the local prep report
at
/home/ubuntu/thinkr-work/dockerfiler-cran-0.3.0-prep.mdforthe full audit). Companion follow-up is the
user+renv_paths_cacheout-of-box fix in another PR (#100).