Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/bundlePackage.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ computePackageDependencies <- function(
extraPackages,
verbose = verbose
)
} else if (file.exists(renvLockFile(bundleDir))) {
} else if (ensureRenvLockFile(bundleDir)) {
# This ignores extraPackages; if you're using a lockfile it's your
# responsibility to install any other packages you need
taskStart(quiet, "Capturing R dependencies from renv.lock")
Expand Down
39 changes: 39 additions & 0 deletions R/bundlePackageRenv.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ snapshotRenvDependencies <- function(
progress = FALSE
)
renv::snapshot(bundleDir, packages = deps$Package, prompt = FALSE)
# renv::snapshot() respects RENV_PATHS_LOCKFILE and renv profiles, so the
# lockfile may have been written to a non-standard location. Copy it into the
# bundle dir so parseRenvDependencies() can find it.
ensureRenvLockFile(bundleDir)
defer(removeRenv(bundleDir))

parseRenvDependencies(bundleDir, snapshot = TRUE)
Expand Down Expand Up @@ -172,6 +176,41 @@ renvLockFile <- function(bundleDir) {
file.path(bundleDir, "renv.lock")
}

# Ensure the renv lockfile is at the standard bundle location.
# If found at a custom location (via RENV_PATHS_LOCKFILE or renv profiles),
# copies it into bundleDir/renv.lock, overwriting any stale lockfile.
# Returns TRUE if the lockfile is available, FALSE otherwise.
ensureRenvLockFile <- function(bundleDir) {
standard <- renvLockFile(bundleDir)
resolved <- renv::paths$lockfile(project = bundleDir)

# If the resolved file exists, and is different from the standard location,
# copy it to the standard location.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to copy it at all? Why can't we use it where it lives? It's a little odd to me that this function would alter the contents of the current working directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that's confusing about this is that it's actually not copying it into the working directory, but actually into the temporary directory the rsconnect creates and copies files over to a temporary directory

bundleDir <- bundleAppDir(
appDir = appDir,
appFiles = appFiles,
appMode = appMetadata$appMode
)
defer(unlink(bundleDir, recursive = TRUE))
which calls

rsconnect/R/bundle.R

Lines 1 to 12 in c704931

# Given a path to an directory and a list of files in that directory, copies
# those files to a new temporary directory. Performs some small modifications
# in this process, including renaming single-file Shiny apps to "app.R" and
# stripping packrat and renv commands from .Rprofile. Returns the path to the
# temporary directory.
bundleAppDir <- function(
appDir,
appFiles,
appPrimaryDoc = NULL,
appMode = NULL,
verbose = FALSE
) {

But thinking about this more, we might need to make sure that we can handle relative paths in RENV_PATHS_LOCKFILE since when this is run inside of the temp bundle dir, those might not work (e.g. if RENV_PATHS_LOCKFILE="../renv.lock" and the app is at /home/jonkeane/Documents/myapp/subdir/, my lock file is at /home/jonkeane/Documents/myapp/renv.lock we don't want the target of this copy to be /tmp/.../renv.lock

if (file.exists(resolved)) {
# Normalize to avoid self-copy (e.g., /var vs /private/var on macOS)
if (
normalizePath(resolved, mustWork = FALSE) !=
normalizePath(standard, mustWork = FALSE)
) {
if (file.exists(standard)) {
cli::cli_warn(c(
"Using lockfile at {.path {resolved}} instead of {.path {standard}}.",
i = "The lockfile in the project root may be outdated.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I run this function twice, I'm going to get this warning, even though the files themselves are identical. Should we be checking the contents of the files instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add checking the contents, but running user-exported commands in rsconnect, this path should not alter someone's app directory (so this shouldn't happen in practice unless someone calls rsconnect:::ensureRenvLockFile (or the other internal functions that always operate on the temp bundle dir

i = "Remove it to silence this warning."
))
}
if (!file.copy(resolved, standard, overwrite = TRUE)) {
cli::cli_abort(
"Failed to copy lockfile from {.path {resolved}} to {.path {standard}}."
)
}
}
return(TRUE)
}

file.exists(standard)
}

removeRenv <- function(path, lockfile = TRUE) {
if (lockfile) {
unlink(renvLockFile(path))
Expand Down
29 changes: 29 additions & 0 deletions tests/testthat/_snaps/bundlePackage.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,35 @@
i Capturing R dependencies from renv.lock
v Found 3 dependencies

# can capture deps from renv lockfile in custom location (RENV_PATHS_LOCKFILE)

Code
pkgs <- bundlePackages(app_dir)
Message
i Capturing R dependencies from renv.lock
v Found 3 dependencies

# warns when custom lockfile overwrites existing standard lockfile

Code
pkgs <- bundlePackages(app_dir)
Condition
Warning:
Using lockfile at '<path>' instead of '<path>'.
i The lockfile in the project root may be outdated.
i Remove it to silence this warning.
Message
i Capturing R dependencies from renv.lock
v Found 3 dependencies

# can capture deps from renv lockfile with renv profile

Code
pkgs <- bundlePackages(app_dir)
Message
i Capturing R dependencies from renv.lock
v Found 3 dependencies

# can capture deps with packrat even when renv lockfile present

Code
Expand Down
77 changes: 77 additions & 0 deletions tests/testthat/test-bundlePackage.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,83 @@ test_that("can capture deps from renv lockfile", {
expect_equal(list.files(app_dir), c("foo.R", "renv.lock"))
})

test_that("can capture deps from renv lockfile in custom location (RENV_PATHS_LOCKFILE)", {
skip_if_not_installed("foreign")
skip_if_not_installed("MASS")

withr::local_options(renv.verbose = FALSE)

app_dir <- local_temp_app(list(foo.R = "library(foreign); library(MASS)"))

# Create renv.lock in the standard location first

renv::snapshot(app_dir, prompt = FALSE)

# Move the lockfile to a custom location outside the app dir
custom_lock_dir <- withr::local_tempdir()
custom_lock_path <- file.path(custom_lock_dir, "renv.lock")
file.rename(file.path(app_dir, "renv.lock"), custom_lock_path)

# Set RENV_PATHS_LOCKFILE to point to the custom location
withr::local_envvar(RENV_PATHS_LOCKFILE = custom_lock_path)

# Should find the lockfile at the custom location and capture deps.
expect_snapshot(pkgs <- bundlePackages(app_dir))

# renv is included by the renv.lock
expect_named(pkgs, c("foreign", "MASS", "renv"), ignore.order = TRUE)
expect_named(pkgs$foreign, c("Source", "Repository", "description"))
expect_named(pkgs$MASS, c("Source", "Repository", "description"))
})

test_that("warns when custom lockfile overwrites existing standard lockfile", {
skip_if_not_installed("foreign")
skip_if_not_installed("MASS")

withr::local_options(renv.verbose = FALSE)

app_dir <- local_temp_app(list(foo.R = "library(foreign); library(MASS)"))

# Create renv.lock in the standard location
renv::snapshot(app_dir, prompt = FALSE)

# Copy the lockfile to a custom location, leaving the original in place
custom_lock_dir <- withr::local_tempdir()
custom_lock_path <- file.path(custom_lock_dir, "renv.lock")
file.copy(file.path(app_dir, "renv.lock"), custom_lock_path)

# Set RENV_PATHS_LOCKFILE to point to the custom location
withr::local_envvar(RENV_PATHS_LOCKFILE = custom_lock_path)

# Should warn about overwriting the standard lockfile
expect_snapshot(pkgs <- bundlePackages(app_dir), transform = function(x) {
gsub("'[^']+'", "'<path>'", x)
})

expect_named(pkgs, c("foreign", "MASS", "renv"), ignore.order = TRUE)
})

test_that("can capture deps from renv lockfile with renv profile", {
skip_if_not_installed("foreign")
skip_if_not_installed("MASS")

withr::local_options(renv.verbose = FALSE)

app_dir <- local_temp_app(list(foo.R = "library(foreign); library(MASS)"))

# Set RENV_PROFILE so renv resolves the lockfile from the profile path
withr::local_envvar(RENV_PROFILE = "testing")
renv::snapshot(app_dir, prompt = FALSE)

# Should find the lockfile at the profile location and capture deps.
expect_snapshot(pkgs <- bundlePackages(app_dir))

# renv is included by the renv.lock
expect_named(pkgs, c("foreign", "MASS", "renv"), ignore.order = TRUE)
expect_named(pkgs$foreign, c("Source", "Repository", "description"))
expect_named(pkgs$MASS, c("Source", "Repository", "description"))
})

test_that("can capture deps with packrat even when renv lockfile present", {
skip_if_not_installed("foreign")
skip_if_not_installed("MASS")
Expand Down
Loading