Merge pull request #103 from chainsawriot/fix102

Fix #102
gesistsa · Mar 2, 2023 · 34b2f96 · 34b2f96
2 parents 7ea374b + cf2b45b
commit 34b2f96
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 19 deletions.
diff --git a/R/pkgref.R b/R/pkgref.R
@@ -38,21 +38,25 @@
     }
     ## remove all @, ?, or # suffixes, we don't support them
     pkgref <- .clean_suffixes(pkgref)
-    res <- strsplit(pkgref, "::")[[1]]
-    if (length(res) == 1) {
-        source <- "cran"
-        handle <- res[1]
-    } else {
-        source <- res[1]
-        handle <- res[2]
-    }
+    res <- strsplit(pkgref, ":+")[[1]]
+    source <- res[1]
+    handle <- res[2]
     if (isTRUE(return_handle)) {
         return(handle)
     }
     return(source)
 }
 
+.is_local <- function(pkg) {
+    ## according to the standard, it must be started by ".", "~", "/"
+    grepl("^[\\.~/]", pkg)
+}
+
 .is_github <- function(pkg) {
+    ## make .is_local precedes .is_github
+    if (isTRUE(.is_local(pkg))) {
+        return(FALSE)
+    }
     if (grepl("github\\.com", pkg)) {
         return(TRUE)
     }
@@ -69,16 +73,10 @@
     pkg %in% bioc_pkgs$Package
 }
 
-
-.is_local <- function(pkg) {
-    ## according to the standard, it must be started by ".", "~", "/"
-    grepl("^[\\.~/]", pkg)
-}
-
 ## TBI: .is_valid_pkgref
 ## pkgref is only valid if: exactly one "::", source %in% c("cran", "github"), if "github", .is_github is TRUE
 .is_pkgref <- function(pkg) {
-    grepl("^github::|^cran::|^local::|^bioc::", pkg)
+    grepl("^github::|^cran::|^local::|^bioc::", pkg) && length(strsplit(pkg, ":+")[[1]]) == 2
 }
 
 .extract_github_handle <- function(url) {
@@ -98,10 +96,8 @@
     if (pkg == "" || is.na(pkg)) {
         stop("Invalid `pkg`.", call. = FALSE)
     }
-    if (isTRUE(.is_github(pkg))) {
-        if (isTRUE(grepl("github\\.com", pkg))) {
-          pkg <- .extract_github_handle(pkg)
-        }
+    if (isTRUE(grepl("github\\.com", pkg))) {
+        pkg <- .extract_github_handle(pkg)
     }
     if (isTRUE(.is_pkgref(pkg))) {
         return(.clean_suffixes(pkg))

diff --git a/tests/testthat/test_pkgref.R b/tests/testthat/test_pkgref.R
@@ -141,8 +141,18 @@ test_that("as_pkgrefs directory", {
 
 ## .is_*
 
+test_that(".is_pkgref", {
+    expect_true(.is_pkgref("cran::rtoot"))
+    expect_false(.is_pkgref("cran::"))
+    expect_false(.is_pkgref("cran:::"))
+    expect_false(.is_pkgref("xran:::"))
+})
+
 test_that(".is_github", {
     expect_true(.is_github("cran/rtoot"))
+    expect_true(.is_github("https://github.com/cran/rtoot"))
+    expect_true(.is_github("https://www.github.com/cran/rtoot"))
+    expect_true(.is_github("git@github.com:r-lib/pak.git"))
     expect_false(.is_github("cran//rtoot"))
     expect_false(.is_github("~/hello"))
     expect_false(.is_github("./hello"))
@@ -169,3 +179,8 @@ test_that(".is_local", {
     expect_true(.is_local("/hello/world/"))
     expect_true(.is_local("../testdata/fakexml2"))
 })
+
+test_that(".is_local precedes .is_github", {
+    expect_false(.is_github("~/helloworld"))
+    expect_false(.is_github("./helloworld"))
+})
diff --git a/tests/testthat/test_resolve.R b/tests/testthat/test_resolve.R
@@ -342,3 +342,13 @@ test_that("dockerize local package as tarball", {
     x <- readLines(file.path(temp_dir, "rang.R"))
     expect_true(any(grepl("^## ## WARNING", x)))
 })
+
+test_that("issue 102 confusion between github and local pkgref", {
+    skip_if_offline()
+    skip_on_cran()
+    original_wd <- getwd()
+    setwd(normalizePath(file.path("../testdata")))
+    expect_error(suppressWarnings(graph <- resolve("local::./askpass", snapshot_date = "2023-01-01")), NA)
+    expect_equal(.parse_pkgref(unique(graph$ranglets[[1]]$original$x_pkgref), FALSE), "local")
+    setwd(original_wd)
+})