From 4c55d69f25ecb9fad2b56bd5021e99e94ac4b22b Mon Sep 17 00:00:00 2001
From: Aron Atkins <aron@gweep.net>
Date: Tue, 25 Jul 2023 08:41:05 -0400
Subject: [PATCH] preserve renv.lock (not the renv directory) for renv projects
 (#927)

* preserve renv.lock (not the renv directory) for renv projects

fixes #926

* named lockfile argument
---
 NEWS.md                             |  4 ++++
 R/bundlePackage.R                   |  5 +++--
 R/bundlePackageRenv.R               |  8 +++++---
 tests/testthat/test-bundlePackage.R |  4 ++--
 tests/testthat/test-writeManifest.R | 21 +++++++++++++++++++++
 5 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 2cd8ec7d..16804010 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,9 @@
 # rsconnect (development version)
 
+* Deploying from an renv project includes the `renv.lock` in the bundle. A
+  manifest created for an renv project references the `renv.lock` in the
+  `manifest.json`. (#926)
+
 # rsconnect 1.0.1
 
 * `deployDoc()` includes `.Rprofile`, `requirements.txt` and `renv.lock` when
diff --git a/R/bundlePackage.R b/R/bundlePackage.R
index 62233b04..cad17ed4 100644
--- a/R/bundlePackage.R
+++ b/R/bundlePackage.R
@@ -39,8 +39,9 @@ computePackageDependencies <- function(bundleDir,
     # responsibility to install any other packages you need
     taskStart(quiet, "Capturing R dependencies from renv.lock")
     deps <- parseRenvDependencies(bundleDir)
-    # Once we've captured the deps, we can remove renv from the bundle
-    removeRenv(bundleDir)
+    # Once we've captured the deps, we can remove the renv directory
+    # from the bundle (retaining the renv.lock).
+    removeRenv(bundleDir, lockfile = FALSE)
   } else if (isFALSE(getOption("rsconnect.packrat", FALSE))) {
     taskStart(quiet, "Capturing R dependencies with renv")
     # TODO: give user option to choose between implicit and explicit
diff --git a/R/bundlePackageRenv.R b/R/bundlePackageRenv.R
index 4f7460f5..85b793a9 100644
--- a/R/bundlePackageRenv.R
+++ b/R/bundlePackageRenv.R
@@ -43,8 +43,8 @@ parseRenvDependencies <- function(bundleDir, snapshot = FALSE) {
 
     # Generate a library from the lockfile
     lib_dir <- dirCreate(file.path(bundleDir, "renv_library"))
-    renv::restore(bundleDir, library = lib_dir, prompt = FALSE)
     defer(unlink(lib_dir, recursive = TRUE))
+    renv::restore(bundleDir, library = lib_dir, prompt = FALSE)
 
     deps$description <- lapply(
       deps$Package,
@@ -136,7 +136,9 @@ renvLockFile <- function(bundleDir) {
   file.path(bundleDir, "renv.lock")
 }
 
-removeRenv <- function(path) {
-  unlink(renvLockFile(path))
+removeRenv <- function(path, lockfile = TRUE) {
+  if (lockfile) {
+    unlink(renvLockFile(path))
+  }
   unlink(file.path(path, "renv"), recursive = TRUE)
 }
diff --git a/tests/testthat/test-bundlePackage.R b/tests/testthat/test-bundlePackage.R
index 7759eea0..21e00ea2 100644
--- a/tests/testthat/test-bundlePackage.R
+++ b/tests/testthat/test-bundlePackage.R
@@ -29,8 +29,8 @@ test_that("can capture deps from renv lockfile", {
   expect_named(pkgs$foreign, c("Source", "Repository", "description"))
   expect_named(pkgs$MASS, c("Source", "Repository", "description"))
 
-  # No renv lockfile or directory left behind
-  expect_equal(list.files(app_dir), "foo.R")
+  # No renv directory left behind, but the renv.lock is preserved.
+  expect_equal(list.files(app_dir), c("foo.R", "renv.lock"))
 })
 
 # -------------------------------------------------------------------------
diff --git a/tests/testthat/test-writeManifest.R b/tests/testthat/test-writeManifest.R
index 7d7a0637..711d9384 100644
--- a/tests/testthat/test-writeManifest.R
+++ b/tests/testthat/test-writeManifest.R
@@ -7,6 +7,27 @@ makeManifest <- function(appDir, appPrimaryDoc = NULL, ...) {
   manifestJson
 }
 
+test_that("renv.lock is included for renv projects", {
+  withr::local_options(renv.verbose = FALSE)
+
+  app_dir <- local_temp_app(list(app.R = "library(foreign); library(MASS)"))
+  renv::snapshot(app_dir, prompt = FALSE)
+
+  manifest <- makeManifest(app_dir)
+  # note: we don't see an .Rprofile here because we only renv::snapshot and
+  # do not fully create an renv project.
+  expect_named(manifest$files, c("app.R", "renv.lock"))
+})
+
+test_that("renv.lock is not included for non-renv projects", {
+  withr::local_options(renv.verbose = FALSE)
+
+  app_dir <- local_temp_app(list(app.R = "library(foreign); library(MASS)"))
+
+  manifest <- makeManifest(app_dir)
+  expect_named(manifest$files, c("app.R"))
+})
+
 test_that("Rmd with reticulate as a dependency includes python in the manifest", {
   skip_on_cran()
   skip_if_not_installed("reticulate")