Skip to content

Commit

Permalink
Merge pull request #39 from wlandau/review
Browse files Browse the repository at this point in the history
Align with upcoming review policy
  • Loading branch information
wlandau authored Nov 23, 2024
2 parents b4434ef + b5c91bb commit dfc9907
Show file tree
Hide file tree
Showing 30 changed files with 845 additions and 151 deletions.
2 changes: 2 additions & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ Authors@R: c(
Depends:
R (>= 3.6)
Imports:
base64enc,
desc,
gh,
igraph,
jsonlite,
Expand Down
3 changes: 3 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export(assert_cran_url)
export(assert_package)
export(assert_release_exists)
export(get_current_versions)
export(is_member_organization)
export(interpret_issue)
export(issues_checks)
export(issues_dependencies)
Expand All @@ -20,6 +21,8 @@ export(review_pull_request)
export(review_pull_requests)
export(try_message)
export(update_staging)
importFrom(base64enc,base64decode)
importFrom(desc,description)
importFrom(gh,gh)
importFrom(igraph,V)
importFrom(igraph,make_graph)
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# multiverse.internals 0.2.17

* Align contribution reviews with revised review policy in https://github.com/r-multiverse/r-multiverse.github.io/pull/33.

# multiverse.internals 0.2.16

* Add `interpret_issue()` to help create RSS feeds.
Expand Down
22 changes: 17 additions & 5 deletions R/aggregate_contributions.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ aggregate_contributions <- function(
FUN = read_package_listing,
owner_exceptions = owner_exceptions
)
message("Aggregating ", length(listings), " package listings.")
message("Aggregating positive package listings.")
aggregated <- do.call(what = vctrs::vec_rbind, args = listings)
message("Aggregated ", nrow(aggregated), " positive package listings.")
if (!file.exists(dirname(output))) {
dir.create(dirname(output))
}
Expand All @@ -38,13 +39,24 @@ aggregate_contributions <- function(
}

read_package_listing <- function(package, owner_exceptions) {
message("Processing package listing ", package)
name <- trimws(basename(package))
lines <- readLines(con = package, warn = FALSE)
json <- try(jsonlite::parse_json(lines), silent = TRUE)
if (inherits(json, "try-error")) {
url <- try(package_listing_url(name = name, url = lines), silent = TRUE)
if (inherits(json, "try-error") && inherits(url, "try-error")) {
message("Omitting package listing ", package, "(malformed or placeholder)")
return(
data.frame(
package = character(0L),
url = character(0L),
branch = character(0L)
)
)
} else if (inherits(json, "try-error")) {
message("URL package listing ", package)
json <- package_listing_url(name = name, url = lines)
} else {
message("JSON package listing ", package)
json <- package_listing_json(name = name, json = json)
}
decide_owner_exceptions(
Expand All @@ -54,7 +66,7 @@ read_package_listing <- function(package, owner_exceptions) {
}

package_listing_url <- function(name, url) {
message <- assert_package_lite(name = name, url = url)
message <- assert_package_listing(name = name, url = url)
if (!is.null(message)) {
stop(message, call. = FALSE)
}
Expand Down Expand Up @@ -107,7 +119,7 @@ package_listing_json <- function(name, json) {
)
json[[field]] <- trimws(json[[field]])
}
message <- assert_package_lite(name = json$package, url = json$url)
message <- assert_package_listing(name = json$package, url = json$url)
if (!is.null(message)) {
stop(message, call. = FALSE)
}
Expand Down
45 changes: 33 additions & 12 deletions R/assert_package.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@
#' otherwise `NULL` if there are no issues.
#' @param name Character of length 1, package name.
#' @param url Usually a character of length 1 with the package URL.
assert_package <- function(name, url) {
#' @param advisories Character vector of names of packages with advisories
#' in the R Consortium Advisory Database.
assert_package <- function(name, url, advisories = character(0L)) {
if (any(grepl(pattern = "\\}|\\{", x = url))) {
return(
paste("Entry of package", shQuote(name), "looks like custom JSON")
)
return(paste("Listing of package", shQuote(name), "looks like JSON"))
}
if (!is.null(out <- assert_package_lite(name = name, url = url))) {
if (!is.null(out <- assert_package_listing(name = name, url = url))) {
return(out)
}
name <- trimws(name)
url <- trimws(trim_trailing_slash(url))
if (!is.null(out <- assert_no_advisories(name, advisories = advisories))) {
return(out)
}
if (!is.null(out <- assert_package_lints(name = name, url = url))) {
return(out)
}
Expand All @@ -26,10 +29,15 @@ assert_package <- function(name, url) {
if (!is.null(out <- assert_cran_url(name = name, url = url))) {
return(out)
}
assert_release_exists(url = url)
if (!is.null(out <- assert_release_exists(url = url))) {
return(out)
}
if (!is.null(out <- assert_package_description(name = name, url = url))) {
return(out)
}
}

assert_package_lite <- function(name, url) {
assert_package_listing <- function(name, url) {
if (!is_package_name(name)) {
return("Invalid package name")
}
Expand All @@ -49,11 +57,17 @@ assert_package_lite <- function(name, url) {
}
}

is_package_name <- function(name) {
is_character_scalar(name) && grepl(
pattern = "^[a-zA-Z][a-zA-Z0-9.]*[a-zA-Z0-9]$",
x = trimws(name)
)
assert_no_advisories <- function(name, advisories) {
if (name %in% advisories) {
return(
paste(
"Package",
shQuote(name),
"has one or more advisories in the R Consortium Advisory Database",
"at https://github.com/RConsortium/r-advisory-database"
)
)
}
}

assert_package_lints <- function(name, url) {
Expand Down Expand Up @@ -101,3 +115,10 @@ assert_url_exists <- function(url) {
)
}
}

is_package_name <- function(name) {
is_character_scalar(name) && grepl(
pattern = "^[a-zA-Z][a-zA-Z0-9.]*[a-zA-Z0-9]$",
x = trimws(name)
)
}
169 changes: 169 additions & 0 deletions R/assert_package_description.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
#' @title Assert package `DESCRIPTION`
#' @keywords internal
#' @description Run basic assertions on the `DESCRIPTION` file of a package
#' on GitHub or GitLab.
#' @return A character string with an informative message if a problem
#' was found. Otherwise, `NULL` if there are no issues.
#' @param name Character string, name of the package listing contribution
#' to R-multiverse.
#' @param url Character string, URL of the package on GitHub or GitLab.
assert_package_description <- function(name, url) {
text <- try(get_repo_file(url, "DESCRIPTION"), silent = TRUE)
if (inherits(text, "try-error")) {
return(
paste(
"Could not read a DESCRIPTION file at the top-level of",
url,
sprintf("(error: %s)", conditionMessage(attr(text, "condition")))
)
)
}
if (!is.null(out <- assert_description_local(name, text))) {
return(out)
}
license <- desc::description$new(text = text)$get("License")
if (any(grepl("LICENSE", license))) {
path <- "LICENSE"
} else if (any(grepl("LICENCE", license))) {
path <- "LICENCE"
} else {
return()
}
text <- try(get_repo_file(url, path), silent = TRUE)
if (inherits(text, "try-error")) {
return(
paste(
"Could not download license file",
path,
"of package",
name,
sprintf("(error: %s)", conditionMessage(attr(text, "condition")))
)
)
}
if (!is.null(out <- assert_license_local(name, path, text))) {
return(out)
}
}

assert_description_local <- function(name, text) {
description <- try(
desc::description$new(text = text),
silent = TRUE
)
if (inherits(description, "try-error")) {
return(
paste(
"DESCRPTION file could not be parsed. Original error:",
conditionMessage(attr(description, "condition"))
)
)
}
assert_parsed_description(name, description)
}

assert_parsed_description <- function(name, description) {
if (!identical(name, as.character(description$get("Package")))) {
return(
paste(
"R-multiverse listing name",
shQuote(name),
"is different from the package name in the DESCRIPTION FILE:",
shQuote(description$get("Package"))
)
)
}
authors <- c(
description$get("Authors@R"),
description$get("Author"),
description$get("Maintainer")
)
if (!length(authors) || all(is.na(authors))) {
return(
paste(
"DESCRIPTION of package",
name,
"does not list an author or maintainer.",
"Each package contributed to R-multiverse must correctly attribute",
"authorship and ownership to protect the intellectual property",
"rights of the package owners."
)
)
}
license <- description$get("License")
if (!(license %in% trusted_licenses)) {
return(
paste(
"Detected license",
shQuote(license),
"which requires review by a moderator.",
"Each package contributed to R-multiverse must have a valid",
"open-source license to protect the intellectual property",
"rights of the package owners."
)
)
}
}

assert_license_local <- function(name, path, text) {
lines <- unlist(strsplit(text, split = "\n"))
keys <- trimws(gsub(":.*$", "", lines))
acceptable <- c("COPYRIGHT HOLDER", "ORGANISATION", "ORGANIZATION", "YEAR")
if (!all(keys %in% acceptable)) {
return(
paste(
"Package",
name,
"license file",
path,
"contains text more complicated than the usual",
"'YEAR', 'COPYRIGHT HOLDER', and 'ORGANIZATION' key-value pairs."
)
)
}
values <- trimws(gsub("^.*:", "", lines))
if (!all(nzchar(values))) {
return(
paste(
"Package",
name,
"license file",
path,
"contains colon-separated key-value pairs with empty values."
)
)
}
}

trusted_licenses <- c(
"Apache License (== 2.0)",
"Apache License (>= 2.0)",
"Artistic-2.0",
"Artistic License 2.0",
"BSD_2_clause + file LICENCE",
"BSD_3_clause + file LICENCE",
"BSD_2_clause + file LICENSE",
"BSD_3_clause + file LICENSE",
"GPL-2",
"GPL-3",
"GPL (== 2)",
"GPL (== 2)",
"GPL (== 2.0)",
"GPL (== 3)",
"GPL (== 3.0)",
"GPL (>= 2)",
"GPL (>= 2)",
"GPL (>= 2.0)",
"GPL (>= 3)",
"GPL (>= 3.0)",
"LGPL-2",
"LGPL-3",
"LGPL (== 2)",
"LGPL (== 2.1)",
"LGPL (== 3)",
"LGPL (>= 2)",
"LGPL (>= 2.1)",
"LGPL (>= 3)",
"MIT + file LICENCE",
"MIT + file LICENSE"
)
37 changes: 37 additions & 0 deletions R/get_repo_file.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#' @title Get repo file
#' @keywords internal
#' @description Get the contents of a file from a GitHub or GitLab repo.
#' @return A character string with the contents of the file.
#' @param url Character string, URL of a GitHub or GitLab repository.
#' @param path Character string, path to the file in the repository.
get_repo_file <- function(url, path) {
host <- url_parse(url)[["host"]]
if (host == "github.com") {
get_repo_file_github(url, path)
} else if (host == "gitlab.com") {
get_repo_file_gitlab(url, path)
}
}

get_repo_file_github <- function(url, path) {
response <- gh::gh(
"/repos/{owner}/{repo}/contents/{path}",
owner = basename(dirname(url)),
repo = basename(url),
path = path
)
rawToChar(base64enc::base64decode(response$content))
}

get_repo_file_gitlab <- function(url, path) {
query <- sprintf(
"https://gitlab.com/api/v4/projects/%s%s%s/repository/files/%s/raw",
basename(dirname(url)),
"%2F",
basename(url),
path
)
response <- nanonext::ncurl(url = query)
stopifnot(response$response == 200L)
response$data
}
14 changes: 14 additions & 0 deletions R/is_member_organization.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#' @title Check GitHub organization membership
#' @export
#' @keywords internal
#' @description Check if a GitHub user is a member of at least one
#' of the organizations given.
#' @return `TRUE` if the user is a member of at least one of the given
#' GitHub organizations, `FALSE` otherwise.
#' @param user Character string, GitHub user name.
#' @param organizations Character vector of names of GitHub organizations.
is_member_organization <- function(user, organizations) {
response <- gh::gh("/users/{user}/orgs", user = user)
membership <- unlist(lapply(response, function(entry) entry$login))
any(membership %in% organizations)
}
Loading

0 comments on commit dfc9907

Please sign in to comment.