Add support for Artifactory archive types

[cfmack]

In RTFACT-17392, Artifactory has a bug dealing with how CRAN archives. While that is indeed a bug, I have found in 3.5.1 install.packages, varying implementation of REMOTES, and/or versions of packrat, each package attempt to solve these varying implementations differently. Some need to reference a "archive.rds". Others must have a package.rds while others sufficiently use a PACKAGE file.

The ask and the associated PR is allowing the loosening of these restrictions to be backwards compatible. Remotes has a similar bug and PR

[pyltime]

This has come up with a customer using Posit Connect in support ticket 90580.

[kevinushey]

We have a solution for this on the renv side; perhaps we could port the relevant code to Packrat? That code lives here: https://github.com/rstudio/renv/blob/029c4457c0c2846eafd9a1da8bf3f95daf49fe45/R/retrieve.R#L806-L860

[jimhester]

The recent version of Connect which bundles a newer version of packrat exposed us to this issue as well. It would be great to either adapt the code from the linked PR or the linked renv solution to support this.

In the short term I have patched the vendored version of packrat for our use, but it would be great to avoid this workaround and use the one bundled with Connect.

[aronatkins]

@jimhester - the implementation of downloadWithRetries() has changed since #584; do you have an alternate patch that you have been using? Would you be able to share it?

[aronatkins]

Related: recent versions of renv assume that Artifactory uses a CRAN-like repository layout, but that may not always be appropriate. rstudio/renv#1996

[jimhester]

Sure, here's the diff, it is based off of https://github.com/rstudio/packrat/tree/a71a5feebb4f4a9fe95c6421f30b5e51424b1863 that fixes the issue for our use. I unfortunately don't recall why I based it off of that particular commit.

--- packrat-a71a5feebb4f4a9fe95c6421f30b5e51424b1863/R/restore.R	2023-10-18 16:41:38
+++ packrat-nflx/R/restore.R	2023-11-17 10:33:09
@@ -199,10 +199,16 @@
       # each named repository.
       foundVersion <- FALSE
       for (repo in repos) {
+        for (archiveUrl in c(
+          file.path(repo, "src/contrib/Archive",
+            pkgRecord$name,
+            pkgSrcFile),
+          file.path(repo, "src/contrib/Archive",
+            pkgRecord$name,
+            pkgRecord$version,
+            pkgSrcFile)
+          )) {
         tryCatch({
-          archiveUrl <- file.path(repo, "src/contrib/Archive",
-                                  pkgRecord$name,
-                                  pkgSrcFile)
           downloadWithRetries(archiveUrl,
                               destfile = file.path(pkgSrcDir, pkgSrcFile),
                               mode = "wb", quiet = TRUE)
@@ -212,7 +218,7 @@
         }, error = function(e) {
           # Ignore error and try the next repository
         })
-      }
+      }}
       if (!foundVersion) {
         message("FAILED")
         stopMsg <- sprintf("Failed to retrieve package sources for %s %s from CRAN (internet connectivity issue?)",

[aronatkins]

Thanks, @jimhester. I'm surprised that you don't need an additional break with the second loop. I'll push a PR shortly that's similar.

The commit you referenced is the one we pinned before switching to 7062580, which has some improvements when many restores happen concurrently.

[aronatkins]

This will probably do it: #730

I want to add some tests before merging, too.