chore: small polish bundle (typos, dogfood, doc accuracy)

NEWS.md

@@ -56,6 +56,13 @@
   function arguments, so the branch was unreachable; the success path
   always ran when `build()` returned. The branch is removed; failures
   of `pkgbuild::build()` propagate normally via `stop()`. Closes #98.
+- Small polish bundle (no behavioural changes for end users): fix two
+  `length(x > 0)` typos in `dock_from_desc()` (intent was
+  `length(x) > 0`); drop a duplicate `@export` tag in `dockerignore.R`;
+  use the new `dock$ARG(name, default = ...)` form internally instead
+  of inlining the `=`; tighten a previously brittle regression test
+  that checked for any occurrence of `"remotes"` in the generated
+  Dockerfile.
 
 
 # dockerfiler 0.2.6
@@ -70,9 +77,10 @@
   instead of the lockfile's repo URL; and the RUN is prefixed with
   `. /etc/os-release && ` when the line uses `$VERSION_CODENAME`.
   User-pinned codenames and snapshot-date URLs (e.g. `cran/2024-01-15`)
-  are preserved as-is. The user's PPM scheme and host (including
-  `packagemanager.rstudio.com` and internal mirrors) are preserved on
-  rewrite. Multi-entry `repos` vectors and non-PPM repos are left
+  are preserved as-is. The user's PPM scheme and host (so a
+  `packagemanager.rstudio.com` URL stays on rstudio.com) are preserved
+  on rewrite. Non-PPM repos (including internal mirrors not on the
+  official PPM hosts) and multi-entry `repos` vectors are left
   untouched.
 - `dock_from_renv()` no longer installs `remotes` when `renv_version = NULL`,
   since `remotes` was only needed for the `install_version()` path.

R/dock_from_desc.R

@@ -209,7 +209,7 @@ dock_from_desc <- function(
 
   dock$RUN("R -e 'install.packages(\"remotes\")'")
 
-  if (length(packages_on_cran > 0)) {
+  if (length(packages_on_cran) > 0) {
     ping <- mapply(
       function(dock, ver, nm) {
         res <- dock$RUN(sprintf("Rscript -e 'remotes::install_version(\"%s\",upgrade=\"never\", version = %s)'",
@@ -221,7 +221,7 @@ dock_from_desc <- function(
     )
   }
 
-  if (length(packages_not_on_cran > 0)) {
+  if (length(packages_not_on_cran) > 0) {
     nn <- as.data.frame(
       do.call(
         rbind,

R/dock_from_renv.R

@@ -105,7 +105,7 @@ dock_from_renv <- function(
     AS = AS
   )
   .github_pat_setup(dock, github_pat)
-  dock$ARG(sprintf("RENV_PATHS_CACHE=%s", renv_paths_cache))
+  dock$ARG("RENV_PATHS_CACHE", default = renv_paths_cache)
   dock$ENV(key = "RENV_PATHS_CACHE", value = "${RENV_PATHS_CACHE}")
   if (!is.null(user)) {
     dock$USER(user)
@@ -313,8 +313,11 @@ dock_from_renv <- function(
 
   # Strip a single trailing slash so `cran/latest/` matches `cran/latest`.
   user_url_norm <- sub("/$", "", user_url)
-  # Preserve the user's scheme + host (so a `packagemanager.rstudio.com`
-  # URL or an internal mirror is not silently rewritten to posit.co).
+  # Preserve the user's scheme + host on rewrite (so a
+  # `packagemanager.rstudio.com` URL stays on rstudio.com and is not
+  # silently swapped to posit.co). The PPM detection regex above only
+  # matches the two official PPM hosts; non-PPM internal mirrors are
+  # not entered into this branch at all.
   user_host_prefix <- sub("/cran(/.*)?$", "", user_url_norm)
 
   # Only rewrite when the URL is the bare `cran` or `cran/latest` form.

R/dockerignore.R

@@ -7,8 +7,6 @@
 #' @importFrom fs path file_exists file_create
 #' @importFrom cli cat_bullet
 #'
-#' @export
-#'
 #' @examples
 #' \dontrun{
 #'   docker_ignore_add()

tests/testthat/test-dock_from_renv.R

@@ -210,11 +210,18 @@ socle_install_version <- "remotes::install_version\\(\"renv\", version = \""
   installs_latest <- is.null(renv_version) ||
     (identical(renv_version, "missing") && lf == the_lockfile)
   if (installs_latest) {
-    # When using the latest renv, `remotes` must not be installed at all.
+    # When using the latest renv, `remotes` must not be installed at
+    # all. Tighter regex than `grepl("remotes", ...)` so we don't
+    # false-positive on incidental occurrences of the substring (e.g.
+    # a host name) elsewhere in the Dockerfile.
     expect_false(
       any(grepl("install\\.packages\\([^)]*remotes", out$Dockerfile)),
       info = paste(lf, " & ", renv_version, " => no remotes install")
     )
+    expect_false(
+      any(grepl("remotes::install_version", out$Dockerfile)),
+      info = paste(lf, " & ", renv_version, " => no remotes::install_version")
+    )
   }
 
 }}