From 195c9806d78193cc88618cb562cdc435c94608fd Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 8 Nov 2024 15:38:32 +0000 Subject: [PATCH 01/24] Created initial diagnostics script with proxy checking function --- DESCRIPTION | 2 + NAMESPACE | 2 + R/diagnostic_test.R | 62 +++++++++++++++++++++++++++ man/check_proxy_settings.Rd | 25 +++++++++++ man/diagnostic_test.Rd | 19 ++++++++ man/pretty_num.Rd | 3 +- tests/testthat/test-diagnostic_test.R | 20 +++++++++ 7 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 R/diagnostic_test.R create mode 100644 man/check_proxy_settings.Rd create mode 100644 man/diagnostic_test.Rd create mode 100644 tests/testthat/test-diagnostic_test.R diff --git a/DESCRIPTION b/DESCRIPTION index 3f1d2d40..9f72adaf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -24,10 +24,12 @@ Depends: Imports: dplyr, emoji, + git2r, httr, jsonlite, lifecycle, magrittr, + purrr, renv, rlang, tidyselect, diff --git a/NAMESPACE b/NAMESPACE index 2c1339ea..c00793d6 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,7 +1,9 @@ # Generated by roxygen2: do not edit by hand +export(check_proxy_settings) export(comma_sep) export(create_project) +export(diagnostic_test) export(fetch_countries) export(fetch_lads) export(fetch_las) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R new file mode 100644 index 00000000..4bef3bd3 --- /dev/null +++ b/R/diagnostic_test.R @@ -0,0 +1,62 @@ +#' Diagnostic testing +#' +#' @inheritParams check_proxy_settings +#' +#' @return NULL +#' @export +#' +#' @examples +#' \dontrun{ +#' diagnostic_test() +#' } +diagnostic_test <- function(user_input = TRUE) { + check_proxy_settings(user_input = user_input) +} + +#' Check proxy settings +#' +#' @param proxy_setting_names Vector of proxy parameters to check for. Default: c("http.proxy", +#' "https.proxy") +#' @param user_input Ask for user choices on cleaning actions (TRUE / FALSE) +#' +#' @return NULL +#' @export +#' +#' @examples +#' \dontrun{ +#' check_proxy_settings() +#' } +check_proxy_settings <- function( + proxy_setting_names = c("http.proxy", "https.proxy"), + user_input = FALSE) { + git_config <- git2r::config() + proxy_config <- git_config |> + magrittr::extract2("global") |> + magrittr::extract(proxy_setting_names) + proxy_config <- purrr::keep(proxy_config, !is.na(names(proxy_config))) + if (any(!is.na(names(proxy_config)))) { + message("Found proxy settings:") + paste(names(proxy_config), "=", proxy_config, collapse = "\n") |> + message() + if (user_input) { + accept <- readline( + prompt = "Do you wish to remove the proxy settings (Y/n)? " + ) + } else { + accept <- TRUE + } + if (accept %in% c("Y", "y", "Yes", "yes", "YES", "TRUE", "")) { + proxy_args <- proxy_config |> + lapply(function(list) { + NULL + }) + rlang::inject(git2r::config(!!!proxy_args, global = TRUE)) + message("Git proxy settings have been cleared.") + } else { + message("Git proxy setting have been left in place.") + } + } else { + message("No proxy settings found in your Git configuration.") + } + return(proxy_config) +} diff --git a/man/check_proxy_settings.Rd b/man/check_proxy_settings.Rd new file mode 100644 index 00000000..f948d2b2 --- /dev/null +++ b/man/check_proxy_settings.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{check_proxy_settings} +\alias{check_proxy_settings} +\title{Check proxy settings} +\usage{ +check_proxy_settings( + proxy_setting_names = c("http.proxy", "https.proxy"), + user_input = FALSE +) +} +\arguments{ +\item{proxy_setting_names}{Vector of proxy parameters to check for. Default: c("http.proxy", +"https.proxy")} + +\item{user_input}{Ask for user choices on cleaning actions (TRUE / FALSE)} +} +\description{ +Check proxy settings +} +\examples{ +\dontrun{ +check_proxy_settings() +} +} diff --git a/man/diagnostic_test.Rd b/man/diagnostic_test.Rd new file mode 100644 index 00000000..385608f0 --- /dev/null +++ b/man/diagnostic_test.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{diagnostic_test} +\alias{diagnostic_test} +\title{Diagnostic testing} +\usage{ +diagnostic_test(user_input = TRUE) +} +\arguments{ +\item{user_input}{Ask for user choices on cleaning actions (TRUE / FALSE)} +} +\description{ +Diagnostic testing +} +\examples{ +\dontrun{ +diagnostic_test() +} +} diff --git a/man/pretty_num.Rd b/man/pretty_num.Rd index dde3e8fb..9ac0fdcb 100644 --- a/man/pretty_num.Rd +++ b/man/pretty_num.Rd @@ -34,7 +34,8 @@ converted and return original value} \item{nsmall}{minimum number of digits to the right of the decimal point. If NULL, the value of \code{dp} will be used. -If value of \code{dp} is less than 0, then \code{nsmall} will automatically be set to 0.} +If the value of \code{dp} is less than 0, then \code{nsmall} will +automatically be set to 0.} } \value{ string featuring prettified value diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R new file mode 100644 index 00000000..380b3be5 --- /dev/null +++ b/tests/testthat/test-diagnostic_test.R @@ -0,0 +1,20 @@ +test_that("Check proxy settings identifies and removes proxy setting", { + # Set a dummy config parameter for the purposes of testing + git2r::config(http.proxy.test = "this-is-a-test-entry", global = TRUE) + # Check that check_proxy_settings identifies the rogue entry + expect_equal( + check_proxy_settings( + proxy_setting_names = c("http.proxy.test", "https.proxy.test") + ) |> + suppressMessages(), + list(http.proxy.test = "this-is-a-test-entry") + ) + # Now double check that it cleared out the entry by running again... + expect_equal( + check_proxy_settings( + proxy_setting_names = c("http.proxy.test", "https.proxy.test") + ) |> + suppressMessages(), + purrr::keep(list(x = NULL), names(list(x = NULL)) != "x") + ) +}) From 60a349ed445a5c0f990cf3784454d9b6485526ba Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 8 Nov 2024 16:53:40 +0000 Subject: [PATCH 02/24] Added RENV_DOWNLOAD_METHOD diagnostic script --- R/diagnostic_test.R | 59 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 4bef3bd3..f5ed5739 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -60,3 +60,62 @@ check_proxy_settings <- function( } return(proxy_config) } + +#' Check renv download method +#' +#' @return +#' @export +#' +#' @examples +check_renv_download_method <- function(renviron_file = "~/.Renviron", user_input = TRUE) { + if (file.exists(renviron_file)) { + .renviron <- readLines(renviron_file) + } else { + .renviron <- c() + } + rdm_present <- .renviron %>% stringr::str_detect("RENV_DOWNLOAD_METHOD") + if (any(rdm_present)) { + message("Found RENV_DOWNLOAD_METHOD in .Renviron:") + message(" ", .renviron[rdm_present]) + detected_method <- .renviron[rdm_present] |> + stringr::str_split("=") |> + unlist() |> + magrittr::extract(2) + } else { + detected_method <- NA + } + if (is.na(detected_method) || detected_method != "\"curl\"") { + message("RENV_DOWNLOAD_METHOD is not set to curl. This may cause issues on DfE systems.") + message("==============================================================================") + if (user_input) { + accept <- readline(prompt = "Do you wish to set the RENV_DOWNLOAD_METHOD in .Renviron (Y/n)? ") + } else { + accept <- "n" + } + if (accept %in% c("", "Y", "y", "Yes", "YES", TRUE)) { + if (any(rdm_present)) { + .renviron <- .renviron[!rdm_present] + } + .renviron <- c( + .renviron, + "RENV_DOWNLOAD_METHOD=\"curl\"" + ) + print(.renviron) + cat(.renviron, file = renviron_file, sep = "\n") + message("The curl download method has automatically been set in your .Renviron file.") + readRenviron(renviron_file) + } else { + message("If you wish to manually update your .Renviron file, follow these steps:") + message(" - Run the following command in the R console to open the .Renviron file:") + message(" usethis::edit_r_environ()") + if (any(rdm_present)) { + message(" - Remove the following line from .Renviron:") + message(" ", .renviron[rdm_present]) + } + message(" - Add the following line to .Renviron:") + message(" RENV_DOWNLOAD_METHOD=\"curl\"") + } + } else { + message("Your RENV_DOWNLOAD_METHOD checks out as expected.") + } +} From f02301fc436caef8581575816d21a2503bc1ea73 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 14:41:14 +0000 Subject: [PATCH 03/24] Simplified diagnostic test workflows --- NAMESPACE | 1 + R/diagnostic_test.R | 107 ++++++++++++++------------ man/check_proxy_settings.Rd | 10 ++- man/check_renv_download_method.Rd | 29 +++++++ man/diagnostic_test.Rd | 16 +++- tests/testthat/test-diagnostic_test.R | 14 +++- 6 files changed, 121 insertions(+), 56 deletions(-) create mode 100644 man/check_renv_download_method.Rd diff --git a/NAMESPACE b/NAMESPACE index c00793d6..a0c0ee43 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,7 @@ # Generated by roxygen2: do not edit by hand export(check_proxy_settings) +export(check_renv_download_method) export(comma_sep) export(create_project) export(diagnostic_test) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index f5ed5739..d2967e16 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -1,25 +1,39 @@ #' Diagnostic testing #' +#' @description +#' Run a set of diagnostic tests to check for common issues found when setting up R on a DfE +#' system. This includes: +#' - Checking for proxy settings in the Git configuration +#' - Checking for correct download method used by renv (curl) +#' #' @inheritParams check_proxy_settings #' -#' @return NULL +#' @return List object of detected parameters #' @export #' #' @examples #' \dontrun{ #' diagnostic_test() #' } -diagnostic_test <- function(user_input = TRUE) { - check_proxy_settings(user_input = user_input) +diagnostic_test <- function( + clean = FALSE, + verbose = FALSE + ) { + results <- c( + check_proxy_settings(clean = clean, verbose = verbose), + check_renv_download_method(clean = clean, verbose = verbose) + ) + return(results) } #' Check proxy settings #' #' @param proxy_setting_names Vector of proxy parameters to check for. Default: c("http.proxy", #' "https.proxy") -#' @param user_input Ask for user choices on cleaning actions (TRUE / FALSE) +#' @param clean Attempt to clean settings +#' @param verbose Run in verbose mode #' -#' @return NULL +#' @return List of problem proxy settings #' @export #' #' @examples @@ -28,46 +42,47 @@ diagnostic_test <- function(user_input = TRUE) { #' } check_proxy_settings <- function( proxy_setting_names = c("http.proxy", "https.proxy"), - user_input = FALSE) { + clean = FALSE, + verbose = FALSE) { git_config <- git2r::config() proxy_config <- git_config |> magrittr::extract2("global") |> magrittr::extract(proxy_setting_names) proxy_config <- purrr::keep(proxy_config, !is.na(names(proxy_config))) if (any(!is.na(names(proxy_config)))) { - message("Found proxy settings:") + dfeR::toggle_message("Found proxy settings:", verbose = verbose) paste(names(proxy_config), "=", proxy_config, collapse = "\n") |> - message() - if (user_input) { - accept <- readline( - prompt = "Do you wish to remove the proxy settings (Y/n)? " - ) - } else { - accept <- TRUE - } - if (accept %in% c("Y", "y", "Yes", "yes", "YES", "TRUE", "")) { - proxy_args <- proxy_config |> - lapply(function(list) { - NULL - }) - rlang::inject(git2r::config(!!!proxy_args, global = TRUE)) - message("Git proxy settings have been cleared.") + toggle_message(verbose = verbose) + if (clean) { + proxy_args <- proxy_config |> + lapply(function(list) { + NULL + }) + rlang::inject(git2r::config(!!!proxy_args, global = TRUE)) + message("FIXED: Git proxy settings have been cleared.") } else { - message("Git proxy setting have been left in place.") + message("FAIL: Git proxy setting have been left in place.") } } else { - message("No proxy settings found in your Git configuration.") + message("PASS: No proxy settings found in your Git configuration.") } return(proxy_config) } #' Check renv download method #' -#' @return +#' @param renviron_file Location of .Renviron file. Default: ~/.Renviron +#' @inheritParams check_proxy_settings +#' +#' @return List object containing RENV_DOWNLOAD_METHOD #' @export #' #' @examples -check_renv_download_method <- function(renviron_file = "~/.Renviron", user_input = TRUE) { +#' check_renv_download_method() +check_renv_download_method <- function( + renviron_file = "~/.Renviron", + clean = FALSE, + verbose = FALSE) { if (file.exists(renviron_file)) { .renviron <- readLines(renviron_file) } else { @@ -75,8 +90,8 @@ check_renv_download_method <- function(renviron_file = "~/.Renviron", user_input } rdm_present <- .renviron %>% stringr::str_detect("RENV_DOWNLOAD_METHOD") if (any(rdm_present)) { - message("Found RENV_DOWNLOAD_METHOD in .Renviron:") - message(" ", .renviron[rdm_present]) + dfeR::toggle_message("Found RENV_DOWNLOAD_METHOD in .Renviron:", verbose = verbose) + dfeR::toggle_message(message(" ", .renviron[rdm_present]), verbose = verbose) detected_method <- .renviron[rdm_present] |> stringr::str_split("=") |> unlist() |> @@ -85,26 +100,19 @@ check_renv_download_method <- function(renviron_file = "~/.Renviron", user_input detected_method <- NA } if (is.na(detected_method) || detected_method != "\"curl\"") { - message("RENV_DOWNLOAD_METHOD is not set to curl. This may cause issues on DfE systems.") - message("==============================================================================") - if (user_input) { - accept <- readline(prompt = "Do you wish to set the RENV_DOWNLOAD_METHOD in .Renviron (Y/n)? ") - } else { - accept <- "n" - } - if (accept %in% c("", "Y", "y", "Yes", "YES", TRUE)) { - if (any(rdm_present)) { - .renviron <- .renviron[!rdm_present] - } - .renviron <- c( - .renviron, - "RENV_DOWNLOAD_METHOD=\"curl\"" - ) - print(.renviron) - cat(.renviron, file = renviron_file, sep = "\n") - message("The curl download method has automatically been set in your .Renviron file.") - readRenviron(renviron_file) - } else { + if (clean) { + if (any(rdm_present)) { + .renviron <- .renviron[!rdm_present] + } + .renviron <- c( + .renviron, + "RENV_DOWNLOAD_METHOD=\"curl\"" + ) + cat(.renviron, file = renviron_file, sep = "\n") + message("FIXED: The curl download method has automatically been set in your .Renviron file.") + readRenviron(renviron_file) + cleaned <- TRUE + } else { message("If you wish to manually update your .Renviron file, follow these steps:") message(" - Run the following command in the R console to open the .Renviron file:") message(" usethis::edit_r_environ()") @@ -116,6 +124,7 @@ check_renv_download_method <- function(renviron_file = "~/.Renviron", user_input message(" RENV_DOWNLOAD_METHOD=\"curl\"") } } else { - message("Your RENV_DOWNLOAD_METHOD checks out as expected.") + message("PASS: Your RENV_DOWNLOAD_METHOD is set to curl.") } + return(list(RENV_DOWNLOAD_METHOD = detected_method)) } diff --git a/man/check_proxy_settings.Rd b/man/check_proxy_settings.Rd index f948d2b2..0832160d 100644 --- a/man/check_proxy_settings.Rd +++ b/man/check_proxy_settings.Rd @@ -6,14 +6,20 @@ \usage{ check_proxy_settings( proxy_setting_names = c("http.proxy", "https.proxy"), - user_input = FALSE + clean = FALSE, + verbose = FALSE ) } \arguments{ \item{proxy_setting_names}{Vector of proxy parameters to check for. Default: c("http.proxy", "https.proxy")} -\item{user_input}{Ask for user choices on cleaning actions (TRUE / FALSE)} +\item{clean}{Attempt to clean settings} + +\item{verbose}{Run in verbose mode} +} +\value{ +List of problem proxy settings } \description{ Check proxy settings diff --git a/man/check_renv_download_method.Rd b/man/check_renv_download_method.Rd new file mode 100644 index 00000000..ad9a47cc --- /dev/null +++ b/man/check_renv_download_method.Rd @@ -0,0 +1,29 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{check_renv_download_method} +\alias{check_renv_download_method} +\title{Check renv download method} +\usage{ +check_renv_download_method( + renviron_file = "~/.Renviron", + user_input = TRUE, + clean = FALSE, + verbose = FALSE +) +} +\arguments{ +\item{renviron_file}{Location of .Renviron file. Default: ~/.Renviron} + +\item{clean}{Attempt to clean settings} + +\item{verbose}{Run in verbose mode} +} +\value{ +List object containing RENV_DOWNLOAD_METHOD +} +\description{ +Check renv download method +} +\examples{ +check_renv_download_method() +} diff --git a/man/diagnostic_test.Rd b/man/diagnostic_test.Rd index 385608f0..e261a023 100644 --- a/man/diagnostic_test.Rd +++ b/man/diagnostic_test.Rd @@ -4,13 +4,23 @@ \alias{diagnostic_test} \title{Diagnostic testing} \usage{ -diagnostic_test(user_input = TRUE) +diagnostic_test(clean = FALSE, verbose = FALSE) } \arguments{ -\item{user_input}{Ask for user choices on cleaning actions (TRUE / FALSE)} +\item{clean}{Attempt to clean settings} + +\item{verbose}{Run in verbose mode} +} +\value{ +List object of detected parameters } \description{ -Diagnostic testing +Run a set of diagnostic tests to check for common issues found when setting up R on a DfE +system. This includes: +\itemize{ +\item Checking for proxy settings in the Git configuration +\item Checking for correct download method used by renv (curl) +} } \examples{ \dontrun{ diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 380b3be5..2ed3b002 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -4,12 +4,22 @@ test_that("Check proxy settings identifies and removes proxy setting", { # Check that check_proxy_settings identifies the rogue entry expect_equal( check_proxy_settings( - proxy_setting_names = c("http.proxy.test", "https.proxy.test") + proxy_setting_names = c("http.proxy.test", "https.proxy.test"), + clean = FALSE + ) |> + suppressMessages(), + list(http.proxy.test = "this-is-a-test-entry") + ) + # Run the check in clean mode + expect_equal( + check_proxy_settings( + proxy_setting_names = c("http.proxy.test", "https.proxy.test"), + clean = TRUE ) |> suppressMessages(), list(http.proxy.test = "this-is-a-test-entry") ) - # Now double check that it cleared out the entry by running again... + # And now run again to see if clean mode worked in removing the rogue setting expect_equal( check_proxy_settings( proxy_setting_names = c("http.proxy.test", "https.proxy.test") From 3d23dfdd8def3b1c0ff0a5ad0c0122afdb2c6d84 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 14:49:30 +0000 Subject: [PATCH 04/24] Lintr and code styling --- R/diagnostic_test.R | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index d2967e16..4bd5e2dc 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -17,11 +17,10 @@ #' } diagnostic_test <- function( clean = FALSE, - verbose = FALSE - ) { + verbose = FALSE) { results <- c( - check_proxy_settings(clean = clean, verbose = verbose), - check_renv_download_method(clean = clean, verbose = verbose) + check_proxy_settings(clean = clean, verbose = verbose), + check_renv_download_method(clean = clean, verbose = verbose) ) return(results) } @@ -54,12 +53,12 @@ check_proxy_settings <- function( paste(names(proxy_config), "=", proxy_config, collapse = "\n") |> toggle_message(verbose = verbose) if (clean) { - proxy_args <- proxy_config |> - lapply(function(list) { - NULL - }) - rlang::inject(git2r::config(!!!proxy_args, global = TRUE)) - message("FIXED: Git proxy settings have been cleared.") + proxy_args <- proxy_config |> + lapply(function(list) { + NULL + }) + rlang::inject(git2r::config(!!!proxy_args, global = TRUE)) + message("FIXED: Git proxy settings have been cleared.") } else { message("FAIL: Git proxy setting have been left in place.") } @@ -101,18 +100,17 @@ check_renv_download_method <- function( } if (is.na(detected_method) || detected_method != "\"curl\"") { if (clean) { - if (any(rdm_present)) { - .renviron <- .renviron[!rdm_present] - } - .renviron <- c( - .renviron, - "RENV_DOWNLOAD_METHOD=\"curl\"" - ) - cat(.renviron, file = renviron_file, sep = "\n") - message("FIXED: The curl download method has automatically been set in your .Renviron file.") - readRenviron(renviron_file) - cleaned <- TRUE - } else { + if (any(rdm_present)) { + .renviron <- .renviron[!rdm_present] + } + .renviron <- c( + .renviron, + "RENV_DOWNLOAD_METHOD=\"curl\"" + ) + cat(.renviron, file = renviron_file, sep = "\n") + message("FIXED: The renv download method has been set to curl in your .Renviron file.") + readRenviron(renviron_file) + } else { message("If you wish to manually update your .Renviron file, follow these steps:") message(" - Run the following command in the R console to open the .Renviron file:") message(" usethis::edit_r_environ()") From e18e029395d5e347acecde744ca16a41204cc4a9 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 14:52:39 +0000 Subject: [PATCH 05/24] More lintr and code styling --- R/diagnostic_test.R | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 4bd5e2dc..0ac167ce 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -1,7 +1,8 @@ #' Diagnostic testing #' #' @description -#' Run a set of diagnostic tests to check for common issues found when setting up R on a DfE +#' Run a set of diagnostic tests to check for common issues found when setting +#' up R on a DfE #' system. This includes: #' - Checking for proxy settings in the Git configuration #' - Checking for correct download method used by renv (curl) @@ -27,8 +28,8 @@ diagnostic_test <- function( #' Check proxy settings #' -#' @param proxy_setting_names Vector of proxy parameters to check for. Default: c("http.proxy", -#' "https.proxy") +#' @param proxy_setting_names Vector of proxy parameters to check for. Default: +#' c("http.proxy", "https.proxy") #' @param clean Attempt to clean settings #' @param verbose Run in verbose mode #' @@ -89,8 +90,11 @@ check_renv_download_method <- function( } rdm_present <- .renviron %>% stringr::str_detect("RENV_DOWNLOAD_METHOD") if (any(rdm_present)) { - dfeR::toggle_message("Found RENV_DOWNLOAD_METHOD in .Renviron:", verbose = verbose) - dfeR::toggle_message(message(" ", .renviron[rdm_present]), verbose = verbose) + dfeR::toggle_message( + "Found RENV_DOWNLOAD_METHOD in .Renviron:", + verbose = verbose + ) + dfeR::toggle_message(" ", .renviron[rdm_present], verbose = verbose) detected_method <- .renviron[rdm_present] |> stringr::str_split("=") |> unlist() |> @@ -108,11 +112,16 @@ check_renv_download_method <- function( "RENV_DOWNLOAD_METHOD=\"curl\"" ) cat(.renviron, file = renviron_file, sep = "\n") - message("FIXED: The renv download method has been set to curl in your .Renviron file.") + message( + paste0( + "FIXED: The renv download method has been set to curl in your ", + ".Renviron file." + ) + ) readRenviron(renviron_file) } else { - message("If you wish to manually update your .Renviron file, follow these steps:") - message(" - Run the following command in the R console to open the .Renviron file:") + message("If you wish to manually update your .Renviron file:") + message(" - Run the command in the R console to open .Renviron:") message(" usethis::edit_r_environ()") if (any(rdm_present)) { message(" - Remove the following line from .Renviron:") From faa6a6e9c2dff9e05ee49ef31e5f5e779bd19899 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 15:43:59 +0000 Subject: [PATCH 06/24] Added some description to diagnostic scripts --- R/diagnostic_test.R | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 0ac167ce..73921b55 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -28,6 +28,13 @@ diagnostic_test <- function( #' Check proxy settings #' +#' @description +#' This script checks for "bad" proxy settings. Prior to the pandemic, analysts +#' in the DfE would need to add some proxy settings to their GitHub config. +#' These settings now prevent Git from connecting to remote archives on GitHub +#' and Azure DevOps if present, so this script identifies and (if clean=TRUE is +#' set) removes them. +#' #' @param proxy_setting_names Vector of proxy parameters to check for. Default: #' c("http.proxy", "https.proxy") #' @param clean Attempt to clean settings @@ -71,6 +78,12 @@ check_proxy_settings <- function( #' Check renv download method #' +#' @description +#' The renv package can retrieve packages either using curl or wininet, but +#' wininet doesn't work from within the DfE network. This script checks for +#' the parameter controlling which of these is used (RENV_DOWNLOAD_METHOD) and +#' sets it to use curl. +#' #' @param renviron_file Location of .Renviron file. Default: ~/.Renviron #' @inheritParams check_proxy_settings #' From f029e7674587c0d940627916ad9916047716638a Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 16:07:19 +0000 Subject: [PATCH 07/24] Added a test for the renv download method diagnostic check --- R/diagnostic_test.R | 12 ++++++++---- man/check_proxy_settings.Rd | 10 +++++++--- man/check_renv_download_method.Rd | 6 ++++-- man/diagnostic_test.Rd | 3 ++- tests/testthat/test-diagnostic_test.R | 12 ++++++++++++ 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 73921b55..eb84bf48 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -103,16 +103,16 @@ check_renv_download_method <- function( } rdm_present <- .renviron %>% stringr::str_detect("RENV_DOWNLOAD_METHOD") if (any(rdm_present)) { - dfeR::toggle_message( - "Found RENV_DOWNLOAD_METHOD in .Renviron:", - verbose = verbose + current_setting_message <- paste0( + "RENV_DOWNLOAD_METHOD is currently set to:\n ", + .renviron[rdm_present] ) - dfeR::toggle_message(" ", .renviron[rdm_present], verbose = verbose) detected_method <- .renviron[rdm_present] |> stringr::str_split("=") |> unlist() |> magrittr::extract(2) } else { + current_setting_message <- "RENV_DOWNLOAD_METHOD is not currently set." detected_method <- NA } if (is.na(detected_method) || detected_method != "\"curl\"") { @@ -133,6 +133,9 @@ check_renv_download_method <- function( ) readRenviron(renviron_file) } else { + dfeR::toggle_message(current_setting_message, + verbose = verbose + ) message("If you wish to manually update your .Renviron file:") message(" - Run the command in the R console to open .Renviron:") message(" usethis::edit_r_environ()") @@ -142,6 +145,7 @@ check_renv_download_method <- function( } message(" - Add the following line to .Renviron:") message(" RENV_DOWNLOAD_METHOD=\"curl\"") + message("Or run `dfeR::check_renv_download_method(clean=TRUE)`") } } else { message("PASS: Your RENV_DOWNLOAD_METHOD is set to curl.") diff --git a/man/check_proxy_settings.Rd b/man/check_proxy_settings.Rd index 0832160d..bcf49c2f 100644 --- a/man/check_proxy_settings.Rd +++ b/man/check_proxy_settings.Rd @@ -11,8 +11,8 @@ check_proxy_settings( ) } \arguments{ -\item{proxy_setting_names}{Vector of proxy parameters to check for. Default: c("http.proxy", -"https.proxy")} +\item{proxy_setting_names}{Vector of proxy parameters to check for. Default: +c("http.proxy", "https.proxy")} \item{clean}{Attempt to clean settings} @@ -22,7 +22,11 @@ check_proxy_settings( List of problem proxy settings } \description{ -Check proxy settings +This script checks for "bad" proxy settings. Prior to the pandemic, analysts +in the DfE would need to add some proxy settings to their GitHub config. +These settings now prevent Git from connecting to remote archives on GitHub +and Azure DevOps if present, so this script identifies and (if clean=TRUE is +set) removes them. } \examples{ \dontrun{ diff --git a/man/check_renv_download_method.Rd b/man/check_renv_download_method.Rd index ad9a47cc..5ec319f1 100644 --- a/man/check_renv_download_method.Rd +++ b/man/check_renv_download_method.Rd @@ -6,7 +6,6 @@ \usage{ check_renv_download_method( renviron_file = "~/.Renviron", - user_input = TRUE, clean = FALSE, verbose = FALSE ) @@ -22,7 +21,10 @@ check_renv_download_method( List object containing RENV_DOWNLOAD_METHOD } \description{ -Check renv download method +The renv package can retrieve packages either using curl or wininet, but +wininet doesn't work from within the DfE network. This script checks for +the parameter controlling which of these is used (RENV_DOWNLOAD_METHOD) and +sets it to use curl. } \examples{ check_renv_download_method() diff --git a/man/diagnostic_test.Rd b/man/diagnostic_test.Rd index e261a023..3bbf2fcc 100644 --- a/man/diagnostic_test.Rd +++ b/man/diagnostic_test.Rd @@ -15,7 +15,8 @@ diagnostic_test(clean = FALSE, verbose = FALSE) List object of detected parameters } \description{ -Run a set of diagnostic tests to check for common issues found when setting up R on a DfE +Run a set of diagnostic tests to check for common issues found when setting +up R on a DfE system. This includes: \itemize{ \item Checking for proxy settings in the Git configuration diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 2ed3b002..5e21c504 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -28,3 +28,15 @@ test_that("Check proxy settings identifies and removes proxy setting", { purrr::keep(list(x = NULL), names(list(x = NULL)) != "x") ) }) + +test_that("Check RENV_DOWNLOAD_METHOD", { + # Check that check_proxy_settings identifies the rogue entry + expect_equal( + check_renv_download_method( + ".Renviron_test", + clean = FALSE + ) |> + suppressMessages(), + list(RENV_DOWNLOAD_METHOD = NA) + ) +}) From 89a7d4ca79ea2aa92f5f420d39835d9339a7baf4 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Mon, 11 Nov 2024 16:19:33 +0000 Subject: [PATCH 08/24] Updating diagnostics documentation --- R/diagnostic_test.R | 4 +--- _pkgdown.yml | 7 +++++++ man/diagnostic_test.Rd | 2 -- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index eb84bf48..53ef2b4b 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -13,9 +13,7 @@ #' @export #' #' @examples -#' \dontrun{ #' diagnostic_test() -#' } diagnostic_test <- function( clean = FALSE, verbose = FALSE) { @@ -133,7 +131,7 @@ check_renv_download_method <- function( ) readRenviron(renviron_file) } else { - dfeR::toggle_message(current_setting_message, + dfeR::toggle_message(paste("FAIL:", current_setting_message), verbose = verbose ) message("If you wish to manually update your .Renviron file:") diff --git a/_pkgdown.yml b/_pkgdown.yml index 94448fea..47e7eff4 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -42,6 +42,13 @@ reference: contents: - starts_with("format_") +- title: System diagnostics + desc: Functions to diagnose and fix common system issues + contents: + - diagnostic_test + - check_proxy_settings + - check_renv_download_method + - title: Helpers desc: > Functions used in other functions across the package, exported as they can also diff --git a/man/diagnostic_test.Rd b/man/diagnostic_test.Rd index 3bbf2fcc..c75809e7 100644 --- a/man/diagnostic_test.Rd +++ b/man/diagnostic_test.Rd @@ -24,7 +24,5 @@ system. This includes: } } \examples{ -\dontrun{ diagnostic_test() } -} From 42e060025195d46943ac72a6c6fd65149af7c1db Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 13:59:02 +0000 Subject: [PATCH 09/24] Added GITHUB_PAT check to diagnostic tools --- R/diagnostic_test.R | 44 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 53ef2b4b..abdfcd78 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -19,6 +19,7 @@ diagnostic_test <- function( verbose = FALSE) { results <- c( check_proxy_settings(clean = clean, verbose = verbose), + check_github_pat(clean = clean, verbose = verbose), check_renv_download_method(clean = clean, verbose = verbose) ) return(results) @@ -42,9 +43,7 @@ diagnostic_test <- function( #' @export #' #' @examples -#' \dontrun{ #' check_proxy_settings() -#' } check_proxy_settings <- function( proxy_setting_names = c("http.proxy", "https.proxy"), clean = FALSE, @@ -74,6 +73,47 @@ check_proxy_settings <- function( return(proxy_config) } +#' Check GITHUB_PAT setting +#' +#' @description +#' If the GITHUB_PAT keyword is set, then it can cause issues with R installing +#' packages from GitHub (usually with an error of "ERROR [curl: (22) The +#' requested URL returned error: 401]"). This script checks whether the keyword +#' is set and can then clear it (if clear=TRUE). +#' The user will then need to identify where the "GITHUB_PAT" variable is being +#' set from and remove it to permanently fix the issue. +#' +#' @inheritParams check_proxy_settings +#' +#' @return List object containing the github_pat keyword content. +#' @export +#' +#' @examples +#' check_github_pat() +check_github_pat <- function(clean = FALSE, + verbose = FALSE) { + github_pat <- Sys.getenv("GITHUB_PAT") + if (github_pat != "") { + message( + "FAIL: GITHUB_PAT is set to ", + github_pat, + ". This may cause issues with installing packages from GitHub", + " such as dfeR and dfeshiny." + ) + if (clean) { + message("Clearing GITHUB_PAT keyword from system settings.") + Sys.unsetenv("GITHUB_PAT") + message( + "This issue may recur if you have some software that is", + "initialising the GITHUB_PAT keyword automatically." + ) + } + } else { + message("PASS: The GITHUB_PAT system variable is clear.") + } + return(list(GITHUB_PAT = github_pat)) +} + #' Check renv download method #' #' @description From 03f043781bc7d2e7a4b302084832427d04c3cdb1 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 14:22:04 +0000 Subject: [PATCH 10/24] Adapted proxy settings check output to be more consistent with rest of checks output --- R/diagnostic_test.R | 2 ++ tests/testthat/test-diagnostic_test.R | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index abdfcd78..e27f090e 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -69,6 +69,8 @@ check_proxy_settings <- function( } } else { message("PASS: No proxy settings found in your Git configuration.") + proxy_config <- as.list(rep("", length(proxy_setting_names))) |> + setNames(proxy_setting_names) } return(proxy_config) } diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 5e21c504..2295d058 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -1,10 +1,11 @@ test_that("Check proxy settings identifies and removes proxy setting", { # Set a dummy config parameter for the purposes of testing git2r::config(http.proxy.test = "this-is-a-test-entry", global = TRUE) + proxy_setting_names = c("http.proxy.test", "https.proxy.test") # Check that check_proxy_settings identifies the rogue entry expect_equal( check_proxy_settings( - proxy_setting_names = c("http.proxy.test", "https.proxy.test"), + proxy_setting_names = proxy_setting_names, clean = FALSE ) |> suppressMessages(), @@ -13,7 +14,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { # Run the check in clean mode expect_equal( check_proxy_settings( - proxy_setting_names = c("http.proxy.test", "https.proxy.test"), + proxy_setting_names = proxy_setting_names, clean = TRUE ) |> suppressMessages(), @@ -22,10 +23,22 @@ test_that("Check proxy settings identifies and removes proxy setting", { # And now run again to see if clean mode worked in removing the rogue setting expect_equal( check_proxy_settings( - proxy_setting_names = c("http.proxy.test", "https.proxy.test") + proxy_setting_names = proxy_setting_names ) |> suppressMessages(), - purrr::keep(list(x = NULL), names(list(x = NULL)) != "x") + proxy_config <- as.list(rep("", length(proxy_setting_names))) |> + setNames(proxy_setting_names) + ) +}) + +test_that("Test GITHUB_PAT diagnostic check", { + # Check that check_proxy_settings identifies the rogue entry + expect_equal( + check_github_pat( + clean = FALSE + ) |> + suppressMessages(), + list(GITHUB_PAT = "") ) }) From 0d1e2f2cba113c611c925945f8ce8508c5cac71a Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 15:24:52 +0000 Subject: [PATCH 11/24] Cleaning up and updating pkgdown with check github_pat --- NAMESPACE | 1 + R/diagnostic_test.R | 6 +++--- _pkgdown.yml | 1 + man/check_github_pat.Rd | 27 +++++++++++++++++++++++++++ man/check_proxy_settings.Rd | 2 -- tests/testthat/test-diagnostic_test.R | 4 ++-- 6 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 man/check_github_pat.Rd diff --git a/NAMESPACE b/NAMESPACE index a0c0ee43..087133ef 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,5 +1,6 @@ # Generated by roxygen2: do not edit by hand +export(check_github_pat) export(check_proxy_settings) export(check_renv_download_method) export(comma_sep) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index e27f090e..e46883ed 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -70,7 +70,7 @@ check_proxy_settings <- function( } else { message("PASS: No proxy settings found in your Git configuration.") proxy_config <- as.list(rep("", length(proxy_setting_names))) |> - setNames(proxy_setting_names) + stats::setNames(proxy_setting_names) } return(proxy_config) } @@ -79,8 +79,8 @@ check_proxy_settings <- function( #' #' @description #' If the GITHUB_PAT keyword is set, then it can cause issues with R installing -#' packages from GitHub (usually with an error of "ERROR [curl: (22) The -#' requested URL returned error: 401]"). This script checks whether the keyword +#' packages from GitHub (usually with an error of "ERROR \[curl: (22) The +#' requested URL returned error: 401\]"). This script checks whether the keyword #' is set and can then clear it (if clear=TRUE). #' The user will then need to identify where the "GITHUB_PAT" variable is being #' set from and remove it to permanently fix the issue. diff --git a/_pkgdown.yml b/_pkgdown.yml index 47e7eff4..d1831d11 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -47,6 +47,7 @@ reference: contents: - diagnostic_test - check_proxy_settings + - check_github_pat - check_renv_download_method - title: Helpers diff --git a/man/check_github_pat.Rd b/man/check_github_pat.Rd new file mode 100644 index 00000000..6390f1bf --- /dev/null +++ b/man/check_github_pat.Rd @@ -0,0 +1,27 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{check_github_pat} +\alias{check_github_pat} +\title{Check GITHUB_PAT setting} +\usage{ +check_github_pat(clean = FALSE, verbose = FALSE) +} +\arguments{ +\item{clean}{Attempt to clean settings} + +\item{verbose}{Run in verbose mode} +} +\value{ +List object containing the github_pat keyword content. +} +\description{ +If the GITHUB_PAT keyword is set, then it can cause issues with R installing +packages from GitHub (usually with an error of "ERROR [curl: (22) The +requested URL returned error: 401]"). This script checks whether the keyword +is set and can then clear it (if clear=TRUE). +The user will then need to identify where the "GITHUB_PAT" variable is being +set from and remove it to permanently fix the issue. +} +\examples{ +check_github_pat() +} diff --git a/man/check_proxy_settings.Rd b/man/check_proxy_settings.Rd index bcf49c2f..cc15de01 100644 --- a/man/check_proxy_settings.Rd +++ b/man/check_proxy_settings.Rd @@ -29,7 +29,5 @@ and Azure DevOps if present, so this script identifies and (if clean=TRUE is set) removes them. } \examples{ -\dontrun{ check_proxy_settings() } -} diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 2295d058..17ff89cc 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -1,7 +1,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { # Set a dummy config parameter for the purposes of testing git2r::config(http.proxy.test = "this-is-a-test-entry", global = TRUE) - proxy_setting_names = c("http.proxy.test", "https.proxy.test") + proxy_setting_names <- c("http.proxy.test", "https.proxy.test") # Check that check_proxy_settings identifies the rogue entry expect_equal( check_proxy_settings( @@ -27,7 +27,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { ) |> suppressMessages(), proxy_config <- as.list(rep("", length(proxy_setting_names))) |> - setNames(proxy_setting_names) + stats::setNames(proxy_setting_names) ) }) From 1853b75821622cd7b161aa0e69d478bb8703b25e Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 15:33:42 +0000 Subject: [PATCH 12/24] Accounting for GitHub Actions GITHUB_PAT system variable of *** --- R/diagnostic_test.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index e46883ed..23c246d9 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -94,7 +94,8 @@ check_proxy_settings <- function( #' check_github_pat() check_github_pat <- function(clean = FALSE, verbose = FALSE) { - github_pat <- Sys.getenv("GITHUB_PAT") + github_pat <- Sys.getenv("GITHUB_PAT") |> + stringr::str_replace_all("\\*", "") # Accounting for GitHub Actions "***" if (github_pat != "") { message( "FAIL: GITHUB_PAT is set to ", From 17ed957988e3c9618baf5a4b971fb6d91975e12e Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 15:55:16 +0000 Subject: [PATCH 13/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- tests/testthat/test-diagnostic_test.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 17ff89cc..fc0323c3 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -36,8 +36,7 @@ test_that("Test GITHUB_PAT diagnostic check", { expect_equal( check_github_pat( clean = FALSE - ) |> - suppressMessages(), + ), list(GITHUB_PAT = "") ) }) From f1c7beee809578d31f09ad9e0fcd6a3c26c3dead Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:05:27 +0000 Subject: [PATCH 14/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- R/diagnostic_test.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 23c246d9..9293b0e8 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -96,6 +96,9 @@ check_github_pat <- function(clean = FALSE, verbose = FALSE) { github_pat <- Sys.getenv("GITHUB_PAT") |> stringr::str_replace_all("\\*", "") # Accounting for GitHub Actions "***" + print(github_pat) + if(github_pat == "***"){print("Found *** GITHUB_PAT")} + cat("==================================") if (github_pat != "") { message( "FAIL: GITHUB_PAT is set to ", From 545fe68fb190ce37e88a379379c92615b013a256 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:13:18 +0000 Subject: [PATCH 15/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- R/diagnostic_test.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 9293b0e8..fdbbce92 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -95,7 +95,9 @@ check_proxy_settings <- function( check_github_pat <- function(clean = FALSE, verbose = FALSE) { github_pat <- Sys.getenv("GITHUB_PAT") |> - stringr::str_replace_all("\\*", "") # Accounting for GitHub Actions "***" + stringr::str_replace_all(stringr::regex("\\W+"), "") + # Replace above to remove non alphanumeric characters when run on GitHub + # Actions print(github_pat) if(github_pat == "***"){print("Found *** GITHUB_PAT")} cat("==================================") From 1970f0f3b957ad322c5cf47b90dd8f04bab727df Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:21:54 +0000 Subject: [PATCH 16/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- R/diagnostic_test.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index fdbbce92..51fbe105 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -99,6 +99,8 @@ check_github_pat <- function(clean = FALSE, # Replace above to remove non alphanumeric characters when run on GitHub # Actions print(github_pat) + print(pillar::type_sum(github_pat)) + print(typeof(github_pat)) if(github_pat == "***"){print("Found *** GITHUB_PAT")} cat("==================================") if (github_pat != "") { From 4fb6e61b6b845cd3f19de7da7641c1e3c0dc6aee Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:39:09 +0000 Subject: [PATCH 17/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- R/diagnostic_test.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 51fbe105..c6338154 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -98,6 +98,7 @@ check_github_pat <- function(clean = FALSE, stringr::str_replace_all(stringr::regex("\\W+"), "") # Replace above to remove non alphanumeric characters when run on GitHub # Actions + print(Sys.info()) print(github_pat) print(pillar::type_sum(github_pat)) print(typeof(github_pat)) From fd8d896c8d46b5c317b56f34546a108249c00220 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:49:25 +0000 Subject: [PATCH 18/24] Switching messages on for GITHUB_PAT check to help diagnose issue on GitHub Actions --- R/diagnostic_test.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index c6338154..5d11dfbf 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -94,7 +94,7 @@ check_proxy_settings <- function( #' check_github_pat() check_github_pat <- function(clean = FALSE, verbose = FALSE) { - github_pat <- Sys.getenv("GITHUB_PAT") |> + github_pat <- Sys.getenv("GITHUB_PAT", unset = NA) |> stringr::str_replace_all(stringr::regex("\\W+"), "") # Replace above to remove non alphanumeric characters when run on GitHub # Actions @@ -102,9 +102,8 @@ check_github_pat <- function(clean = FALSE, print(github_pat) print(pillar::type_sum(github_pat)) print(typeof(github_pat)) - if(github_pat == "***"){print("Found *** GITHUB_PAT")} cat("==================================") - if (github_pat != "") { + if (!is.na(github_pat)) { message( "FAIL: GITHUB_PAT is set to ", github_pat, @@ -121,6 +120,7 @@ check_github_pat <- function(clean = FALSE, } } else { message("PASS: The GITHUB_PAT system variable is clear.") + github_pat <- "" } return(list(GITHUB_PAT = github_pat)) } From 1ea5261d858c11b6bdfee10c4309a3c76a4a09b4 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 16:56:30 +0000 Subject: [PATCH 19/24] Removing GITHUB_PAT check test as GitHub actions won't return the environment variable to the test script --- R/diagnostic_test.R | 14 +++++--------- tests/testthat/test-diagnostic_test.R | 10 ---------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 5d11dfbf..18bd4052 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -92,16 +92,13 @@ check_proxy_settings <- function( #' #' @examples #' check_github_pat() -check_github_pat <- function(clean = FALSE, - verbose = FALSE) { - github_pat <- Sys.getenv("GITHUB_PAT", unset = NA) |> - stringr::str_replace_all(stringr::regex("\\W+"), "") +check_github_pat <- function( + clean = FALSE, + verbose = FALSE + ) { + github_pat <- Sys.getenv("GITHUB_PAT") # Replace above to remove non alphanumeric characters when run on GitHub # Actions - print(Sys.info()) - print(github_pat) - print(pillar::type_sum(github_pat)) - print(typeof(github_pat)) cat("==================================") if (!is.na(github_pat)) { message( @@ -120,7 +117,6 @@ check_github_pat <- function(clean = FALSE, } } else { message("PASS: The GITHUB_PAT system variable is clear.") - github_pat <- "" } return(list(GITHUB_PAT = github_pat)) } diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index fc0323c3..2e3dbc55 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -31,16 +31,6 @@ test_that("Check proxy settings identifies and removes proxy setting", { ) }) -test_that("Test GITHUB_PAT diagnostic check", { - # Check that check_proxy_settings identifies the rogue entry - expect_equal( - check_github_pat( - clean = FALSE - ), - list(GITHUB_PAT = "") - ) -}) - test_that("Check RENV_DOWNLOAD_METHOD", { # Check that check_proxy_settings identifies the rogue entry expect_equal( From ec631aaf878b767ac22fb23b5a9b9b1ff646e4ec Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Tue, 12 Nov 2024 17:05:27 +0000 Subject: [PATCH 20/24] Code styling --- R/diagnostic_test.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 18bd4052..c76ff192 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -94,8 +94,7 @@ check_proxy_settings <- function( #' check_github_pat() check_github_pat <- function( clean = FALSE, - verbose = FALSE - ) { + verbose = FALSE) { github_pat <- Sys.getenv("GITHUB_PAT") # Replace above to remove non alphanumeric characters when run on GitHub # Actions From 7b9095dd46d5fde2275e4a0eb44444f2edc6195b Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 3 Jan 2025 10:38:25 +0000 Subject: [PATCH 21/24] Expanded proxy settings check to include environment variables --- R/diagnostic_test.R | 53 +++++++++++++++++++++---- dfeR.Rproj | 1 + man/check_proxy_settings.Rd | 7 ++-- tests/testthat/test-diagnostic_test.R | 56 ++++++++++++++++++++++++--- 4 files changed, 101 insertions(+), 16 deletions(-) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index c76ff192..87515973 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -29,10 +29,11 @@ diagnostic_test <- function( #' #' @description #' This script checks for "bad" proxy settings. Prior to the pandemic, analysts -#' in the DfE would need to add some proxy settings to their GitHub config. +#' in the DfE would need to add some proxy settings to either their GitHub +#' config or environment variables. #' These settings now prevent Git from connecting to remote archives on GitHub -#' and Azure DevOps if present, so this script identifies and (if clean=TRUE is -#' set) removes them. +#' and Azure DevOps, so this script identifies and (if clean=TRUE is set) +#' removes them. #' #' @param proxy_setting_names Vector of proxy parameters to check for. Default: #' c("http.proxy", "https.proxy") @@ -48,15 +49,21 @@ check_proxy_settings <- function( proxy_setting_names = c("http.proxy", "https.proxy"), clean = FALSE, verbose = FALSE) { + proxy_settings_detected <- FALSE + # Check for proxy settings in the Git configuration git_config <- git2r::config() proxy_config <- git_config |> magrittr::extract2("global") |> magrittr::extract(proxy_setting_names) proxy_config <- purrr::keep(proxy_config, !is.na(names(proxy_config))) if (any(!is.na(names(proxy_config)))) { - dfeR::toggle_message("Found proxy settings:", verbose = verbose) + dfeR::toggle_message( + "Found proxy settings in Git config:", + verbose = verbose + ) paste(names(proxy_config), "=", proxy_config, collapse = "\n") |> toggle_message(verbose = verbose) + proxy_settings_detected <- TRUE if (clean) { proxy_args <- proxy_config |> lapply(function(list) { @@ -68,11 +75,41 @@ check_proxy_settings <- function( message("FAIL: Git proxy setting have been left in place.") } } else { - message("PASS: No proxy settings found in your Git configuration.") - proxy_config <- as.list(rep("", length(proxy_setting_names))) |> - stats::setNames(proxy_setting_names) + proxy_config <- NULL + } + # Check for proxy settings set in the system environment (dfeR version) + proxy_system <- Sys.getenv(proxy_setting_names) |> + as.list() + proxy_system <- purrr::keep(proxy_system, proxy_system != "") + if (any(proxy_system != "")) { + dfeR::toggle_message( + "Found proxy settings in system environment:", + verbose = verbose + ) + paste(names(proxy_system), "=", proxy_system, collapse = "\n") |> + toggle_message(verbose = verbose) + proxy_settings_detected <- TRUE + if (clean) { + proxy_args <- proxy_system |> + lapply(function(list) { + "" + }) + rlang::inject(Sys.setenv(!!!proxy_args)) + message("FIXED: System environment proxy settings have been cleared.") + } else { + message( + "FAIL: System environment proxy settings have been left in place." + ) + } + } else { + proxy_system <- NULL + } + if (!proxy_settings_detected) { + message( + "PASS: No proxy settings found in your Git config or system environment." + ) } - return(proxy_config) + return(list(git = proxy_config, system = proxy_system)) } #' Check GITHUB_PAT setting diff --git a/dfeR.Rproj b/dfeR.Rproj index 38b90112..cf196880 100644 --- a/dfeR.Rproj +++ b/dfeR.Rproj @@ -1,4 +1,5 @@ Version: 1.0 +ProjectId: 27106e85-ccc2-4843-b5cd-3833d61941bb RestoreWorkspace: No SaveWorkspace: No diff --git a/man/check_proxy_settings.Rd b/man/check_proxy_settings.Rd index cc15de01..4fbaa4c3 100644 --- a/man/check_proxy_settings.Rd +++ b/man/check_proxy_settings.Rd @@ -23,10 +23,11 @@ List of problem proxy settings } \description{ This script checks for "bad" proxy settings. Prior to the pandemic, analysts -in the DfE would need to add some proxy settings to their GitHub config. +in the DfE would need to add some proxy settings to either their GitHub +config or environment variables. These settings now prevent Git from connecting to remote archives on GitHub -and Azure DevOps if present, so this script identifies and (if clean=TRUE is -set) removes them. +and Azure DevOps, so this script identifies and (if clean=TRUE is set) +removes them. } \examples{ check_proxy_settings() diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 2e3dbc55..4b347dde 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -1,7 +1,45 @@ test_that("Check proxy settings identifies and removes proxy setting", { # Set a dummy config parameter for the purposes of testing - git2r::config(http.proxy.test = "this-is-a-test-entry", global = TRUE) proxy_setting_names <- c("http.proxy.test", "https.proxy.test") + # Check functionality with Git config + git2r::config(http.proxy.test = "this-is-a-test-entry", global = TRUE) + # Check that check_proxy_settings identifies the rogue entry + expect_equal( + check_proxy_settings( + proxy_setting_names = proxy_setting_names, + clean = FALSE + ) |> + suppressMessages(), + list( + git = list(http.proxy.test = "this-is-a-test-entry"), + system = NULL + ) + ) + # Run the check in clean mode + expect_equal( + check_proxy_settings( + proxy_setting_names = proxy_setting_names, + clean = TRUE + ) |> + suppressMessages(), + list( + git = list(http.proxy.test = "this-is-a-test-entry"), + system = NULL + ) + ) + # And now run again to see if clean mode worked in removing the rogue setting + expect_equal( + check_proxy_settings( + proxy_setting_names = proxy_setting_names + ) |> + suppressMessages(), + list( + git = NULL, + system = NULL + ) + ) + # Check functionality with system environment variables + Sys.setenv(http.proxy.test = "this-is-a-test-entry") # Check that check_proxy_settings identifies the rogue entry expect_equal( check_proxy_settings( @@ -9,7 +47,10 @@ test_that("Check proxy settings identifies and removes proxy setting", { clean = FALSE ) |> suppressMessages(), - list(http.proxy.test = "this-is-a-test-entry") + list( + git = NULL, + system = list(http.proxy.test = "this-is-a-test-entry") + ) ) # Run the check in clean mode expect_equal( @@ -18,7 +59,10 @@ test_that("Check proxy settings identifies and removes proxy setting", { clean = TRUE ) |> suppressMessages(), - list(http.proxy.test = "this-is-a-test-entry") + list( + git = NULL, + system = list(http.proxy.test = "this-is-a-test-entry") + ) ) # And now run again to see if clean mode worked in removing the rogue setting expect_equal( @@ -26,8 +70,10 @@ test_that("Check proxy settings identifies and removes proxy setting", { proxy_setting_names = proxy_setting_names ) |> suppressMessages(), - proxy_config <- as.list(rep("", length(proxy_setting_names))) |> - stats::setNames(proxy_setting_names) + list( + git = NULL, + system = NULL + ) ) }) From ebcb2d6194461aa31988094da42f9c3f00f08159 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 3 Jan 2025 11:05:39 +0000 Subject: [PATCH 22/24] Few bits of small clean-up on the proxy diagnostics --- DESCRIPTION | 1 - R/diagnostic_test.R | 11 ++++++----- tests/testthat/test-diagnostic_test.R | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 9f72adaf..3372ff1f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -29,7 +29,6 @@ Imports: jsonlite, lifecycle, magrittr, - purrr, renv, rlang, tidyselect, diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 87515973..05182650 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -55,7 +55,7 @@ check_proxy_settings <- function( proxy_config <- git_config |> magrittr::extract2("global") |> magrittr::extract(proxy_setting_names) - proxy_config <- purrr::keep(proxy_config, !is.na(names(proxy_config))) + proxy_config <- proxy_config[!is.na(names(proxy_config))] if (any(!is.na(names(proxy_config)))) { dfeR::toggle_message( "Found proxy settings in Git config:", @@ -78,9 +78,11 @@ check_proxy_settings <- function( proxy_config <- NULL } # Check for proxy settings set in the system environment (dfeR version) - proxy_system <- Sys.getenv(proxy_setting_names) |> + # The sys variables have a slightly different syntax with "_" rather than "." + proxy_sysvar_names <- gsub("\\.", "_", proxy_setting_names) + proxy_system <- Sys.getenv(proxy_sysvar_names) |> as.list() - proxy_system <- purrr::keep(proxy_system, proxy_system != "") + proxy_system <- proxy_system[proxy_system != ""] if (any(proxy_system != "")) { dfeR::toggle_message( "Found proxy settings in system environment:", @@ -135,8 +137,7 @@ check_github_pat <- function( github_pat <- Sys.getenv("GITHUB_PAT") # Replace above to remove non alphanumeric characters when run on GitHub # Actions - cat("==================================") - if (!is.na(github_pat)) { + if (github_pat != "") { message( "FAIL: GITHUB_PAT is set to ", github_pat, diff --git a/tests/testthat/test-diagnostic_test.R b/tests/testthat/test-diagnostic_test.R index 4b347dde..5c7ed2e1 100644 --- a/tests/testthat/test-diagnostic_test.R +++ b/tests/testthat/test-diagnostic_test.R @@ -39,7 +39,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { ) ) # Check functionality with system environment variables - Sys.setenv(http.proxy.test = "this-is-a-test-entry") + Sys.setenv(http_proxy_test = "this-is-a-test-entry") # Check that check_proxy_settings identifies the rogue entry expect_equal( check_proxy_settings( @@ -49,7 +49,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { suppressMessages(), list( git = NULL, - system = list(http.proxy.test = "this-is-a-test-entry") + system = list(http_proxy_test = "this-is-a-test-entry") ) ) # Run the check in clean mode @@ -61,7 +61,7 @@ test_that("Check proxy settings identifies and removes proxy setting", { suppressMessages(), list( git = NULL, - system = list(http.proxy.test = "this-is-a-test-entry") + system = list(http_proxy_test = "this-is-a-test-entry") ) ) # And now run again to see if clean mode worked in removing the rogue setting From 840a9f38ce9d5283f56e0ce5963e11b36cc21e4d Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 3 Jan 2025 12:42:19 +0000 Subject: [PATCH 23/24] Added sslverify check --- NAMESPACE | 1 + R/diagnostic_test.R | 60 ++++++++++++++++++++++++++++++++++++++ man/check_git_sslverify.Rd | 30 +++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 man/check_git_sslverify.Rd diff --git a/NAMESPACE b/NAMESPACE index 087133ef..6934131c 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,5 +1,6 @@ # Generated by roxygen2: do not edit by hand +export(check_git_sslverify) export(check_github_pat) export(check_proxy_settings) export(check_renv_download_method) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 05182650..36e4b724 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -19,6 +19,7 @@ diagnostic_test <- function( verbose = FALSE) { results <- c( check_proxy_settings(clean = clean, verbose = verbose), + check_git_sslverify(clean = clean, verbose = verbose), check_github_pat(clean = clean, verbose = verbose), check_renv_download_method(clean = clean, verbose = verbose) ) @@ -114,6 +115,65 @@ check_proxy_settings <- function( return(list(git = proxy_config, system = proxy_system)) } +#' Check Git sslverify setting +#' +#' @description +#' This script checks the values of the specified settings in the Git config and +#' if they're set to FALSE, it changes them to "TRUE" if clean=TRUE. +#' The defaut is to check for "http.sslVerify" and "https.sslVerify" +#' +#' @param ssl_verify_vars Vector of variables to check for in the Git config +#' @param clean Attempt to clean settings +#' @param verbose Run in verbose mode +#' +#' @return List of sslverify settings +#' @export +#' +#' @examples +#' check_git_sslverify() +check_git_sslverify <- function( + ssl_verify_vars = c("http.sslverify", "https.sslverify"), + clean = FALSE, + verbose = FALSE) { + # Check for proxy settings in the Git configuration + git_config <- git2r::config() |> + magrittr::extract2("global") |> + magrittr::extract(ssl_verify_vars) + git_config <- git_config[!is.na(names(git_config))] + if (any(!is.na(names(git_config)))) { + dfeR::toggle_message( + "Found specified settings in Git config:", + verbose = verbose + ) + paste(names(git_config), "=", git_config, collapse = "\n") |> + toggle_message(verbose = verbose) + if (any(tolower(git_config) == "false")) { + if (clean) { + git_args <- git_config[tolower(git_config) == "false"] |> + lapply(function(list) { + "TRUE" + }) + rlang::inject(git2r::config(!!!git_args, global = TRUE)) + message("FIXED: Specified Git settings have been set") + } else { + message("FAIL: sslverify is set to FALSE.") + message("Specified Git settings have been left in place.") + } + } else { + message( + "PASS: sslverify is set to TRUE." + ) + } + } else { + git_config <- NULL + message( + "PASS: sslverify is not explicitly set." + ) + } + return(list(ssl_verify=git_config)) +} + + #' Check GITHUB_PAT setting #' #' @description diff --git a/man/check_git_sslverify.Rd b/man/check_git_sslverify.Rd new file mode 100644 index 00000000..9340e6ae --- /dev/null +++ b/man/check_git_sslverify.Rd @@ -0,0 +1,30 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{check_git_sslverify} +\alias{check_git_sslverify} +\title{Check Git sslverify setting} +\usage{ +check_git_sslverify( + ssl_verify_vars = c("http.sslverify", "https.sslverify"), + clean = FALSE, + verbose = FALSE +) +} +\arguments{ +\item{ssl_verify_vars}{Vector of variables to check for in the Git config} + +\item{clean}{Attempt to clean settings} + +\item{verbose}{Run in verbose mode} +} +\value{ +List of sslverify settings +} +\description{ +This script checks the values of the specified settings in the Git config and +if they're set to FALSE, it changes them to "TRUE" if clean=TRUE. +The defaut is to check for "http.sslVerify" and "https.sslVerify" +} +\examples{ +check_git_sslverify() +} From 0f1557c228804fc67bf9744191207047726ea094 Mon Sep 17 00:00:00 2001 From: Rich Bielby Date: Fri, 3 Jan 2025 17:28:53 +0000 Subject: [PATCH 24/24] Added beginnings of function to check location of the Git global config file --- NAMESPACE | 1 + R/diagnostic_test.R | 30 +++++++++++++++++++++++++++++- man/check_gitconfig_location.Rd | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 man/check_gitconfig_location.Rd diff --git a/NAMESPACE b/NAMESPACE index 6934131c..472d3c38 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,7 @@ # Generated by roxygen2: do not edit by hand export(check_git_sslverify) +export(check_gitconfig_location) export(check_github_pat) export(check_proxy_settings) export(check_renv_download_method) diff --git a/R/diagnostic_test.R b/R/diagnostic_test.R index 36e4b724..0de92cb1 100644 --- a/R/diagnostic_test.R +++ b/R/diagnostic_test.R @@ -26,6 +26,34 @@ diagnostic_test <- function( return(results) } +#' Title +#' +#' @param clean +#' +#' @returns MULL +#' @export +#' +#' @examples +#' check_gitconfig_location() +check_gitconfig_location <- function( + clean = FALSE) { + config_files <- git2r:::git_config_files() + global_path <- config_files |> + dplyr::filter(file == "global") |> + dplyr::pull("path") + if (grepl("Users", global_path)) { + message( + "FAIL: Your global .gitconfig file is saved in a non-standard location." + ) + message("It may not be recognised and read in when using Git.") + message("We recommend using the standard location:") + message(" C:/Users//.gitconfig") + message("It contains the following settings:") + print(git2r::config() |> magrittr::extract2("global")) + } +} + + #' Check proxy settings #' #' @description @@ -170,7 +198,7 @@ check_git_sslverify <- function( "PASS: sslverify is not explicitly set." ) } - return(list(ssl_verify=git_config)) + return(list(ssl_verify = git_config)) } diff --git a/man/check_gitconfig_location.Rd b/man/check_gitconfig_location.Rd new file mode 100644 index 00000000..a7b5c36e --- /dev/null +++ b/man/check_gitconfig_location.Rd @@ -0,0 +1,14 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/diagnostic_test.R +\name{check_gitconfig_location} +\alias{check_gitconfig_location} +\title{Title} +\usage{ +check_gitconfig_location(clean = FALSE) +} +\arguments{ +\item{clean}{} +} +\description{ +Title +}