Parse PACKAGES* with trailing newline correctly

.github/workflows/R-CMD-check.yaml

@@ -65,6 +65,8 @@ jobs:
           if (.Platform$OS.type == "windows") cat("CURL_SSL_BACKEND=openssl\n", file = Sys.getenv("GITHUB_ENV"), append = TRUE)
         shell: Rscript {0}
 
+      - uses: r-hub/actions/debug-shell@v1
+
       - uses: r-lib/actions/check-r-package@v2
         with:
           upload-snapshots: true

NEWS.md

@@ -1,5 +1,8 @@
 # pkgcache (development version)
 
+* `parse_packages()` now parses files ending with an extra newline
+  correctly (#122).
+
 # pkgcache 2.2.3
 
 * The metadata cache now does not use source URLs for packages in `Archive`

src/lib.c

@@ -361,7 +361,7 @@ SEXP pkgcache_parse_packages_raw(SEXP raw) {
   while (*p != '\0') {
     switch (state) {
 
-    /* -- at the begining of a package --------------------------------- */
+    /* -- at the beginning of a package -------------------------------- */
     case S_BG:
       if (*p == '\r') {
         p++;
@@ -479,6 +479,9 @@ SEXP pkgcache_parse_packages_raw(SEXP raw) {
   p = (char*) RAW(raw);
   p[len - 1] = tail;
   if (state == S_VL && tail != '\n') vlsize++;
+  /* if the tail is a \n, we don't need that. We also drop \r, which
+     is possibly not correct, but in practice better */
+  if (state == S_NL && (tail == '\n' || tail == '\r')) vlsize--;
 
   if (state == S_KW) {
     R_THROW_ERROR("PACKAGES file ended while parsing a key");

tests/async/test-cancel-early.R

@@ -9,7 +9,7 @@ test_that("auto-cancellation", {
 
     response_time <- async(function(url) {
       idx <<- idx + 1
-      httpx[[idx]] <<- http_head(url)
+      httpx[[idx]] <<- http_head(url, options = list(http_version = 2))
       httpx[[idx]]$
         then(function(x) { req_done <<- req_done + 1L ; x })$
         then(http_stop_for_status)$

tests/async/test-event-loop.R

@@ -52,7 +52,10 @@ test_that("repeated delay", {
   expect_null(error)
   expect_equal(result, 1:10)
   expect_true(end - start >= as.difftime(1, units = "secs"))
-  expect_true(end - start <= as.difftime(2, units = "secs"))
+  expect_true(end - start <= as.difftime(3, units = "secs"))
+  if (end - start > as.difftime(3, units = "secs")) {
+    message("Took ", end - start, " secs.")
+  }
 })
 
 test_that("nested event loops", {

tests/async/test-http.R

@@ -159,9 +159,9 @@ test_that("error, invalid arg", {
 test_that("automatic cancellation", {
   called <- 0L
   do <- function() {
-    r1 <- http_get(http$url("/delay/5"))$
+    r1 <- http_get(http$url("/delay/5"), options = list(http_version = 2))$
       then(function() called <<- called + 1L)
-    r2 <- http_get(http$url("/get"))$
+    r2 <- http_get(http$url("/get"), options = list(http_version = 2))$
       then(function() called <<- called + 1L)
     when_any(r1, r2)
   }

tests/test-async.R

@@ -1,5 +1,5 @@
 
-if (file.exists("async") && Sys.getenv("NOT_CRAN") == "true") {
+if (file.exists("async") && Sys.getenv("NOT_CRAN") == "true" && Sys.getenv("R_COVR") == "") {
   library(testthat)
   library(pkgcache)
   print(sessioninfo::package_info("pkgcache", dependencies = TRUE))

tests/testthat/_snaps/metadata-errors.md

@@ -4,5 +4,5 @@
       suppressMessages(get_private(cmc)$update_replica_rds())
     Condition
       Error in `parse_packages()`:
-      ! PACKAGES file ended while parsing a key @lib.c:484 (pkgcache_parse_packages_raw)
+      ! PACKAGES file ended while parsing a key @lib.c:487 (pkgcache_parse_packages_raw)
 

tests/testthat/test-1-metadata-cache-1.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("get_cache_files", {
   dir.create(pri <- fs::path_norm(tempfile()))
   on.exit(unlink(pri, recursive = TRUE), add = TRUE)

tests/testthat/test-1-metadata-cache-2.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("check_update", {
   setup_fake_apps()
 

tests/testthat/test-1-metadata-cache-3.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("concurrency in update", {
   setup_fake_apps()
 

tests/testthat/test-2-metadata-api.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("metadata api", {
 
   if (packageVersion("callr") < "3.1.0.9000") skip("Need newer callr")

tests/testthat/test-3-cran-metadata.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("what if cran.r-pkg.org is down?", {
   setup_fake_apps()
 

tests/testthat/test-archive.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("API", {
   setup_fake_apps()
   withr::local_options(

tests/testthat/test-installed.R

@@ -156,6 +156,27 @@ test_that("parse_packages empty file", {
   expect_snapshot(parse_packages(tmp))
 })
 
+# https://github.com/r-lib/pkgcache/issues/122
+test_that("parse_packages edge case (#122)", {
+  res <- list(Package = "foo")
+  do <- function(cnt) {
+    .Call(pkgcache_parse_packages_raw, charToRaw(cnt))
+  }
+  expect_equal(do("Package: foo"), res)
+  expect_equal(do("Package: foo\n"), res)
+  expect_equal(do("Package: foo\n\n"), res)
+  expect_equal(do("Package: foo\n\n\n"), res)
+  expect_equal(do("Package: foo\n\n\n\n"), res)
+  expect_equal(do("Package: foo\n\n\n\n\n"), res)
+  expect_equal(do("Package: foo\n\n\n\n\n\n"), res)
+
+  res2 <- list(Package = "foo\r")
+  expect_equal(do("Package: foo\r\n"), res2)
+  expect_equal(do("Package: foo\r\n\r\n"), res2)
+  expect_equal(do("Package: foo\r\n\r\n\r\n"), res2)
+  expect_equal(do("Package: foo\r\n\r\n\r\n\r\n"), res2)
+})
+
 test_that("parse_installed", {
   testthat::local_edition(3)
   testthat::local_reproducible_output(width = 60)

tests/testthat/test-metadata-utils.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("bioconductor$get_repos", {
   on.exit(bioconductor$.internal$clear_cache())
   setup_fake_apps()

tests/testthat/test-platform.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("current_r_platform_data", {
   mockery::stub(current_r_platform_data, "get_platform", "x86_64-apple-darwin17.0")
   expect_equal(current_r_platform_data()$platform, "x86_64-apple-darwin17.0")

tests/testthat/test-ppm.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("ppm_repo_url", {
   withr::local_envvar(
     PKGCACHE_PPM_URL = NA_character_,

tests/testthat/test-repo-set.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("repo_get", {
   withr::local_options(repos = character())
   rps <- repo_get()

tests/testthat/test-repo-status.R

@@ -1,4 +1,8 @@
 
+if (Sys.getenv("R_COVR") == "true") {
+  return()
+}
+
 test_that("repo_status", {
   setup_fake_apps()
   withr::local_options(