From d86414e463c76748c748c2748bfb3acdf30c4fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 11:34:43 +0200 Subject: [PATCH 01/28] (min_isolatd): enforce Rcpp min version and select min version duplicate --- R/check.R | 102 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 36 deletions(-) diff --git a/R/check.R b/R/check.R index ce702fea..df1891bd 100644 --- a/R/check.R +++ b/R/check.R @@ -114,15 +114,69 @@ solve_ip.deps_installation_proposal <- function(ip) { resolve_ignoring_release_remote(ip) } -#' For each direct dependency, resolve that package using PPM snapshot as of release date + 1. -#' Finally, combine resolutions and run solve. +#' Resolve the dependencies of package based on the release data + 1 +#' #' @keywords internal #' @importFrom pkgcache ppm_repo_url #' @importFrom pkgdepends new_pkg_deps parse_pkg_ref +resolve_ppm_snapshot <- function(i_pkg, i_op, i_op_ver, i_ref_str) { + + i_ref <- pkgdepends::parse_pkg_ref(i_ref_str) + + i_ref_minver <- get_ref_min_incl_cran(i_ref, i_op, i_op_ver) + + i_release_date <- get_release_date(i_ref_minver) + + if (is.na(i_release_date)) { + ppm_repo <- file.path(pkgcache::ppm_repo_url(), "latest") + } else { + ppm_repo <- parse_ppm_url(get_ppm_snapshot_by_date(i_release_date)) + } + + i_pkg_deps <- pkgdepends::new_pkg_deps( + if (inherits(i_ref_minver, "remote_ref_github")) i_ref_minver$ref else i_ref$ref, + config = list(dependencies = "hard", cran_mirror = ppm_repo) + ) + suppressMessages(i_pkg_deps$resolve()) + + i_res <- i_pkg_deps$get_resolution() + i_res$direct <- i_res$directpkg <- FALSE + i_res$parent <- i_pkg + i_res +} + +#' Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 +#' A change in base R on 4.0.0 makes Rcpp incompatible in previous versions +#' @keywords internal +enforce_rcpp <- function(pkg_resolution) { + rcpp_index <- pkg_resolution$package == "Rcpp" + if (!any(rcpp_index)) return(NULL) + + version_lt_1 <- as.numeric_version(pkg_resolution$version) < as.numeric_version("1") & + rcpp_index + + if (NROW(pkg_resolution[version_lt_1,]) == 0) return(NULL) + + if (as.numeric_version(R.version$major) < as.numeric_version("4")) { + return(NULL) + } + + # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this + # hard requirement + rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0", "Rcpp") + pkg_resolution[version_lt_1,] <- rcpp_res + + pkg_resolution +} + +#' For each direct dependency, resolve that package using PPM snapshot as of release date + 1. +#' Finally, combine resolutions and run solve. +#' @keywords internal #' @exportS3Method solve_ip min_isolated_deps_installation_proposal solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint ip$resolve() res <- ip$get_resolution() + res$parent <- NA_character_ deps <- res[1, "deps"][[1]] ## copy op and version to Config\Needs\verdepcheck rows @@ -137,46 +191,22 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint cli_pb_init("min_isolated", total = nrow(deps)) - deps_res <- lapply( - seq_len(nrow(deps)), - function(i) { - i_pkg <- deps[i, "package"] - - cli_pb_update(package = i_pkg, n = 4L) - - if (i_pkg %in% base_pkgs()) { - return(NULL) - } + deps_res <- lapply(seq_len(nrow(deps)), function(i) { + i_pkg <- deps[i, "package"] - i_op <- deps[i, "op"] - i_op_ver <- deps[i, "version"] + cli_pb_update(package = i_pkg, n = 4L) - i_ref_str <- deps[i, "ref"] - i_ref <- pkgdepends::parse_pkg_ref(i_ref_str) - - i_ref_minver <- get_ref_min_incl_cran(i_ref, i_op, i_op_ver) - - i_release_date <- get_release_date(i_ref_minver) - - if (is.na(i_release_date)) { - ppm_repo <- file.path(pkgcache::ppm_repo_url(), "latest") - } else { - ppm_repo <- parse_ppm_url(get_ppm_snapshot_by_date(i_release_date)) - } - - i_pkg_deps <- pkgdepends::new_pkg_deps( - if (inherits(i_ref_minver, "remote_ref_github")) i_ref_minver$ref else i_ref$ref, - config = list(dependencies = "hard", cran_mirror = ppm_repo) - ) - suppressMessages(i_pkg_deps$resolve()) - i_res <- i_pkg_deps$get_resolution() - i_res$direct <- i_res$directpkg <- FALSE - i_res + if (i_pkg %in% base_pkgs()) { + return(NULL) } - ) + + resolve_ppm_snapshot(i_pkg, deps[i, "op"], deps[i, "version"], deps[i, "ref"]) + }) new_res <- rbind(res[1, ], do.call(rbind, deps_res)) + new_res <- new_res[order(as.numeric_version(new_res$version)),] new_res <- new_res[!duplicated(new_res), ] + new_res <- enforce_rcpp(new_res) ip$.__enclos_env__$private$plan$.__enclos_env__$private$resolution$result <- new_res ip$solve() From aad9662d9879652f9ee9eab0ee7afd9723cd815f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 11:49:20 +0200 Subject: [PATCH 02/28] docs: typo --- R/check.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/check.R b/R/check.R index df1891bd..11618c5c 100644 --- a/R/check.R +++ b/R/check.R @@ -114,7 +114,7 @@ solve_ip.deps_installation_proposal <- function(ip) { resolve_ignoring_release_remote(ip) } -#' Resolve the dependencies of package based on the release data + 1 +#' Resolve the dependencies of package based on the release date + 1 #' #' @keywords internal #' @importFrom pkgcache ppm_repo_url From 261ed9b08ed0f3e8da69d434adf906ff2457703b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:20:15 +0200 Subject: [PATCH 03/28] fix: minor bugs and keeps only minimum version of packages --- R/check.R | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/R/check.R b/R/check.R index 11618c5c..46106f27 100644 --- a/R/check.R +++ b/R/check.R @@ -119,11 +119,11 @@ solve_ip.deps_installation_proposal <- function(ip) { #' @keywords internal #' @importFrom pkgcache ppm_repo_url #' @importFrom pkgdepends new_pkg_deps parse_pkg_ref -resolve_ppm_snapshot <- function(i_pkg, i_op, i_op_ver, i_ref_str) { +resolve_ppm_snapshot <- function(pkg_pkg, operator, pkg_version, pkg_ref_str) { - i_ref <- pkgdepends::parse_pkg_ref(i_ref_str) + i_ref <- pkgdepends::parse_pkg_ref(pkg_ref_str) - i_ref_minver <- get_ref_min_incl_cran(i_ref, i_op, i_op_ver) + i_ref_minver <- get_ref_min_incl_cran(i_ref, operator, pkg_version) i_release_date <- get_release_date(i_ref_minver) @@ -134,14 +134,18 @@ resolve_ppm_snapshot <- function(i_pkg, i_op, i_op_ver, i_ref_str) { } i_pkg_deps <- pkgdepends::new_pkg_deps( - if (inherits(i_ref_minver, "remote_ref_github")) i_ref_minver$ref else i_ref$ref, + ifelse( + inherits(i_ref_minver, "remote_ref_github"), + i_ref_minver$ref, + i_ref$ref + ), config = list(dependencies = "hard", cran_mirror = ppm_repo) ) suppressMessages(i_pkg_deps$resolve()) i_res <- i_pkg_deps$get_resolution() i_res$direct <- i_res$directpkg <- FALSE - i_res$parent <- i_pkg + i_res$parent <- pkg_pkg i_res } @@ -150,15 +154,15 @@ resolve_ppm_snapshot <- function(i_pkg, i_op, i_op_ver, i_ref_str) { #' @keywords internal enforce_rcpp <- function(pkg_resolution) { rcpp_index <- pkg_resolution$package == "Rcpp" - if (!any(rcpp_index)) return(NULL) + if (!any(rcpp_index)) return(pkg_resolution) version_lt_1 <- as.numeric_version(pkg_resolution$version) < as.numeric_version("1") & rcpp_index - if (NROW(pkg_resolution[version_lt_1,]) == 0) return(NULL) + if (NROW(pkg_resolution[version_lt_1,]) == 0) return(pkg_resolution) if (as.numeric_version(R.version$major) < as.numeric_version("4")) { - return(NULL) + return(pkg_resolution) } # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this @@ -200,12 +204,27 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint return(NULL) } - resolve_ppm_snapshot(i_pkg, deps[i, "op"], deps[i, "version"], deps[i, "ref"]) + resolve_ppm_snapshot( + i_pkg, + deps[i, "op"], + deps[i, "version"], + deps[i, "ref"]) }) - new_res <- rbind(res[1, ], do.call(rbind, deps_res)) - new_res <- new_res[order(as.numeric_version(new_res$version)),] - new_res <- new_res[!duplicated(new_res), ] + new_res <- do.call(rbind, deps_res) + # Order by package name, version number and mirror + # for reproducible resolution + order_index <- order( new_res$package, + as.numeric_version(new_res$version), + new_res$mirror + ) + new_res <- new_res[order_index,] + new_res <- new_res[!duplicated(new_res[,c("ref","package", "version")]), ] + + # Keep res at top + new_res <- rbind(res[1, ], new_res) + + # Enforce minimum Rcpp version for R > 4.0 new_res <- enforce_rcpp(new_res) ip$.__enclos_env__$private$plan$.__enclos_env__$private$resolution$result <- new_res From c7d2368828f510bdc0fb2eeb350ffe36c2cf412d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:30:05 +0200 Subject: [PATCH 04/28] fix: typo --- R/check.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/check.R b/R/check.R index 46106f27..54ddb545 100644 --- a/R/check.R +++ b/R/check.R @@ -219,7 +219,7 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint new_res$mirror ) new_res <- new_res[order_index,] - new_res <- new_res[!duplicated(new_res[,c("ref","package", "version")]), ] + new_res <- new_res[!duplicated(new_res[,c("ref","package")]), ] # Keep res at top new_res <- rbind(res[1, ], new_res) From 81dcef030ead48a57b8287e2fd31b7fc3d5457c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 13:42:34 +0200 Subject: [PATCH 05/28] remove duplicate packages with same version --- R/check.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/check.R b/R/check.R index 54ddb545..46106f27 100644 --- a/R/check.R +++ b/R/check.R @@ -219,7 +219,7 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint new_res$mirror ) new_res <- new_res[order_index,] - new_res <- new_res[!duplicated(new_res[,c("ref","package")]), ] + new_res <- new_res[!duplicated(new_res[,c("ref","package", "version")]), ] # Keep res at top new_res <- rbind(res[1, ], new_res) From 3edf4e600c8be9f0dccd0a67f8eae2ba9b019c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 16:10:37 +0200 Subject: [PATCH 06/28] fix: keep primary dependency version it was lost in favor of indirect dependencies, when they existed --- R/check.R | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/R/check.R b/R/check.R index 46106f27..150df756 100644 --- a/R/check.R +++ b/R/check.R @@ -119,7 +119,7 @@ solve_ip.deps_installation_proposal <- function(ip) { #' @keywords internal #' @importFrom pkgcache ppm_repo_url #' @importFrom pkgdepends new_pkg_deps parse_pkg_ref -resolve_ppm_snapshot <- function(pkg_pkg, operator, pkg_version, pkg_ref_str) { +resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { i_ref <- pkgdepends::parse_pkg_ref(pkg_ref_str) @@ -145,7 +145,7 @@ resolve_ppm_snapshot <- function(pkg_pkg, operator, pkg_version, pkg_ref_str) { i_res <- i_pkg_deps$get_resolution() i_res$direct <- i_res$directpkg <- FALSE - i_res$parent <- pkg_pkg + i_res$parent <- pkg_ref_str i_res } @@ -167,7 +167,7 @@ enforce_rcpp <- function(pkg_resolution) { # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this # hard requirement - rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0", "Rcpp") + rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0") pkg_resolution[version_lt_1,] <- rcpp_res pkg_resolution @@ -205,13 +205,13 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint } resolve_ppm_snapshot( - i_pkg, + deps[i, "ref"], deps[i, "op"], - deps[i, "version"], - deps[i, "ref"]) + deps[i, "version"] + ) }) - new_res <- do.call(rbind, deps_res) + new_res <- do.call(rbind, deps_res) # Order by package name, version number and mirror # for reproducible resolution order_index <- order( new_res$package, @@ -219,7 +219,14 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint new_res$mirror ) new_res <- new_res[order_index,] - new_res <- new_res[!duplicated(new_res[,c("ref","package", "version")]), ] + new_res <- new_res[!duplicated(new_res[,c("ref", "package", "version")]), ] + + # Force primary dependencies versions + new_res <- new_res[ + !(new_res$package %in% deps$package) | + new_res$package == new_res$parent + , + ] # Keep res at top new_res <- rbind(res[1, ], new_res) From 37b67484378d86bb954ac4a5110c224bc00889e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 16:19:40 +0200 Subject: [PATCH 07/28] docs: move function to utils --- R/check.R | 59 ------------------------------------------------------- R/utils.R | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/R/check.R b/R/check.R index 150df756..875bc536 100644 --- a/R/check.R +++ b/R/check.R @@ -114,65 +114,6 @@ solve_ip.deps_installation_proposal <- function(ip) { resolve_ignoring_release_remote(ip) } -#' Resolve the dependencies of package based on the release date + 1 -#' -#' @keywords internal -#' @importFrom pkgcache ppm_repo_url -#' @importFrom pkgdepends new_pkg_deps parse_pkg_ref -resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { - - i_ref <- pkgdepends::parse_pkg_ref(pkg_ref_str) - - i_ref_minver <- get_ref_min_incl_cran(i_ref, operator, pkg_version) - - i_release_date <- get_release_date(i_ref_minver) - - if (is.na(i_release_date)) { - ppm_repo <- file.path(pkgcache::ppm_repo_url(), "latest") - } else { - ppm_repo <- parse_ppm_url(get_ppm_snapshot_by_date(i_release_date)) - } - - i_pkg_deps <- pkgdepends::new_pkg_deps( - ifelse( - inherits(i_ref_minver, "remote_ref_github"), - i_ref_minver$ref, - i_ref$ref - ), - config = list(dependencies = "hard", cran_mirror = ppm_repo) - ) - suppressMessages(i_pkg_deps$resolve()) - - i_res <- i_pkg_deps$get_resolution() - i_res$direct <- i_res$directpkg <- FALSE - i_res$parent <- pkg_ref_str - i_res -} - -#' Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 -#' A change in base R on 4.0.0 makes Rcpp incompatible in previous versions -#' @keywords internal -enforce_rcpp <- function(pkg_resolution) { - rcpp_index <- pkg_resolution$package == "Rcpp" - if (!any(rcpp_index)) return(pkg_resolution) - - version_lt_1 <- as.numeric_version(pkg_resolution$version) < as.numeric_version("1") & - rcpp_index - - if (NROW(pkg_resolution[version_lt_1,]) == 0) return(pkg_resolution) - - if (as.numeric_version(R.version$major) < as.numeric_version("4")) { - return(pkg_resolution) - } - - # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this - # hard requirement - rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0") - pkg_resolution[version_lt_1,] <- rcpp_res - - pkg_resolution -} - #' For each direct dependency, resolve that package using PPM snapshot as of release date + 1. #' Finally, combine resolutions and run solve. #' @keywords internal diff --git a/R/utils.R b/R/utils.R index 9a76e4ef..647166ba 100644 --- a/R/utils.R +++ b/R/utils.R @@ -31,3 +31,62 @@ get_ppm_snapshot_by_date <- function(date) { parse_ppm_url <- function(snapshot) { file.path(pkgcache::ppm_repo_url(), snapshot) } + +#' Resolve the dependencies of package based on the release date + 1 +#' +#' @keywords internal +#' @importFrom pkgcache ppm_repo_url +#' @importFrom pkgdepends new_pkg_deps parse_pkg_ref +resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { + + i_ref <- pkgdepends::parse_pkg_ref(pkg_ref_str) + + i_ref_minver <- get_ref_min_incl_cran(i_ref, operator, pkg_version) + + i_release_date <- get_release_date(i_ref_minver) + + if (is.na(i_release_date)) { + ppm_repo <- file.path(pkgcache::ppm_repo_url(), "latest") + } else { + ppm_repo <- parse_ppm_url(get_ppm_snapshot_by_date(i_release_date)) + } + + i_pkg_deps <- pkgdepends::new_pkg_deps( + ifelse( + inherits(i_ref_minver, "remote_ref_github"), + i_ref_minver$ref, + i_ref$ref + ), + config = list(dependencies = "hard", cran_mirror = ppm_repo) + ) + suppressMessages(i_pkg_deps$resolve()) + + i_res <- i_pkg_deps$get_resolution() + i_res$direct <- i_res$directpkg <- FALSE + i_res$parent <- pkg_ref_str + i_res +} + +#' Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 +#' A change in base R on 4.0.0 makes Rcpp incompatible in previous versions +#' @keywords internal +enforce_rcpp <- function(pkg_resolution) { + rcpp_index <- pkg_resolution$package == "Rcpp" + if (!any(rcpp_index)) return(pkg_resolution) + + version_lt_1 <- as.numeric_version(pkg_resolution$version) < as.numeric_version("1") & + rcpp_index + + if (NROW(pkg_resolution[version_lt_1,]) == 0) return(pkg_resolution) + + if (as.numeric_version(R.version$major) < as.numeric_version("4")) { + return(pkg_resolution) + } + + # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this + # hard requirement + rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0") + pkg_resolution[version_lt_1,] <- rcpp_res + + pkg_resolution +} From 982138304c9d22cc4d518cd1408858ba0eec1069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 10 Jul 2023 17:57:03 +0200 Subject: [PATCH 08/28] fix: improve code on keeping primary dependency version --- R/check.R | 11 +++-------- man/enforce_rcpp.Rd | 14 ++++++++++++++ man/resolve_ppm_snapshot.Rd | 12 ++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 man/enforce_rcpp.Rd create mode 100644 man/resolve_ppm_snapshot.Rd diff --git a/R/check.R b/R/check.R index 875bc536..7e7f5bd5 100644 --- a/R/check.R +++ b/R/check.R @@ -145,11 +145,13 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint return(NULL) } - resolve_ppm_snapshot( + result <- resolve_ppm_snapshot( deps[i, "ref"], deps[i, "op"], deps[i, "version"] ) + # remove duplicate entries of current package + result[result$ref != deps[i, "ref"] | !duplicated(result$ref),] }) new_res <- do.call(rbind, deps_res) @@ -162,13 +164,6 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint new_res <- new_res[order_index,] new_res <- new_res[!duplicated(new_res[,c("ref", "package", "version")]), ] - # Force primary dependencies versions - new_res <- new_res[ - !(new_res$package %in% deps$package) | - new_res$package == new_res$parent - , - ] - # Keep res at top new_res <- rbind(res[1, ], new_res) diff --git a/man/enforce_rcpp.Rd b/man/enforce_rcpp.Rd new file mode 100644 index 00000000..912d03c3 --- /dev/null +++ b/man/enforce_rcpp.Rd @@ -0,0 +1,14 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{enforce_rcpp} +\alias{enforce_rcpp} +\title{Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 +A change in base R on 4.0.0 makes Rcpp incompatible in previous versions} +\usage{ +enforce_rcpp(pkg_resolution) +} +\description{ +Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 +A change in base R on 4.0.0 makes Rcpp incompatible in previous versions +} +\keyword{internal} diff --git a/man/resolve_ppm_snapshot.Rd b/man/resolve_ppm_snapshot.Rd new file mode 100644 index 00000000..11b8613c --- /dev/null +++ b/man/resolve_ppm_snapshot.Rd @@ -0,0 +1,12 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{resolve_ppm_snapshot} +\alias{resolve_ppm_snapshot} +\title{Resolve the dependencies of package based on the release date + 1} +\usage{ +resolve_ppm_snapshot(pkg_ref_str, operator, pkg_version) +} +\description{ +Resolve the dependencies of package based on the release date + 1 +} +\keyword{internal} From c9f98e581ce2aa791d5a876175c409998cfdbbee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 11 Jul 2023 10:29:09 +0200 Subject: [PATCH 09/28] fix: keep primary dependency min version discard any other resolution for that same dep --- R/check.R | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/R/check.R b/R/check.R index 7e7f5bd5..a9d4f854 100644 --- a/R/check.R +++ b/R/check.R @@ -141,16 +141,15 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint cli_pb_update(package = i_pkg, n = 4L) - if (i_pkg %in% base_pkgs()) { - return(NULL) - } + if (i_pkg %in% base_pkgs()) return(NULL) result <- resolve_ppm_snapshot( deps[i, "ref"], deps[i, "op"], deps[i, "version"] ) - # remove duplicate entries of current package + + # Remove duplicate entries of current package result[result$ref != deps[i, "ref"] | !duplicated(result$ref),] }) @@ -164,6 +163,14 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint new_res <- new_res[order_index,] new_res <- new_res[!duplicated(new_res[,c("ref", "package", "version")]), ] + # Keep primary dependencies versions (in case primary dependency is + # resolved in other dependencies) + new_res <- new_res[ + !(new_res$package %in% deps$package) | + new_res$package == new_res$parent + , + ] + # Keep res at top new_res <- rbind(res[1, ], new_res) From 4352850a77e8d0d21b59e0e5ada6e4892ea7c9d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 11 Jul 2023 11:04:36 +0200 Subject: [PATCH 10/28] remove Rcpp enforment --- R/check.R | 14 ++------------ R/utils.R | 24 ------------------------ 2 files changed, 2 insertions(+), 36 deletions(-) diff --git a/R/check.R b/R/check.R index a9d4f854..1bb91f19 100644 --- a/R/check.R +++ b/R/check.R @@ -150,18 +150,11 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint ) # Remove duplicate entries of current package - result[result$ref != deps[i, "ref"] | !duplicated(result$ref),] + result[result$ref != deps[i, "ref"] | !duplicated(result$ref), ] }) new_res <- do.call(rbind, deps_res) - # Order by package name, version number and mirror - # for reproducible resolution - order_index <- order( new_res$package, - as.numeric_version(new_res$version), - new_res$mirror - ) - new_res <- new_res[order_index,] - new_res <- new_res[!duplicated(new_res[,c("ref", "package", "version")]), ] + new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] # Keep primary dependencies versions (in case primary dependency is # resolved in other dependencies) @@ -174,9 +167,6 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint # Keep res at top new_res <- rbind(res[1, ], new_res) - # Enforce minimum Rcpp version for R > 4.0 - new_res <- enforce_rcpp(new_res) - ip$.__enclos_env__$private$plan$.__enclos_env__$private$resolution$result <- new_res ip$solve() diff --git a/R/utils.R b/R/utils.R index 647166ba..9ce62245 100644 --- a/R/utils.R +++ b/R/utils.R @@ -66,27 +66,3 @@ resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { i_res$parent <- pkg_ref_str i_res } - -#' Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 -#' A change in base R on 4.0.0 makes Rcpp incompatible in previous versions -#' @keywords internal -enforce_rcpp <- function(pkg_resolution) { - rcpp_index <- pkg_resolution$package == "Rcpp" - if (!any(rcpp_index)) return(pkg_resolution) - - version_lt_1 <- as.numeric_version(pkg_resolution$version) < as.numeric_version("1") & - rcpp_index - - if (NROW(pkg_resolution[version_lt_1,]) == 0) return(pkg_resolution) - - if (as.numeric_version(R.version$major) < as.numeric_version("4")) { - return(pkg_resolution) - } - - # Resolve for Rcpp_1.0.0 to replace entries that don't comply with this - # hard requirement - rcpp_res <- resolve_ppm_snapshot("Rcpp", "==", "1.0.0") - pkg_resolution[version_lt_1,] <- rcpp_res - - pkg_resolution -} From d94085339a7ddf40a88fa662c7acf9679c9c7f5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 11 Jul 2023 16:12:04 +0200 Subject: [PATCH 11/28] revert minimal version enforcement --- R/check.R | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/R/check.R b/R/check.R index 1bb91f19..7d053502 100644 --- a/R/check.R +++ b/R/check.R @@ -143,26 +143,16 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint if (i_pkg %in% base_pkgs()) return(NULL) - result <- resolve_ppm_snapshot( + resolve_ppm_snapshot( deps[i, "ref"], deps[i, "op"], deps[i, "version"] ) - - # Remove duplicate entries of current package - result[result$ref != deps[i, "ref"] | !duplicated(result$ref), ] }) new_res <- do.call(rbind, deps_res) - new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] - # Keep primary dependencies versions (in case primary dependency is - # resolved in other dependencies) - new_res <- new_res[ - !(new_res$package %in% deps$package) | - new_res$package == new_res$parent - , - ] + new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] # Keep res at top new_res <- rbind(res[1, ], new_res) From 0579af9291dc12884cdf5fe695054470507179ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 11 Jul 2023 17:18:33 +0200 Subject: [PATCH 12/28] feat: shows error log from package building Uses pkgdepends format to show trace of errors with 'OE>' prefix --- R/check.R | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/R/check.R b/R/check.R index 7d053502..ad4666c3 100644 --- a/R/check.R +++ b/R/check.R @@ -272,7 +272,13 @@ download_ip <- function(ip) { #' @export install_ip <- function(ip) { ip$install_sysreqs() - ip$install() + tryCatch( + ip$install(), + error = function(err) { + # Print compilation error when installation fails to help debug + print(err) + stop(err) + }) return(invisible(ip)) } From c2d4b995ae90fcf977f40c8e03ab230c73b9f132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 12 Jul 2023 11:57:30 +0200 Subject: [PATCH 13/28] fix: account for multiple dates from gh query --- R/utils.R | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index 9ce62245..c8244ef8 100644 --- a/R/utils.R +++ b/R/utils.R @@ -22,7 +22,10 @@ base_pkgs <- function() { #' @importFrom pkgcache ppm_snapshots get_ppm_snapshot_by_date <- function(date) { snaps <- pkgcache::ppm_snapshots() - res <- as.character(as.Date(head(snaps[as.Date(snaps$date) > as.Date(date), "date"], 1))) + res <- as.character(as.Date(head( + snaps[as.Date(snaps$date) > max(as.Date(date), na.rm = TRUE), "date"], + 1 + ))) if (length(res) == 0) stop(sprintf("Cannot find PPM snapshot for date after %s.", as.character(date))) res } @@ -45,7 +48,7 @@ resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { i_release_date <- get_release_date(i_ref_minver) - if (is.na(i_release_date)) { + if (all(is.na(i_release_date))) { ppm_repo <- file.path(pkgcache::ppm_repo_url(), "latest") } else { ppm_repo <- parse_ppm_url(get_ppm_snapshot_by_date(i_release_date)) From 89f3b764f00a5dd6078503b09ba3e8d14cf0bbb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 12 Jul 2023 17:53:02 +0200 Subject: [PATCH 14/28] feat: supports github only dependencies --- R/get_ref.R | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/R/get_ref.R b/R/get_ref.R index 114ed06f..d99bb7cf 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -150,8 +150,19 @@ get_ref_min.remote_ref_github <- function(remote_ref, op = "", op_ver = "") { } } - new_ref <- sprintf("%s=%s/%s%s", remote_ref$package, remote_ref$username, remote_ref$repo, ref_suffix) # @TODO - pkgdepends::parse_pkg_ref(new_ref) + new_ref <- sprintf( + "%s=%s/%s%s", + remote_ref$package, + remote_ref$username, + remote_ref$repo, + ref_suffix + ) + result <- pkgdepends::parse_pkg_ref(new_ref) + + # Needs to restore original ref as it needs it to solve the pkg tree. With error: + # Can't install dependency (>= ) + result$ref <- remote_ref$ref + result } # Get list of releases if not empty else get list of tags From f65c8b3b39ab06763752b98e067e98c58880811a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Thu, 13 Jul 2023 12:05:09 +0200 Subject: [PATCH 15/28] docs: remove documentation for deleted function --- man/enforce_rcpp.Rd | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 man/enforce_rcpp.Rd diff --git a/man/enforce_rcpp.Rd b/man/enforce_rcpp.Rd deleted file mode 100644 index 912d03c3..00000000 --- a/man/enforce_rcpp.Rd +++ /dev/null @@ -1,14 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/utils.R -\name{enforce_rcpp} -\alias{enforce_rcpp} -\title{Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 -A change in base R on 4.0.0 makes Rcpp incompatible in previous versions} -\usage{ -enforce_rcpp(pkg_resolution) -} -\description{ -Enforce a minimum version of Rcpp (>=1.0.0) for R version above 4.0.0 -A change in base R on 4.0.0 makes Rcpp incompatible in previous versions -} -\keyword{internal} From e58166a6c0e26a5f9a04df7d8f14eaefb27e1642 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:43:15 +0200 Subject: [PATCH 16/28] fix: improve on graphql query and adds unit test using known past release tags of teal and rlang --- R/get_ref.R | 35 +++++++++++++++++++++++++++++------ R/utils.R | 2 +- tests/testthat/test-get_ref.R | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 tests/testthat/test-get_ref.R diff --git a/R/get_ref.R b/R/get_ref.R index d99bb7cf..03158139 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -174,6 +174,7 @@ get_gh_refs <- function(org, repo) { } get_gh_tags(org, repo) } + #' @importFrom gh gh_gql #' @keywords internal get_gh_releases <- function(org, repo, max_date = Sys.Date() + 1, min_date = as.Date("1900-01-01")) { @@ -198,6 +199,7 @@ get_gh_releases <- function(org, repo, max_date = Sys.Date() + 1, min_date = as. ) vapply(res, `[[`, character(1), "tagName") } + #' @importFrom gh gh_gql #' @keywords internal get_gh_tags <- function(org, repo, max_date = Sys.Date() + 1, min_date = as.Date("1900-01-01")) { @@ -329,25 +331,46 @@ get_release_date <- function(remote_ref) { } #' @importFrom gh gh_gql #' @export +#' @examples +#' remote_ref <- pkgdepends::parse_pkg_ref("insightsengineering/teal@v0.10.0") +#' get_release_date.remote_ref_github(remote_ref) get_release_date.remote_ref_github <- function(remote_ref) { gql_query <- sprintf("{ repository(owner: \"%s\", name: \"%s\") { refs(refPrefix: \"refs/tags/\", query: \"%s\", first: 100) { - nodes { - target { - ... on Commit { - committedDate + edges { + node { + name + target { + ... on Commit { + committedDate + } } - } + } } } } }", remote_ref$username, remote_ref$repo, remote_ref$commitish) + resp <- try(gh::gh_gql(gql_query), silent = TRUE) if (inherits(resp, "try-error")) { return(character(0)) } - vapply(resp$data$repository$refs$nodes, function(x) x$target$committedDate, character(1)) + + result <- vapply( + resp$data$repository$refs$edges, + function(x) { + if (x$node$name != remote_ref$commitish) return(NA_character_) + x$node$target$committedDate + }, + character(1) + ) + + if (length(result) <= 1) { + return(result %||% NA_character_) + } + + max(result, na.rm = TRUE) } #' @export get_release_date.remote_ref_cran <- function(remote_ref) { diff --git a/R/utils.R b/R/utils.R index c8244ef8..82feb75c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -23,7 +23,7 @@ base_pkgs <- function() { get_ppm_snapshot_by_date <- function(date) { snaps <- pkgcache::ppm_snapshots() res <- as.character(as.Date(head( - snaps[as.Date(snaps$date) > max(as.Date(date), na.rm = TRUE), "date"], + snaps[as.Date(snaps$date) > as.Date(date), "date"], 1 ))) if (length(res) == 0) stop(sprintf("Cannot find PPM snapshot for date after %s.", as.character(date))) diff --git a/tests/testthat/test-get_ref.R b/tests/testthat/test-get_ref.R new file mode 100644 index 00000000..d7cbf23f --- /dev/null +++ b/tests/testthat/test-get_ref.R @@ -0,0 +1,35 @@ +test_that("get_release_date.remote_ref_github will only retrieve 1 date for teal@v0.10.0", { + skip_if_offline() + skip_if_empty_gh_token() + + # Teal v0.10.0 has 2 tags (release candidate and release) + remote_ref <- pkgdepends::parse_pkg_ref("insightsengineering/teal@v0.10.0") + result <- get_release_date.remote_ref_github(remote_ref) + + expect_length(result, 1) + expect_identical(result, "2021-10-08T15:10:35Z") +}) + +test_that("get_release_date.remote_ref_github will only retrieve 1 date for rlang@1.0.0", { + skip_if_offline() + skip_if_empty_gh_token() + + # Teal v0.10.0 has 2 tags (release candidate and release) + remote_ref <- pkgdepends::parse_pkg_ref("r-lib/rlang@v1.0.0") + result <- get_release_date.remote_ref_github(remote_ref) + + expect_length(result, 1) + expect_identical(result, "2022-01-20T16:47:02Z") +}) + +test_that("get_release_date.remote_ref_github will only retrieve 1 date for rlang@0.0.0", { + skip_if_offline() + skip_if_empty_gh_token() + + # Teal v0.10.0 has 2 tags (release candidate and release) + remote_ref <- pkgdepends::parse_pkg_ref("r-lib/rlang@v0.0.0") + result <- get_release_date.remote_ref_github(remote_ref) + + expect_length(result, 1) + expect_true(is.na(result)) +}) From 33e7277f3d7e9808f3c983648c5776cfe3194e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:58:46 +0200 Subject: [PATCH 17/28] chore: adds documentation and compare dates that come from GH GraphQL instead of exact strings --- R/get_ref.R | 15 +++++++++++++++ man/get_release_date.remote_ref_cran.Rd | 18 ++++++++++++++++++ man/get_release_date.remote_ref_github.Rd | 18 ++++++++++++++++++ tests/testthat/test-get_ref.R | 4 ++-- 4 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 man/get_release_date.remote_ref_cran.Rd create mode 100644 man/get_release_date.remote_ref_github.Rd diff --git a/R/get_ref.R b/R/get_ref.R index 03158139..7c8d8c4a 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -329,6 +329,11 @@ cond_parse_pkg_ref_release <- function(remote_ref) { get_release_date <- function(remote_ref) { UseMethod("get_release_date", remote_ref) } + +#' Get release date from GitHub references +#' +#' @inheritParams get_release_date +#' #' @importFrom gh gh_gql #' @export #' @examples @@ -372,7 +377,15 @@ get_release_date.remote_ref_github <- function(remote_ref) { max(result, na.rm = TRUE) } + +#' Get release date from GitHub references +#' +#' @inheritParams get_release_date +#' #' @export +#' @examples +#' remote_ref <- pkgdepends::parse_pkg_ref("rlang@1.0.0") +#' get_release_date.remote_ref_cran(remote_ref) get_release_date.remote_ref_cran <- function(remote_ref) { subset( get_cran_data(remote_ref$package), @@ -380,10 +393,12 @@ get_release_date.remote_ref_cran <- function(remote_ref) { mtime )[[1]][1] } + #' @export get_release_date.remote_ref_standard <- function(remote_ref) { get_release_date.remote_ref_cran(remote_ref) } + #' @export get_release_date.remote_ref <- function(remote_ref) { NA diff --git a/man/get_release_date.remote_ref_cran.Rd b/man/get_release_date.remote_ref_cran.Rd new file mode 100644 index 00000000..7baed828 --- /dev/null +++ b/man/get_release_date.remote_ref_cran.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/get_ref.R +\name{get_release_date.remote_ref_cran} +\alias{get_release_date.remote_ref_cran} +\title{Get release date from GitHub references} +\usage{ +\method{get_release_date}{remote_ref_cran}(remote_ref) +} +\arguments{ +\item{remote_ref}{(\code{remote_ref}) object created with \code{\link[pkgdepends:parse_pkg_refs]{pkgdepends::parse_pkg_ref()}}} +} +\description{ +Get release date from GitHub references +} +\examples{ +remote_ref <- pkgdepends::parse_pkg_ref("rlang@1.0.0") +get_release_date.remote_ref_cran(remote_ref) +} diff --git a/man/get_release_date.remote_ref_github.Rd b/man/get_release_date.remote_ref_github.Rd new file mode 100644 index 00000000..fc0bb8dc --- /dev/null +++ b/man/get_release_date.remote_ref_github.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/get_ref.R +\name{get_release_date.remote_ref_github} +\alias{get_release_date.remote_ref_github} +\title{Get release date from GitHub references} +\usage{ +\method{get_release_date}{remote_ref_github}(remote_ref) +} +\arguments{ +\item{remote_ref}{(\code{remote_ref}) object created with \code{\link[pkgdepends:parse_pkg_refs]{pkgdepends::parse_pkg_ref()}}} +} +\description{ +Get release date from GitHub references +} +\examples{ +remote_ref <- pkgdepends::parse_pkg_ref("insightsengineering/teal@v0.10.0") +get_release_date.remote_ref_github(remote_ref) +} diff --git a/tests/testthat/test-get_ref.R b/tests/testthat/test-get_ref.R index d7cbf23f..d513894c 100644 --- a/tests/testthat/test-get_ref.R +++ b/tests/testthat/test-get_ref.R @@ -7,7 +7,7 @@ test_that("get_release_date.remote_ref_github will only retrieve 1 date for teal result <- get_release_date.remote_ref_github(remote_ref) expect_length(result, 1) - expect_identical(result, "2021-10-08T15:10:35Z") + expect_identical(as.Date(result), as.Date("2021-10-08T15:10:35Z")) }) test_that("get_release_date.remote_ref_github will only retrieve 1 date for rlang@1.0.0", { @@ -19,7 +19,7 @@ test_that("get_release_date.remote_ref_github will only retrieve 1 date for rlan result <- get_release_date.remote_ref_github(remote_ref) expect_length(result, 1) - expect_identical(result, "2022-01-20T16:47:02Z") + expect_identical(as.Date(result), as.Date("2022-01-20T16:47:02Z")) }) test_that("get_release_date.remote_ref_github will only retrieve 1 date for rlang@0.0.0", { From 846f5b0f0e847d7defbaaf55469d4181ef5878c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Fri, 14 Jul 2023 11:27:51 +0200 Subject: [PATCH 18/28] chore: adds if clause for examples --- R/get_ref.R | 4 ++-- man/get_release_date.remote_ref_cran.Rd | 2 ++ man/get_release_date.remote_ref_github.Rd | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/R/get_ref.R b/R/get_ref.R index 7c8d8c4a..ef30802a 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -336,7 +336,7 @@ get_release_date <- function(remote_ref) { #' #' @importFrom gh gh_gql #' @export -#' @examples +#' @examplesIf gh::gh_token() != "" #' remote_ref <- pkgdepends::parse_pkg_ref("insightsengineering/teal@v0.10.0") #' get_release_date.remote_ref_github(remote_ref) get_release_date.remote_ref_github <- function(remote_ref) { @@ -383,7 +383,7 @@ get_release_date.remote_ref_github <- function(remote_ref) { #' @inheritParams get_release_date #' #' @export -#' @examples +#' @examplesIf Sys.getenv("R_USER_CACHE_DIR", "") != "" #' remote_ref <- pkgdepends::parse_pkg_ref("rlang@1.0.0") #' get_release_date.remote_ref_cran(remote_ref) get_release_date.remote_ref_cran <- function(remote_ref) { diff --git a/man/get_release_date.remote_ref_cran.Rd b/man/get_release_date.remote_ref_cran.Rd index 7baed828..3cf70744 100644 --- a/man/get_release_date.remote_ref_cran.Rd +++ b/man/get_release_date.remote_ref_cran.Rd @@ -13,6 +13,8 @@ Get release date from GitHub references } \examples{ +\dontshow{if (Sys.getenv("R_USER_CACHE_DIR", "") != "") (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} remote_ref <- pkgdepends::parse_pkg_ref("rlang@1.0.0") get_release_date.remote_ref_cran(remote_ref) +\dontshow{\}) # examplesIf} } diff --git a/man/get_release_date.remote_ref_github.Rd b/man/get_release_date.remote_ref_github.Rd index fc0bb8dc..c7f47cd9 100644 --- a/man/get_release_date.remote_ref_github.Rd +++ b/man/get_release_date.remote_ref_github.Rd @@ -13,6 +13,8 @@ Get release date from GitHub references } \examples{ +\dontshow{if (gh::gh_token() != "") (if (getRversion() >= "3.4") withAutoprint else force)(\{ # examplesIf} remote_ref <- pkgdepends::parse_pkg_ref("insightsengineering/teal@v0.10.0") get_release_date.remote_ref_github(remote_ref) +\dontshow{\}) # examplesIf} } From a470d97cee8784bfee2d8082b5728c4e1429e572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:16:59 +0200 Subject: [PATCH 19/28] fix: solution for Remotes conflicts in version and key (ref) --- R/deps_installation_proposal.R | 56 +++++++++++++++++++++++++++++++++- R/get_ref.R | 3 -- man/desc_remotes_cleanup.Rd | 23 ++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 man/desc_remotes_cleanup.Rd diff --git a/R/deps_installation_proposal.R b/R/deps_installation_proposal.R index d57aeb37..b68b98d5 100644 --- a/R/deps_installation_proposal.R +++ b/R/deps_installation_proposal.R @@ -101,6 +101,7 @@ new_release_deps_installation_proposal <- function(path, # nolint } new_refs_str <- vapply(new_refs, `[[`, character(1), "ref") + d <- desc_remotes_cleanup(d, new_refs) d <- desc_cond_set_refs(d, new_refs_str) res <- desc_to_ip(d, config) @@ -260,6 +261,7 @@ new_min_isolated_deps_installation_proposal <- function(path, # nolint ) new_refs_str <- vapply(new_refs, `[[`, character(1), "ref") + d <- desc_remotes_cleanup(d, new_refs) d <- desc_cond_set_refs(d, new_refs_str) res <- desc_to_ip(d, config) @@ -299,6 +301,56 @@ get_refs_from_desc <- function(d) { res[res_idx] } +#' Replace Remotes in the `desc` that have been resolved to a GitHub tag or are +#' in CRAN +#' +#' Replaces any existing Remotes entry with the resolved GitHub tag from the +#' `new_refs`. +#' +#' It keeps all the existing Remotes that have not been resolved in `new_refs`. +#' +#' @param d (`desc`) DESCRIPTION object +#' @param new_refs (`list`) remote references that have been resolved and are +#' being updated in `Config/Needs/verdepcheck` +#' @keywords internal +desc_remotes_cleanup <- function(d, new_refs) { + # Parse the remotes to retrieve the package names + remotes <- pkgdepends::parse_pkg_refs(d$get_remotes()) + + # Get the packages defined in remotes + # (making sure that only packages that are already defined here are modified) + remotes_pkg <- vapply(remotes, `[[`, character(1), "package") + + # Find which packages of the new_refs are defined in Remotes + new_refs_gh <- vapply( + new_refs, + function(.x) { + isTRUE(.x$package %in% remotes_pkg) && inherits(.x, "remote_ref_github") + }, + logical(1) + ) + + # New remotes ref to use when replacing + new_ref_remote <- vapply(new_refs[new_refs_gh], `[[`, character(1), "ref") + + # Remotes that are not in new_refs are kept, as well as the ones that were + # resolved to be a github repo + d$clear_remotes() + + new_ref_pkg <- vapply(new_refs, `[[`, character(1), "package") + new_remotes <- c( + # Keep remotes + d$get_remotes()[!(remotes_pkg %in% new_ref_pkg)], + # Modified remotes + new_ref_remote + ) + + # Return clause without Remotes section + if (is.null(new_remotes) || length(new_remotes) == 0) return(d) + d$set_remotes(new_remotes) + d +} + #' Set `"Config/Needs/verdepcheck"` section into the `desc` object if not empty else clear this section. #' @keywords internal desc_cond_set_refs <- function(d, refs) { @@ -329,7 +381,9 @@ desc_to_ip <- function(d, config) { cli_pb_init <- function(type, total, ...) { cli::cli_progress_bar( format = paste( - "{cli::pb_spin} Resolving {cli::pb_extra$type} version of {cli::pb_extra$package}", + "{cli::pb_spin} Resolving", + "{cli::style_bold(cli::col_yellow(cli::pb_extra$type))}", + "version of {cli::col_blue(cli::pb_extra$package)}", "[{cli::pb_current}/{cli::pb_total}] ETA:{cli::pb_eta}" ), format_done = paste0( diff --git a/R/get_ref.R b/R/get_ref.R index ef30802a..72ddfe49 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -159,9 +159,6 @@ get_ref_min.remote_ref_github <- function(remote_ref, op = "", op_ver = "") { ) result <- pkgdepends::parse_pkg_ref(new_ref) - # Needs to restore original ref as it needs it to solve the pkg tree. With error: - # Can't install dependency (>= ) - result$ref <- remote_ref$ref result } diff --git a/man/desc_remotes_cleanup.Rd b/man/desc_remotes_cleanup.Rd new file mode 100644 index 00000000..c26608a2 --- /dev/null +++ b/man/desc_remotes_cleanup.Rd @@ -0,0 +1,23 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/deps_installation_proposal.R +\name{desc_remotes_cleanup} +\alias{desc_remotes_cleanup} +\title{Replace Remotes in the \code{desc} that have been resolved to a GitHub tag or are +in CRAN} +\usage{ +desc_remotes_cleanup(d, new_refs) +} +\arguments{ +\item{d}{(\code{desc}) DESCRIPTION object} + +\item{new_refs}{(\code{list}) remote references that have been resolved and are +being updated in \code{Config/Needs/verdepcheck}} +} +\description{ +Replaces any existing Remotes entry with the resolved GitHub tag from the +\code{new_refs}. +} +\details{ +It keeps all the existing Remotes that have not been resolved in \code{new_refs}. +} +\keyword{internal} From 9796b1238fcab7ab64bd8ce8bded91fb3dff3539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:36:11 +0200 Subject: [PATCH 20/28] fix: Applied Remotes cleanup to min_cohort too --- R/deps_installation_proposal.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/deps_installation_proposal.R b/R/deps_installation_proposal.R index b68b98d5..53cfb9c4 100644 --- a/R/deps_installation_proposal.R +++ b/R/deps_installation_proposal.R @@ -156,6 +156,7 @@ new_min_cohort_deps_installation_proposal <- function(path, # nolint } ) new_refs_str <- vapply(new_refs, `[[`, character(1), "ref") + d <- desc_remotes_cleanup(d, new_refs) d <- desc_cond_set_refs(d, new_refs_str) # find PPM snapshot From 5a3a6d9e544760136782eb0c0e25ebc7a62f5602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 18 Jul 2023 14:12:10 +0200 Subject: [PATCH 21/28] Adds verbose message to exception that is not self-descriptive --- R/check.R | 3 +-- R/get_ref.R | 9 ++++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/R/check.R b/R/check.R index ad4666c3..bcb52b1e 100644 --- a/R/check.R +++ b/R/check.R @@ -151,8 +151,7 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint }) new_res <- do.call(rbind, deps_res) - - new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] + new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version", "parent")]), ] # Keep res at top new_res <- rbind(res[1, ], new_res) diff --git a/R/get_ref.R b/R/get_ref.R index 72ddfe49..d52fb551 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -96,7 +96,14 @@ get_ref_min.remote_ref_cran <- function(remote_ref, op = "", op_ver = "") { min_ver <- Filter(function(x) x == min(pv), pv) new_ref <- sprintf("%s@%s", remote_ref$ref, names(min_ver)) # @TODO deparse, add ver, parse again - pkgdepends::parse_pkg_ref(new_ref) + tryCatch( + pkgdepends::parse_pkg_ref(new_ref), + error = function(err) { + cli::cli_alert_danger( + "Problem finding version for: `{remote_ref$package} ({op} {op_ver})`" + ) + stop(err) + }) } #' @rdname get_ref_min From 0f6b80eeb2ad0c9c5eae90979a94ac481f4face8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:59:26 +0200 Subject: [PATCH 22/28] Update R/get_ref.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com> --- R/get_ref.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/get_ref.R b/R/get_ref.R index d52fb551..c1924ac3 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -164,9 +164,7 @@ get_ref_min.remote_ref_github <- function(remote_ref, op = "", op_ver = "") { remote_ref$repo, ref_suffix ) - result <- pkgdepends::parse_pkg_ref(new_ref) - - result + pkgdepends::parse_pkg_ref(new_ref) } # Get list of releases if not empty else get list of tags From ac12fea4356ff4fbfa3417e1116d0781d4a65215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 18 Jul 2023 17:01:21 +0200 Subject: [PATCH 23/28] PR feedback: useless expression corrected, code/msg improvement Thanks for the comments @pawelru --- R/deps_installation_proposal.R | 21 ++++++++++++--------- R/get_ref.R | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/R/deps_installation_proposal.R b/R/deps_installation_proposal.R index 53cfb9c4..ffb113eb 100644 --- a/R/deps_installation_proposal.R +++ b/R/deps_installation_proposal.R @@ -323,29 +323,32 @@ desc_remotes_cleanup <- function(d, new_refs) { remotes_pkg <- vapply(remotes, `[[`, character(1), "package") # Find which packages of the new_refs are defined in Remotes - new_refs_gh <- vapply( - new_refs, + new_refs_remotes <- Filter( function(.x) { isTRUE(.x$package %in% remotes_pkg) && inherits(.x, "remote_ref_github") }, - logical(1) + new_refs ) # New remotes ref to use when replacing - new_ref_remote <- vapply(new_refs[new_refs_gh], `[[`, character(1), "ref") - - # Remotes that are not in new_refs are kept, as well as the ones that were - # resolved to be a github repo - d$clear_remotes() + new_ref_remote <- vapply(new_refs_remotes, `[[`, character(1), "ref") new_ref_pkg <- vapply(new_refs, `[[`, character(1), "package") + + # Remove from `Remotes` all package that have been resolved to + # * CRAN package + # * GitHub tag new_remotes <- c( - # Keep remotes + # Keep remotes (if the DESCRIPTION file is correct, this should have no elements) d$get_remotes()[!(remotes_pkg %in% new_ref_pkg)], # Modified remotes new_ref_remote ) + # Remotes that are not in new_refs are kept, as well as the ones that were + # resolved to be a github repo + d$clear_remotes() + # Return clause without Remotes section if (is.null(new_remotes) || length(new_remotes) == 0) return(d) d$set_remotes(new_remotes) diff --git a/R/get_ref.R b/R/get_ref.R index c1924ac3..ca073db2 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -100,7 +100,7 @@ get_ref_min.remote_ref_cran <- function(remote_ref, op = "", op_ver = "") { pkgdepends::parse_pkg_ref(new_ref), error = function(err) { cli::cli_alert_danger( - "Problem finding version for: `{remote_ref$package} ({op} {op_ver})`" + "Possible problem finding release for: `{remote_ref$package} ({op} {op_ver})`" ) stop(err) }) From 31ff4481bacb92b539884101b63bf5f794b19810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Tue, 18 Jul 2023 17:09:50 +0200 Subject: [PATCH 24/28] text: Improves on message --- R/get_ref.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/get_ref.R b/R/get_ref.R index ca073db2..8125045b 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -100,7 +100,12 @@ get_ref_min.remote_ref_cran <- function(remote_ref, op = "", op_ver = "") { pkgdepends::parse_pkg_ref(new_ref), error = function(err) { cli::cli_alert_danger( - "Possible problem finding release for: `{remote_ref$package} ({op} {op_ver})`" + paste( + sep = " ", + "Possible problem finding release for:", + "`{remote_ref$package} ({op} {op_ver})`.", + "The version might be invalid." + ) ) stop(err) }) From 537024235c2127293c303cee63cab67c242ad967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 19 Jul 2023 11:04:53 +0200 Subject: [PATCH 25/28] fix: Avoid identical calls to resolve_ppm_snapshot --- R/check.R | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/R/check.R b/R/check.R index bcb52b1e..4679cb14 100644 --- a/R/check.R +++ b/R/check.R @@ -134,6 +134,9 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint deps <- do.call(rbind, deps) deps <- deps[tolower(deps$type) %in% tolower(res[1, "dep_types"][[1]]), ] + # Avoid repeating calls to resolve_ppm_snapshot + deps <- deps[!duplicated(deps[, c("ref", "op", "version")]), ] + cli_pb_init("min_isolated", total = nrow(deps)) deps_res <- lapply(seq_len(nrow(deps)), function(i) { @@ -143,11 +146,7 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint if (i_pkg %in% base_pkgs()) return(NULL) - resolve_ppm_snapshot( - deps[i, "ref"], - deps[i, "op"], - deps[i, "version"] - ) + resolve_ppm_snapshot(deps[i, "ref"], deps[i, "op"], deps[i, "version"]) }) new_res <- do.call(rbind, deps_res) From 7ae7b7275fcf89b9c06e970c80c25c4c39e03ed3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:24:15 +0200 Subject: [PATCH 26/28] cleanup: remove parent column --- R/check.R | 3 +-- R/utils.R | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/R/check.R b/R/check.R index 4679cb14..6d220728 100644 --- a/R/check.R +++ b/R/check.R @@ -121,7 +121,6 @@ solve_ip.deps_installation_proposal <- function(ip) { solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint ip$resolve() res <- ip$get_resolution() - res$parent <- NA_character_ deps <- res[1, "deps"][[1]] ## copy op and version to Config\Needs\verdepcheck rows @@ -150,7 +149,7 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint }) new_res <- do.call(rbind, deps_res) - new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version", "parent")]), ] + new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] # Keep res at top new_res <- rbind(res[1, ], new_res) diff --git a/R/utils.R b/R/utils.R index 82feb75c..51178519 100644 --- a/R/utils.R +++ b/R/utils.R @@ -66,6 +66,5 @@ resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { i_res <- i_pkg_deps$get_resolution() i_res$direct <- i_res$directpkg <- FALSE - i_res$parent <- pkg_ref_str i_res } From 109db00c5a9b77fa990d6ab14f113508f630bb09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 19 Jul 2023 15:57:37 +0200 Subject: [PATCH 27/28] chore: replace tab with spaces --- R/get_ref.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/get_ref.R b/R/get_ref.R index 8125045b..9b784612 100644 --- a/R/get_ref.R +++ b/R/get_ref.R @@ -358,7 +358,7 @@ get_release_date.remote_ref_github <- function(remote_ref) { committedDate } } - } + } } } } From 047d3f2d6531630e13255c9b9936769c97b9d739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Thu, 20 Jul 2023 17:03:35 +0200 Subject: [PATCH 28/28] fix: keep only top version on resolution table to correct non-convergence issue --- R/check.R | 9 +++++++-- R/utils.R | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/R/check.R b/R/check.R index 6d220728..7f6909e8 100644 --- a/R/check.R +++ b/R/check.R @@ -149,10 +149,15 @@ solve_ip.min_isolated_deps_installation_proposal <- function(ip) { # nolint }) new_res <- do.call(rbind, deps_res) - new_res <- new_res[!duplicated(new_res[, c("ref", "package", "version")]), ] + + # Keep only top versions in calculated resolution (new_res). + # Very large resolution tables can become problematic and taking a long in reaching + # a solution. If + new_res <- new_res[order(new_res$ref, package_version(new_res$version), decreasing = TRUE), ] + new_res <- new_res[!duplicated(new_res[, c("ref")]), ] # Keep res at top - new_res <- rbind(res[1, ], new_res) + new_res <- rbind(res[1:2, ], new_res) ip$.__enclos_env__$private$plan$.__enclos_env__$private$resolution$result <- new_res ip$solve() diff --git a/R/utils.R b/R/utils.R index 51178519..f7c9ee01 100644 --- a/R/utils.R +++ b/R/utils.R @@ -60,7 +60,7 @@ resolve_ppm_snapshot <- function(pkg_ref_str, operator, pkg_version) { i_ref_minver$ref, i_ref$ref ), - config = list(dependencies = "hard", cran_mirror = ppm_repo) + config = list(dependencies = "hard", cran_mirror = ppm_repo, library = tempfile()) ) suppressMessages(i_pkg_deps$resolve())