From 4d1f41b7689c04ef57918778ac94d33d55e0f4cb Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 11:22:01 -0700 Subject: [PATCH 01/21] refactor cargo-processx functions --- R/clean.R | 55 ++++++------- R/license_note.R | 167 ++++++++++++++++++++++++++-------------- R/read_cargo_metadata.R | 31 +++++--- R/use_crate.R | 45 ++++++----- 4 files changed, 176 insertions(+), 122 deletions(-) diff --git a/R/clean.R b/R/clean.R index d59c475a..a3e21864 100644 --- a/R/clean.R +++ b/R/clean.R @@ -5,8 +5,19 @@ #' (found by default at `pkg_root/src/rust/target/`). #' Useful when Rust code should be recompiled from scratch. #' @param path \[ string \] Path to the package root. +#' @param echo logical scalar, should cargo command and outputs be printed to +#' console (default is TRUE) +#' #' @export -clean <- function(path = ".") { +#' +#' @examples +#' \dontrun{ +#' clean() +#' } +clean <- function(path = ".", echo = TRUE) { + check_string(path, class = "rextendr_error") + check_bool(echo, class = "rextendr_error") + root <- rprojroot::find_package_root_file(path = path) rust_folder <- normalizePath( @@ -15,7 +26,7 @@ clean <- function(path = ".") { mustWork = FALSE ) - toml_path <- normalizePath( + manifest_path <- normalizePath( file.path(rust_folder, "Cargo.toml"), winslash = "/", mustWork = FALSE @@ -28,7 +39,7 @@ clean <- function(path = ".") { mustWork = FALSE ) - if (!file.exists(toml_path)) { + if (!file.exists(manifest_path)) { cli::cli_abort(c( "Unable to clean binaries.", "!" = "{.file Cargo.toml} not found in {.path {rust_folder}}.", @@ -36,44 +47,26 @@ clean <- function(path = ".") { )) } - cargo_envvars <- get_cargo_envvars() - args <- c( "clean", - glue("--manifest-path={toml_path}"), - glue("--target-dir={target_dir}"), + glue::glue("--manifest-path={manifest_path}"), + glue::glue("--target-dir={target_dir}"), if (tty_has_colors()) { "--color=always" } else { "--color=never" - }, - "--quiet" + } ) - exec_result <- processx::run( + + out <- processx::run( command = "cargo", args = args, - echo_cmd = FALSE, - windows_verbatim_args = FALSE, - stderr = "|", - stdout = "|", - error_on_status = FALSE, - env = cargo_envvars + error_on_status = TRUE, + wd = rust_folder, + echo_cmd = echo, + echo = echo, + env = get_cargo_envvars() ) - if (!isTRUE(exec_result$status == 0)) { - if (!tty_has_colors()) { - err_msg <- cli::ansi_strip(exec_result$stderr) - } else { - err_msg <- exec_result$stderr - } - cli::cli_abort( - c( - "Unable to execute {.code cargo clean}.", - "x" = paste(err_msg, collapse = "\n") - ), - call = caller_env(), - class = "rextendr_error" - ) - } pkgbuild::clean_dll(path = root) } diff --git a/R/license_note.R b/R/license_note.R index d3c19b00..e52da7f5 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -1,92 +1,141 @@ #' Generate LICENSE.note file. #' -#' LICENSE.note generated by this function contains information about Rust crate dependencies. -#' To use this function, the [cargo-license](https://crates.io/crates/cargo-license) command must be installed. -#' @param force Logical indicating whether to regenerate LICENSE.note if LICENSE.note already exists. -#' @inheritParams register_extendr +#' LICENSE.note generated by this function contains information about Rust crate +#' dependencies. To use this function, the +#' [cargo-license](https://crates.io/crates/cargo-license) command must be +#' installed. +#' +#' @param path character scalar, the R package directory +#' @param echo logical scalar, whether to print cargo command and outputs to the +#' console (default is `TRUE`) +#' @param quiet logical scalar, whether to signal successful writing of +#' LICENSE.note (default is `FALSE`) +#' @param force logical scalar, whether to regenerate LICENSE.note if +#' LICENSE.note already exists (default is `TRUE`) +#' #' @return No return value, called for side effects. +#' #' @export -write_license_note <- function(path = ".", quiet = FALSE, force = TRUE) { +#' +#' @examples +#' \dontrun{ +#' write_license_note() +#' } +write_license_note <- function( + path = ".", + echo = TRUE, + quiet = FALSE, + force = TRUE) { + check_string(path, class = "rextendr_error") + check_bool(echo, class = "rextendr_error") + check_bool(force, class = "rextendr_error") + if (!cargo_command_available(c("license", "--help"))) { cli::cli_abort( c( - "The {.code cargo license} command is required to run the {.fun write_license_note} function.", - "*" = "Please install cargo-license ({.url https://crates.io/crates/cargo-license}) first.", + "The {.code cargo license} command is required \\ + to run the {.fun write_license_note} function.", + "*" = "Please install cargo-license \\ + ({.url https://crates.io/crates/cargo-license}) first.", i = "Run {.code cargo install cargo-license} from your terminal." ), class = "rextendr_error" ) } - manifest_file <- rprojroot::find_package_root_file("src", "rust", "Cargo.toml", path = path) - outfile <- rprojroot::find_package_root_file("LICENSE.note", path = path) + rust_folder <- rprojroot::find_package_root_file( + "src", "rust", + path = path + ) + + manifest_path <- normalizePath( + file.path(rust_folder, "Cargo.toml"), + winslash = "/", + mustWork = FALSE + ) - if (!isTRUE(force) && file.exists(outfile)) { + outfile <- rprojroot::find_package_root_file( + "LICENSE.note", + path = path + ) + + if (isFALSE(force) && file.exists(outfile)) { cli::cli_abort( c( "LICENSE.note already exists.", - "If you want to regenerate LICENSE.note, set `force = TRUE` to {.fun write_license_note}." + "If you want to regenerate LICENSE.note, \\ + set `force = TRUE` to {.fun write_license_note}." ), class = "rextendr_error" ) } - list_license <- processx::run( - "cargo", - c( - "license", - "--authors", - "--json", - "--avoid-build-deps", - "--avoid-dev-deps", - "--manifest-path", manifest_file - ) - )$stdout %>% - jsonlite::parse_json() - - package_names <- processx::run( - "cargo", - c( - "metadata", - "--no-deps", - "--format-version", "1", - "--manifest-path", manifest_file - ) - )$stdout %>% - jsonlite::parse_json() %>% - purrr::pluck("packages") %>% - purrr::map_chr("name") + args <- c( + "license", + "--authors", + "--json", + "--avoid-build-deps", + "--avoid-dev-deps", + "--manifest-path", manifest_path + ) + + out <- processx::run( + command = "cargo", + args = args, + error_on_status = TRUE, + wd = rust_folder, + echo_cmd = echo, + echo = echo, + env = get_cargo_envvars() + ) + + licenses <- jsonlite::parse_json( + out[["stdout"]], + simplifyDataFrame = TRUE + ) + + metadata <- read_cargo_metadata(path = path) + + package_names <- metadata[["packages"]][["dependencies"]][[1]][["name"]] + + licenses <- licenses[licenses[["name"]] %in% package_names, ] .prep_authors <- function(authors, package) { - ifelse(!is.null(authors), authors, paste0(package, " authors")) %>% - stringi::stri_replace_all_regex(r"(\ <.+?>)", "") %>% - stringi::stri_replace_all_regex(r"(\|)", ", ") + authors <- ifelse( + is.na(authors), + paste0(package, " authors"), + authors + ) + + authors <- stringi::stri_replace_all_regex(authors, r"(\ <.+?>)", "") + + stringi::stri_replace_all_regex(authors, r"(\|)", ", ") } separator <- "-------------------------------------------------------------" - note_header <- paste0( - "The binary compiled from the source code of this package contains the following Rust crates:\n", - "\n", - "\n", - separator + note_header <- glue::glue( + " + The binary compiled from the source code of this package \\ + contains the following Rust crates: + + + {separator} + " ) - note_body <- list_license %>% - purrr::discard(function(x) x$name %in% package_names) %>% - purrr::map_chr( - function(x) { - paste0( - "\n", - "Name: ", x$name, "\n", - "Repository: ", x$repository, "\n", - "Authors: ", .prep_authors(x$authors, x$name), "\n", - "License: ", x$license, "\n", - "\n", - separator - ) - } - ) + note_body <- glue::glue_data( + licenses, + " + + Name: {name} + Repository: {repository} + Authors: {.prep_authors(authors, name)} + License: {license} + + {separator} + " + ) write_file( text = c(note_header, note_body), diff --git a/R/read_cargo_metadata.R b/R/read_cargo_metadata.R index 4e314c3d..43817d02 100644 --- a/R/read_cargo_metadata.R +++ b/R/read_cargo_metadata.R @@ -1,6 +1,8 @@ #' Retrieve metadata for packages and workspaces #' #' @param path character scalar, the R package directory +#' @param echo logical scalar, should cargo command and outputs be printed to +#' console (default is TRUE) #' #' @details #' For more details, see @@ -25,22 +27,29 @@ #' read_cargo_metadata() #' } #' -read_cargo_metadata <- function(path = ".") { +read_cargo_metadata <- function(path = ".", echo = TRUE) { check_string(path, class = "rextendr_error") + check_bool(echo, class = "rextendr_error") - root <- rprojroot::find_package_root_file(path = path) - - rust_folder <- normalizePath( - file.path(root, "src", "rust"), - winslash = "/", - mustWork = FALSE + rust_folder <- rprojroot::find_package_root_file( + "src", "rust", + path = path ) + args <- c("metadata", "--format-version=1", "--no-deps") + out <- processx::run( - "cargo", - args = c("metadata", "--format-version=1", "--no-deps"), - wd = rust_folder + command = "cargo", + args = args, + error_on_status = TRUE, + wd = rust_folder, + echo_cmd = echo, + echo = echo, + env = get_cargo_envvars() ) - jsonlite::fromJSON(out[["stdout"]]) + jsonlite::parse_json( + out[["stdout"]], + simplifyDataFrame = TRUE + ) } diff --git a/R/use_crate.R b/R/use_crate.R index 66496a7c..5cf287ea 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -10,6 +10,8 @@ #' @param optional boolean scalar, whether to mark the dependency as optional #' (FALSE by default) #' @param path character scalar, the package directory +#' @param echo logical scalar, should cargo command and outputs be printed to +#' console (default is TRUE) #' #' @details #' For more details regarding these and other options, see the @@ -43,14 +45,15 @@ use_crate <- function( git = NULL, version = NULL, optional = FALSE, - path = ".") { - # check args - check_string(crate) - check_character(features, allow_null = TRUE) - check_string(git, allow_null = TRUE) - check_string(version, allow_null = TRUE) - check_bool(optional) - check_string(path) + path = ".", + echo = TRUE) { + check_string(crate, class = "rextendr_error") + check_character(features, allow_null = TRUE, class = "rextendr_error") + check_string(git, allow_null = TRUE, class = "rextendr_error") + check_string(version, allow_null = TRUE, class = "rextendr_error") + check_bool(optional, class = "rextendr_error") + check_string(path, class = "rextendr_error") + check_bool(echo, class = "rextendr_error") if (!is.null(version) && !is.null(git)) { cli::cli_abort( @@ -80,21 +83,21 @@ use_crate <- function( optional <- NULL } - # get rust directory in project folder - root <- rprojroot::find_package_root_file(path = path) - - rust_folder <- normalizePath( - file.path(root, "src", "rust"), - winslash = "/", - mustWork = FALSE + rust_folder <- rprojroot::find_package_root_file( + "src", "rust", + path = path ) - # run the commmand - processx::run( - "cargo", - c("add", crate, features, git, optional), - echo_cmd = TRUE, - wd = rust_folder + args <- c("add", crate, features, git, optional) + + out <- processx::run( + command = "cargo", + args = args, + error_on_status = TRUE, + wd = rust_folder, + echo_cmd = echo, + echo = echo, + env = get_cargo_envvars() ) invisible() From d930a476c7aa862ebcd23386ab8ed8c2ac2a658d Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 15:17:50 -0700 Subject: [PATCH 02/21] set display color option --- R/read_cargo_metadata.R | 16 ++++++++++++++-- R/use_crate.R | 13 ++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/R/read_cargo_metadata.R b/R/read_cargo_metadata.R index 43817d02..d6b7744a 100644 --- a/R/read_cargo_metadata.R +++ b/R/read_cargo_metadata.R @@ -27,7 +27,10 @@ #' read_cargo_metadata() #' } #' -read_cargo_metadata <- function(path = ".", echo = TRUE) { +read_cargo_metadata <- function( + path = ".", + dependencies = FALSE, + echo = TRUE) { check_string(path, class = "rextendr_error") check_bool(echo, class = "rextendr_error") @@ -36,7 +39,16 @@ read_cargo_metadata <- function(path = ".", echo = TRUE) { path = path ) - args <- c("metadata", "--format-version=1", "--no-deps") + args <- c( + "metadata", + "--format-version=1", + "--no-deps", + if (tty_has_colors()) { + "--color=always" + } else { + "--color=never" + } + ) out <- processx::run( command = "cargo", diff --git a/R/use_crate.R b/R/use_crate.R index 5cf287ea..97f9657b 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -88,7 +88,18 @@ use_crate <- function( path = path ) - args <- c("add", crate, features, git, optional) + args <- c( + "add", + crate, + features, + git, + optional, + if (tty_has_colors()) { + "--color=always" + } else { + "--color=never" + } + ) out <- processx::run( command = "cargo", From c985f030c9e61497072aaeffc6b9b932bdc544b2 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 15:21:12 -0700 Subject: [PATCH 03/21] make --no-deps optional in read_cargo_metadata --- R/read_cargo_metadata.R | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/R/read_cargo_metadata.R b/R/read_cargo_metadata.R index d6b7744a..af0c9d3f 100644 --- a/R/read_cargo_metadata.R +++ b/R/read_cargo_metadata.R @@ -1,6 +1,8 @@ #' Retrieve metadata for packages and workspaces #' #' @param path character scalar, the R package directory +#' @param dependencies logical scalar, whether to include all recursive +#' dependencies in stdout (default is FALSE) #' @param echo logical scalar, should cargo command and outputs be printed to #' console (default is TRUE) #' @@ -32,6 +34,7 @@ read_cargo_metadata <- function( dependencies = FALSE, echo = TRUE) { check_string(path, class = "rextendr_error") + check_bool(dependencies, class = "rextendr_error") check_bool(echo, class = "rextendr_error") rust_folder <- rprojroot::find_package_root_file( @@ -42,7 +45,9 @@ read_cargo_metadata <- function( args <- c( "metadata", "--format-version=1", - "--no-deps", + if (isFALSE(dependencies)) { + "--no-deps" + }, if (tty_has_colors()) { "--color=always" } else { From b38534a58e83a7859603e573d0e687efedae829d Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 15:48:56 -0700 Subject: [PATCH 04/21] fix lintr issues --- R/clean.R | 2 +- R/use_crate.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/clean.R b/R/clean.R index a3e21864..ba2762c9 100644 --- a/R/clean.R +++ b/R/clean.R @@ -58,7 +58,7 @@ clean <- function(path = ".", echo = TRUE) { } ) - out <- processx::run( + processx::run( command = "cargo", args = args, error_on_status = TRUE, diff --git a/R/use_crate.R b/R/use_crate.R index 97f9657b..ed9495e4 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -101,7 +101,7 @@ use_crate <- function( } ) - out <- processx::run( + processx::run( command = "cargo", args = args, error_on_status = TRUE, From 2fac744a532ca93551eac9c2fe66beaa26e17fc4 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 15:50:36 -0700 Subject: [PATCH 05/21] use updated read_cargo_metadata --- R/license_note.R | 88 +++++++++++------------------------------------- 1 file changed, 19 insertions(+), 69 deletions(-) diff --git a/R/license_note.R b/R/license_note.R index e52da7f5..45968ae3 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -1,16 +1,14 @@ #' Generate LICENSE.note file. #' -#' LICENSE.note generated by this function contains information about Rust crate -#' dependencies. To use this function, the -#' [cargo-license](https://crates.io/crates/cargo-license) command must be -#' installed. +#' LICENSE.note generated by this function contains information about all +#' recursive dependencies in Rust crate. #' #' @param path character scalar, the R package directory #' @param echo logical scalar, whether to print cargo command and outputs to the #' console (default is `TRUE`) #' @param quiet logical scalar, whether to signal successful writing of #' LICENSE.note (default is `FALSE`) -#' @param force logical scalar, whether to regenerate LICENSE.note if +#' @param overwrite logical scalar, whether to regenerate LICENSE.note if #' LICENSE.note already exists (default is `TRUE`) #' #' @return No return value, called for side effects. @@ -25,80 +23,26 @@ write_license_note <- function( path = ".", echo = TRUE, quiet = FALSE, - force = TRUE) { + overwrite = TRUE) { check_string(path, class = "rextendr_error") check_bool(echo, class = "rextendr_error") - check_bool(force, class = "rextendr_error") - - if (!cargo_command_available(c("license", "--help"))) { - cli::cli_abort( - c( - "The {.code cargo license} command is required \\ - to run the {.fun write_license_note} function.", - "*" = "Please install cargo-license \\ - ({.url https://crates.io/crates/cargo-license}) first.", - i = "Run {.code cargo install cargo-license} from your terminal." - ), - class = "rextendr_error" - ) - } + check_bool(quiet, class = "rextendr_error") + check_bool(overwrite, class = "rextendr_error") rust_folder <- rprojroot::find_package_root_file( "src", "rust", path = path ) - manifest_path <- normalizePath( - file.path(rust_folder, "Cargo.toml"), - winslash = "/", - mustWork = FALSE - ) - outfile <- rprojroot::find_package_root_file( "LICENSE.note", path = path ) - if (isFALSE(force) && file.exists(outfile)) { - cli::cli_abort( - c( - "LICENSE.note already exists.", - "If you want to regenerate LICENSE.note, \\ - set `force = TRUE` to {.fun write_license_note}." - ), - class = "rextendr_error" - ) - } - - args <- c( - "license", - "--authors", - "--json", - "--avoid-build-deps", - "--avoid-dev-deps", - "--manifest-path", manifest_path - ) - - out <- processx::run( - command = "cargo", - args = args, - error_on_status = TRUE, - wd = rust_folder, - echo_cmd = echo, - echo = echo, - env = get_cargo_envvars() - ) - - licenses <- jsonlite::parse_json( - out[["stdout"]], - simplifyDataFrame = TRUE - ) - - metadata <- read_cargo_metadata(path = path) - - package_names <- metadata[["packages"]][["dependencies"]][[1]][["name"]] - - licenses <- licenses[licenses[["name"]] %in% package_names, ] + packages <- read_cargo_metadata( + path = path, + dependencies = TRUE + )[["packages"]] .prep_authors <- function(authors, package) { authors <- ifelse( @@ -112,6 +56,11 @@ write_license_note <- function( stringi::stri_replace_all_regex(authors, r"(\|)", ", ") } + packages[["authors"]] <- lapply( + packages[["authors"]], + function(x) .prep_authors(x, packages[["name"]]) + ) + separator <- "-------------------------------------------------------------" note_header <- glue::glue( @@ -125,12 +74,12 @@ write_license_note <- function( ) note_body <- glue::glue_data( - licenses, + packages, " Name: {name} Repository: {repository} - Authors: {.prep_authors(authors, name)} + Authors: {authors} License: {license} {separator} @@ -141,6 +90,7 @@ write_license_note <- function( text = c(note_header, note_body), path = outfile, search_root_from = path, - quiet = quiet + quiet = quiet, + overwrite = overwrite ) } From e9c97ea055ff28c95590dd271435c1b8f5217899 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:15:37 -0700 Subject: [PATCH 06/21] replace missing values in license note --- R/license_note.R | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/R/license_note.R b/R/license_note.R index 45968ae3..c54a06f5 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -44,7 +44,28 @@ write_license_note <- function( dependencies = TRUE )[["packages"]] - .prep_authors <- function(authors, package) { + replace_na <- function(data, replace = NA, ...) { + if (vctrs::vec_any_missing(data)) { + missing <- vctrs::vec_detect_missing(data) + data <- vctrs::vec_assign(data, missing, replace, + x_arg = "data", + value_arg = "replace" + ) + } + data + } + + packages[["respository"]] <- replace_na( + packages[["repository"]], + "unknown" + ) + + packages[["licenses"]] <- replace_na( + packages[["repository"]], + "not provided" + ) + + prep_authors <- function(authors, package) { authors <- ifelse( is.na(authors), paste0(package, " authors"), @@ -53,13 +74,14 @@ write_license_note <- function( authors <- stringi::stri_replace_all_regex(authors, r"(\ <.+?>)", "") - stringi::stri_replace_all_regex(authors, r"(\|)", ", ") + paste0(authors, collapse = ", ") } - packages[["authors"]] <- lapply( + packages[["authors"]] <- unlist(Map( + prep_authors, packages[["authors"]], - function(x) .prep_authors(x, packages[["name"]]) - ) + packages[["name"]] + )) separator <- "-------------------------------------------------------------" From 2c4c1a4352569a2f2882d000669d8286c7148bdc Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:22:18 -0700 Subject: [PATCH 07/21] remove error tests for write_license_note --- tests/testthat/test-license_note.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/testthat/test-license_note.R b/tests/testthat/test-license_note.R index 280cb870..2785f534 100644 --- a/tests/testthat/test-license_note.R +++ b/tests/testthat/test-license_note.R @@ -7,6 +7,4 @@ test_that("LICENSE.note is generated properly", { write_license_note() expect_snapshot(cat_file("LICENSE.note")) - expect_rextendr_error(write_license_note(), NA) - expect_rextendr_error(write_license_note(force = FALSE), "LICENSE.note already exists.") }) From 4e65a61d143ca42efd77d6d9d49febb3687eaf10 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:36:34 -0700 Subject: [PATCH 08/21] exclude current package from license note --- R/license_note.R | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/R/license_note.R b/R/license_note.R index c54a06f5..5a86a359 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -39,10 +39,17 @@ write_license_note <- function( path = path ) - packages <- read_cargo_metadata( + metadata <- read_cargo_metadata( path = path, dependencies = TRUE - )[["packages"]] + ) + + packages <- metadata[["packages"]] + + # exclude current package from LICENSE.note + current_package <- metadata[["resolve"]][["root"]] + + packages <- packages[packages[["id"]] != current_package, ] replace_na <- function(data, replace = NA, ...) { if (vctrs::vec_any_missing(data)) { From 448e4573131aeb8404d3185a3f5f75a9ec92bac8 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:43:26 -0700 Subject: [PATCH 09/21] update license note snapshot --- tests/testthat/_snaps/license_note.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/testthat/_snaps/license_note.md b/tests/testthat/_snaps/license_note.md index d2757ccb..d32c1ba9 100644 --- a/tests/testthat/_snaps/license_note.md +++ b/tests/testthat/_snaps/license_note.md @@ -32,35 +32,35 @@ Name: once_cell Repository: https://github.com/matklad/once_cell Authors: Aleksey Kladov - License: Apache-2.0 OR MIT + License: MIT OR Apache-2.0 ------------------------------------------------------------- Name: paste Repository: https://github.com/dtolnay/paste Authors: David Tolnay - License: Apache-2.0 OR MIT + License: MIT OR Apache-2.0 ------------------------------------------------------------- Name: proc-macro2 Repository: https://github.com/dtolnay/proc-macro2 Authors: David Tolnay, Alex Crichton - License: Apache-2.0 OR MIT + License: MIT OR Apache-2.0 ------------------------------------------------------------- Name: quote Repository: https://github.com/dtolnay/quote Authors: David Tolnay - License: Apache-2.0 OR MIT + License: MIT OR Apache-2.0 ------------------------------------------------------------- Name: syn Repository: https://github.com/dtolnay/syn Authors: David Tolnay - License: Apache-2.0 OR MIT + License: MIT OR Apache-2.0 ------------------------------------------------------------- From 2e8b8198cf1c606eb733949e8f87ea093f5a4fd8 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:59:22 -0700 Subject: [PATCH 10/21] fix lintr issue --- R/license_note.R | 5 ----- 1 file changed, 5 deletions(-) diff --git a/R/license_note.R b/R/license_note.R index 5a86a359..1ac29a00 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -29,11 +29,6 @@ write_license_note <- function( check_bool(quiet, class = "rextendr_error") check_bool(overwrite, class = "rextendr_error") - rust_folder <- rprojroot::find_package_root_file( - "src", "rust", - path = path - ) - outfile <- rprojroot::find_package_root_file( "LICENSE.note", path = path From 8550996690e3ff4d4fe374be7a296d63c29652d0 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Mon, 18 Nov 2024 16:59:46 -0700 Subject: [PATCH 11/21] devtools::document --- man/clean.Rd | 10 +++++++++- man/read_cargo_metadata.Rd | 8 +++++++- man/use_crate.Rd | 6 +++++- man/write_license_note.Rd | 23 ++++++++++++++++------- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/man/clean.Rd b/man/clean.Rd index 9f0646e4..bf93399c 100644 --- a/man/clean.Rd +++ b/man/clean.Rd @@ -4,10 +4,13 @@ \alias{clean} \title{Clean Rust binaries and package cache.} \usage{ -clean(path = ".") +clean(path = ".", echo = TRUE) } \arguments{ \item{path}{[ string ] Path to the package root.} + +\item{echo}{logical scalar, should cargo command and outputs be printed to +console (default is TRUE)} } \description{ Removes Rust binaries (such as \code{.dll}/\code{.so} libraries), C wrapper object files, @@ -15,3 +18,8 @@ invokes \verb{cargo clean} to reset cargo target directory (found by default at \verb{pkg_root/src/rust/target/}). Useful when Rust code should be recompiled from scratch. } +\examples{ +\dontrun{ +clean() +} +} diff --git a/man/read_cargo_metadata.Rd b/man/read_cargo_metadata.Rd index 70d20284..61509c8b 100644 --- a/man/read_cargo_metadata.Rd +++ b/man/read_cargo_metadata.Rd @@ -4,10 +4,16 @@ \alias{read_cargo_metadata} \title{Retrieve metadata for packages and workspaces} \usage{ -read_cargo_metadata(path = ".") +read_cargo_metadata(path = ".", dependencies = FALSE, echo = TRUE) } \arguments{ \item{path}{character scalar, the R package directory} + +\item{dependencies}{logical scalar, whether to include all recursive +dependencies in stdout (default is FALSE)} + +\item{echo}{logical scalar, should cargo command and outputs be printed to +console (default is TRUE)} } \value{ \code{list}, including the following elements: diff --git a/man/use_crate.Rd b/man/use_crate.Rd index 1500d6bd..a9489281 100644 --- a/man/use_crate.Rd +++ b/man/use_crate.Rd @@ -10,7 +10,8 @@ use_crate( git = NULL, version = NULL, optional = FALSE, - path = "." + path = ".", + echo = TRUE ) } \arguments{ @@ -27,6 +28,9 @@ crate} (FALSE by default)} \item{path}{character scalar, the package directory} + +\item{echo}{logical scalar, should cargo command and outputs be printed to +console (default is TRUE)} } \value{ \code{NULL}, invisibly diff --git a/man/write_license_note.Rd b/man/write_license_note.Rd index cfdfb835..ff20923f 100644 --- a/man/write_license_note.Rd +++ b/man/write_license_note.Rd @@ -4,20 +4,29 @@ \alias{write_license_note} \title{Generate LICENSE.note file.} \usage{ -write_license_note(path = ".", quiet = FALSE, force = TRUE) +write_license_note(path = ".", echo = TRUE, quiet = FALSE, overwrite = TRUE) } \arguments{ -\item{path}{Path from which package root is looked up.} +\item{path}{character scalar, the R package directory} -\item{quiet}{Logical indicating whether any progress messages should be -generated or not.} +\item{echo}{logical scalar, whether to print cargo command and outputs to the +console (default is \code{TRUE})} -\item{force}{Logical indicating whether to regenerate LICENSE.note if LICENSE.note already exists.} +\item{quiet}{logical scalar, whether to signal successful writing of +LICENSE.note (default is \code{FALSE})} + +\item{overwrite}{logical scalar, whether to regenerate LICENSE.note if +LICENSE.note already exists (default is \code{TRUE})} } \value{ No return value, called for side effects. } \description{ -LICENSE.note generated by this function contains information about Rust crate dependencies. -To use this function, the \href{https://crates.io/crates/cargo-license}{cargo-license} command must be installed. +LICENSE.note generated by this function contains information about all +recursive dependencies in Rust crate. +} +\examples{ +\dontrun{ +write_license_note() +} } From 823f786a6af41c9bd893c36e7414a68d726b8779 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Tue, 19 Nov 2024 18:50:50 -0700 Subject: [PATCH 12/21] fix roxygen typo --- R/find_extendr.R | 8 -------- 1 file changed, 8 deletions(-) diff --git a/R/find_extendr.R b/R/find_extendr.R index f423b5bc..033a567c 100644 --- a/R/find_extendr.R +++ b/R/find_extendr.R @@ -6,12 +6,8 @@ #' #' @return character scalar, path to Rust crate #' -#' @examples -#' \dontrun{ -#' find_extendr_crate() #' @keywords internal #' @noRd -#' } find_extendr_crate <- function( path = ".", error_call = rlang::caller_call()) { @@ -41,12 +37,8 @@ find_extendr_crate <- function( #' #' @return character scalar, path to Cargo manifest #' -#' @examples -#' \dontrun{ -#' find_extendr_manifest() #' @keywords internal #' @noRd -#' } find_extendr_manifest <- function( path = ".", error_call = rlang::caller_call()) { From d3fa7c5629968986cfe5c6d8e3dc45d4172e64d6 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 16:12:23 -0700 Subject: [PATCH 13/21] add replace_na utility --- R/utils.R | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/R/utils.R b/R/utils.R index 7b18d76f..d408c454 100644 --- a/R/utils.R +++ b/R/utils.R @@ -57,3 +57,23 @@ try_exec_cmd <- function(cmd, args = character()) { stringi::stri_split_lines1(result$stdout) } } + +#' Replace missing values in vector +#' +#' @param data vector, data with missing values to replace +#' @param replace scalar, value to substitute for missing values in data +#' @param ... currently ignored +#' +#' @keywords internal +#' @noRd +#' +replace_na <- function(data, replace = NA, ...) { + if (vctrs::vec_any_missing(data)) { + missing <- vctrs::vec_detect_missing(data) + data <- vctrs::vec_assign(data, missing, replace, + x_arg = "data", + value_arg = "replace" + ) + } + data +} From 96e97e0ad4972779f1b69b54254ed3705e8182a8 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 16:13:11 -0700 Subject: [PATCH 14/21] add run_cargo utility --- R/run_cargo.R | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 R/run_cargo.R diff --git a/R/run_cargo.R b/R/run_cargo.R new file mode 100644 index 00000000..4cbd11b0 --- /dev/null +++ b/R/run_cargo.R @@ -0,0 +1,69 @@ +#' Run Cargo subcommands +#' +#' This internal function allows us to maintain consistent specifications for +#' `processx::run()` everywhere it uses. +#' +#' @param args character vector, the Cargo subcommand and flags to be executed. +#' @param wd character scalar, location of the Rust crate, (default is +#' `find_extendr_crate()`). +#' @param error_on_status, logical scalar, whether to error if `status != 0`, +#' (default is `TRUE`). +#' @param echo_cmd logical scalar, whether to print Cargo subcommand and flags +#' to the console (default is `TRUE`). +#' @param echo logical scalar, whether to print standard output and error to the +#' console (default is `TRUE`). +#' @param env character vector, environment variables of the child process. +#' @param parse_json logical scalar, whether to parse JSON-structured standard +#' output (default is `FALSE`). +#' @param error_call call scalar, from rlang docs: "the defused call with which +#' the function running in the frame was invoked." (default is +#' `rlang::caller_call()`) +#' +#' @return if `parse_json = TRUE`, result of parsing JSON-structured standard +#' output; otherwise, standard output is returned as a character scalar. +#' +#' @keywords internal +#' @noRd +run_cargo <- function( + args, + wd = find_extendr_crate(), + error_on_status = TRUE, + echo_cmd = TRUE, + echo = TRUE, + env = get_cargo_envvars(), + parse_json = FALSE, + error_call = rlang::caller_call()) { + check_character(args, call = error_call, class = "rextendr_error") + check_string(wd, call = error_call, class = "rextendr_error") + check_bool(error_on_status, call = error_call, class = "rextendr_error") + check_bool(echo_cmd, call = error_call, class = "rextendr_error") + check_bool(echo, call = error_call, class = "rextendr_error") + check_character(env, call = error_call, class = "rextendr_error") + check_bool(parse_json, call = error_call, class = "rextendr_error") + + out <- processx::run( + command = "cargo", + args = args, + error_on_status = error_on_status, + wd = wd, + echo_cmd = echo_cmd, + echo = echo, + env = env + ) + + stdout <- out[["stdout"]] + + if (length(stdout) != 1L || !is.character(stdout) || is.null(stdout)) { + cli::cli_abort( + "{.code cargo paste(args, collapse = ' ')} failed to return stdout.", + call = error_call, + class = "rextendr_error" + ) + } + + if (parse_json) { + stdout <- jsonlite::parse_json(stdout, simplifyDataFrame = TRUE) + } + + stdout +} From 9aa742af992ff552668c527a5aa02a31e41d0974 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 16:27:33 -0700 Subject: [PATCH 15/21] use run_cargo --- R/clean.R | 49 ++++++---------- R/license_note.R | 123 ++++++++++++++++++++++++---------------- R/read_cargo_metadata.R | 22 ++----- R/use_crate.R | 2 +- 4 files changed, 98 insertions(+), 98 deletions(-) diff --git a/R/clean.R b/R/clean.R index ba2762c9..d889e5b8 100644 --- a/R/clean.R +++ b/R/clean.R @@ -4,9 +4,12 @@ #' invokes `cargo clean` to reset cargo target directory #' (found by default at `pkg_root/src/rust/target/`). #' Useful when Rust code should be recompiled from scratch. -#' @param path \[ string \] Path to the package root. +#' +#' @param path character scalar, path to R package root. #' @param echo logical scalar, should cargo command and outputs be printed to -#' console (default is TRUE) +#' console (default is `TRUE`) +#' +#' @return character vector with names of all deleted files (invisibly). #' #' @export #' @@ -18,33 +21,20 @@ clean <- function(path = ".", echo = TRUE) { check_string(path, class = "rextendr_error") check_bool(echo, class = "rextendr_error") - root <- rprojroot::find_package_root_file(path = path) - - rust_folder <- normalizePath( - file.path(root, "src", "rust"), - winslash = "/", - mustWork = FALSE - ) - - manifest_path <- normalizePath( - file.path(rust_folder, "Cargo.toml"), - winslash = "/", - mustWork = FALSE - ) + manifest_path <- find_extendr_manifest(path = path) # Note: This should be adjusted if `TARGET_DIR` changes in `Makevars` - target_dir <- normalizePath( # nolint: object_usage_linter - file.path(rust_folder, "target"), - winslash = "/", - mustWork = FALSE + target_dir <- rprojroot::find_package_root_file( + "src", "rust", "target", + path = path ) - if (!file.exists(manifest_path)) { - cli::cli_abort(c( - "Unable to clean binaries.", - "!" = "{.file Cargo.toml} not found in {.path {rust_folder}}.", + if (!dir.exists(target_dir)) { + cli::cli_abort( + "Could not clean binaries.", + "Target directory not found at {.path target_dir}.", class = "rextendr_error" - )) + ) } args <- c( @@ -58,14 +48,11 @@ clean <- function(path = ".", echo = TRUE) { } ) - processx::run( - command = "cargo", - args = args, - error_on_status = TRUE, - wd = rust_folder, + run_cargo( + args, + wd = find_extendr_crate(path = path), echo_cmd = echo, - echo = echo, - env = get_cargo_envvars() + echo = echo ) pkgbuild::clean_dll(path = root) diff --git a/R/license_note.R b/R/license_note.R index 1ac29a00..2f5cfa03 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -8,10 +8,10 @@ #' console (default is `TRUE`) #' @param quiet logical scalar, whether to signal successful writing of #' LICENSE.note (default is `FALSE`) -#' @param overwrite logical scalar, whether to regenerate LICENSE.note if +#' @param force logical scalar, whether to regenerate LICENSE.note if #' LICENSE.note already exists (default is `TRUE`) #' -#' @return No return value, called for side effects. +#' @return text printed to LICENSE.note (invisibly). #' #' @export #' @@ -23,40 +23,68 @@ write_license_note <- function( path = ".", echo = TRUE, quiet = FALSE, - overwrite = TRUE) { + force = TRUE) { check_string(path, class = "rextendr_error") check_bool(echo, class = "rextendr_error") check_bool(quiet, class = "rextendr_error") - check_bool(overwrite, class = "rextendr_error") + check_bool(force, class = "rextendr_error") outfile <- rprojroot::find_package_root_file( "LICENSE.note", path = path ) - metadata <- read_cargo_metadata( - path = path, - dependencies = TRUE + args <- c( + "metadata", + "--format-version=1" + ) + + metadata <- run_cargo( + args, + wd = find_extendr_crate(path = path), + echo_cmd = echo, + echo = echo, + parse_json = TRUE ) packages <- metadata[["packages"]] + # did we actually get the recursive dependency metadata we need? + required_variables <- c("name", "repository", "authors", "license", "id") + + packages_exist <- is.data.frame(packages) && + !is.null(packages) && + nrow(packages) > 0 && + all(required_variables %in% names(packages)) + + if (!packages_exist) { + cli::cli_abort( + "Unable to write LICENSE.note.", + "Metadata for recursive dependencies not found.", + call = rlang::caller_call(), + class = "rextendr_error" + ) + } + # exclude current package from LICENSE.note current_package <- metadata[["resolve"]][["root"]] - packages <- packages[packages[["id"]] != current_package, ] + current_package_exists <- length(current_package) == 1 && + is.character(current_package) && + !is.null(current_package) - replace_na <- function(data, replace = NA, ...) { - if (vctrs::vec_any_missing(data)) { - missing <- vctrs::vec_detect_missing(data) - data <- vctrs::vec_assign(data, missing, replace, - x_arg = "data", - value_arg = "replace" - ) - } - data + if (!current_package_exists) { + cli::cli_abort( + "Unable to write LICENSE.note.", + "Failed to identify current Rust crate.", + call = rlang::caller_call(), + class = "rextendr_error" + ) } + packages <- packages[packages[["id"]] != current_package, ] + + # replace missing values packages[["respository"]] <- replace_na( packages[["repository"]], "unknown" @@ -67,18 +95,8 @@ write_license_note <- function( "not provided" ) - prep_authors <- function(authors, package) { - authors <- ifelse( - is.na(authors), - paste0(package, " authors"), - authors - ) - - authors <- stringi::stri_replace_all_regex(authors, r"(\ <.+?>)", "") - - paste0(authors, collapse = ", ") - } - + # remove email addresses and special characters and combine all authors + # of a crate into a single character scalar packages[["authors"]] <- unlist(Map( prep_authors, packages[["authors"]], @@ -87,27 +105,22 @@ write_license_note <- function( separator <- "-------------------------------------------------------------" - note_header <- glue::glue( - " - The binary compiled from the source code of this package \\ - contains the following Rust crates: - - - {separator} - " + note_header <- paste0( + "The binary compiled from the source code of this package ", + "contains the following Rust crates:\n", + "\n", + "\n", + separator, "\n" ) - note_body <- glue::glue_data( - packages, - " - - Name: {name} - Repository: {repository} - Authors: {authors} - License: {license} - - {separator} - " + note_body <- paste0( + "\n", + "Name: ", packages[["name"]], "\n", + "Repository: ", packages[["repository"]], "\n", + "Authors: ", packages[["authors"]], "\n", + "License ", packages[["license"]], "\n", + "\n", + separator, "\n" ) write_file( @@ -115,6 +128,18 @@ write_license_note <- function( path = outfile, search_root_from = path, quiet = quiet, - overwrite = overwrite + overwrite = force + ) +} + +prep_authors <- function(authors, package) { + authors <- ifelse( + is.na(authors), + paste0(package, " authors"), + authors ) + + authors <- stringi::stri_replace_all_regex(authors, r"(\ <.+?>)", "") + + paste0(authors, collapse = ", ") } diff --git a/R/read_cargo_metadata.R b/R/read_cargo_metadata.R index af0c9d3f..30c13360 100644 --- a/R/read_cargo_metadata.R +++ b/R/read_cargo_metadata.R @@ -37,15 +37,10 @@ read_cargo_metadata <- function( check_bool(dependencies, class = "rextendr_error") check_bool(echo, class = "rextendr_error") - rust_folder <- rprojroot::find_package_root_file( - "src", "rust", - path = path - ) - args <- c( "metadata", "--format-version=1", - if (isFALSE(dependencies)) { + if (!dependencies) { "--no-deps" }, if (tty_has_colors()) { @@ -55,18 +50,11 @@ read_cargo_metadata <- function( } ) - out <- processx::run( - command = "cargo", - args = args, - error_on_status = TRUE, - wd = rust_folder, + run_cargo( + args, + wd = find_extendr_crate(path = path), echo_cmd = echo, echo = echo, - env = get_cargo_envvars() - ) - - jsonlite::parse_json( - out[["stdout"]], - simplifyDataFrame = TRUE + parse_json = TRUE ) } diff --git a/R/use_crate.R b/R/use_crate.R index ed9495e4..561ab6ee 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -18,7 +18,7 @@ #' \href{https://doc.rust-lang.org/cargo/commands/cargo-add.html}{Cargo docs} #' for `cargo-add`. #' -#' @return `NULL`, invisibly +#' @return `NULL` (invisibly) #' #' @export #' From ccf2264d56e299bdef42fdb640072010e151f340 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 16:31:41 -0700 Subject: [PATCH 16/21] use run_cargo --- R/use_crate.R | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/R/use_crate.R b/R/use_crate.R index 561ab6ee..e8c492af 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -83,11 +83,6 @@ use_crate <- function( optional <- NULL } - rust_folder <- rprojroot::find_package_root_file( - "src", "rust", - path = path - ) - args <- c( "add", crate, @@ -101,14 +96,11 @@ use_crate <- function( } ) - processx::run( - command = "cargo", - args = args, - error_on_status = TRUE, - wd = rust_folder, + run_cargo( + args, + wd = find_extendr_crate(path = path), echo_cmd = echo, - echo = echo, - env = get_cargo_envvars() + echo = echo ) invisible() From 993f6d763cde3115cd05aae7076d7278f9a4658a Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 16:47:20 -0700 Subject: [PATCH 17/21] pass root to clean_dll --- R/clean.R | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/R/clean.R b/R/clean.R index d889e5b8..38452988 100644 --- a/R/clean.R +++ b/R/clean.R @@ -33,6 +33,7 @@ clean <- function(path = ".", echo = TRUE) { cli::cli_abort( "Could not clean binaries.", "Target directory not found at {.path target_dir}.", + call = rlang::caller_call(), class = "rextendr_error" ) } @@ -55,5 +56,16 @@ clean <- function(path = ".", echo = TRUE) { echo = echo ) + root <- rprojroot::find_package_root_file(path = path) + + if (!dir.exists(root)) { + cli::cli_abort( + "Could not clean binaries.", + "R package directory not found at {.path root}.", + call = rlang::caller_call(), + class = "rextendr_error" + ) + } + pkgbuild::clean_dll(path = root) } From 381de67e316b278ac88dc07e91e44fa53e033403 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 17:14:02 -0700 Subject: [PATCH 18/21] replace glue with paste, update snapshot to reflect new unicode license --- R/license_note.R | 6 +++--- tests/testthat/_snaps/license_note.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/license_note.R b/R/license_note.R index 2f5cfa03..b1508c10 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -110,7 +110,7 @@ write_license_note <- function( "contains the following Rust crates:\n", "\n", "\n", - separator, "\n" + separator ) note_body <- paste0( @@ -118,9 +118,9 @@ write_license_note <- function( "Name: ", packages[["name"]], "\n", "Repository: ", packages[["repository"]], "\n", "Authors: ", packages[["authors"]], "\n", - "License ", packages[["license"]], "\n", + "License: ", packages[["license"]], "\n", "\n", - separator, "\n" + separator ) write_file( diff --git a/tests/testthat/_snaps/license_note.md b/tests/testthat/_snaps/license_note.md index d32c1ba9..44be7cc5 100644 --- a/tests/testthat/_snaps/license_note.md +++ b/tests/testthat/_snaps/license_note.md @@ -67,7 +67,7 @@ Name: unicode-ident Repository: https://github.com/dtolnay/unicode-ident Authors: David Tolnay - License: (MIT OR Apache-2.0) AND Unicode-DFS-2016 + License: (MIT OR Apache-2.0) AND Unicode-3.0 ------------------------------------------------------------- From 97463492b1e7a97323de7fa000a81881aaf48691 Mon Sep 17 00:00:00 2001 From: kbvernon Date: Wed, 20 Nov 2024 18:30:36 -0700 Subject: [PATCH 19/21] devtools::document() --- man/clean.Rd | 7 +++++-- man/use_crate.Rd | 2 +- man/write_license_note.Rd | 6 +++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/man/clean.Rd b/man/clean.Rd index bf93399c..6a756a9b 100644 --- a/man/clean.Rd +++ b/man/clean.Rd @@ -7,10 +7,13 @@ clean(path = ".", echo = TRUE) } \arguments{ -\item{path}{[ string ] Path to the package root.} +\item{path}{character scalar, path to R package root.} \item{echo}{logical scalar, should cargo command and outputs be printed to -console (default is TRUE)} +console (default is \code{TRUE})} +} +\value{ +character vector with names of all deleted files (invisibly). } \description{ Removes Rust binaries (such as \code{.dll}/\code{.so} libraries), C wrapper object files, diff --git a/man/use_crate.Rd b/man/use_crate.Rd index a9489281..2341681f 100644 --- a/man/use_crate.Rd +++ b/man/use_crate.Rd @@ -33,7 +33,7 @@ crate} console (default is TRUE)} } \value{ -\code{NULL}, invisibly +\code{NULL} (invisibly) } \description{ Analogous to \code{usethis::use_package()} but for crate dependencies. diff --git a/man/write_license_note.Rd b/man/write_license_note.Rd index ff20923f..56872675 100644 --- a/man/write_license_note.Rd +++ b/man/write_license_note.Rd @@ -4,7 +4,7 @@ \alias{write_license_note} \title{Generate LICENSE.note file.} \usage{ -write_license_note(path = ".", echo = TRUE, quiet = FALSE, overwrite = TRUE) +write_license_note(path = ".", echo = TRUE, quiet = FALSE, force = TRUE) } \arguments{ \item{path}{character scalar, the R package directory} @@ -15,11 +15,11 @@ console (default is \code{TRUE})} \item{quiet}{logical scalar, whether to signal successful writing of LICENSE.note (default is \code{FALSE})} -\item{overwrite}{logical scalar, whether to regenerate LICENSE.note if +\item{force}{logical scalar, whether to regenerate LICENSE.note if LICENSE.note already exists (default is \code{TRUE})} } \value{ -No return value, called for side effects. +text printed to LICENSE.note (invisibly). } \description{ LICENSE.note generated by this function contains information about all From f90145f4780ca4ee45d44a87eb247b327e3abd32 Mon Sep 17 00:00:00 2001 From: Josiah Parry Date: Thu, 21 Nov 2024 13:25:57 -0800 Subject: [PATCH 20/21] appease codecov. modify docs. and refactor run_cargo --- DESCRIPTION | 1 + R/clean.R | 7 ++-- R/license_note.R | 7 +--- R/read_cargo_metadata.R | 30 ++++++++--------- R/run_cargo.R | 54 ++++++++++++++++++------------ R/use_crate.R | 1 - man/read_cargo_metadata.Rd | 28 ++++++++-------- man/vendor_pkgs.Rd | 3 +- man/write_license_note.Rd | 5 +-- tests/testthat/test-clean.R | 7 ++++ tests/testthat/test-license_note.R | 9 ++++- tests/testthat/test-use_crate.R | 5 ++- tests/testthat/test-utils.R | 13 +++++++ 13 files changed, 99 insertions(+), 71 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index f3a90aef..ffefa8c5 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -73,3 +73,4 @@ Roxygen: list(markdown = TRUE) RoxygenNote: 7.3.2 SystemRequirements: Rust 'cargo'; the crate 'libR-sys' must compile without error +Config/rextendr/version: 0.3.1.9001 diff --git a/R/clean.R b/R/clean.R index 38452988..0a57070f 100644 --- a/R/clean.R +++ b/R/clean.R @@ -31,8 +31,10 @@ clean <- function(path = ".", echo = TRUE) { if (!dir.exists(target_dir)) { cli::cli_abort( - "Could not clean binaries.", - "Target directory not found at {.path target_dir}.", + c( + "Could not clean binaries.", + "Target directory not found at {.path target_dir}." + ), call = rlang::caller_call(), class = "rextendr_error" ) @@ -52,7 +54,6 @@ clean <- function(path = ".", echo = TRUE) { run_cargo( args, wd = find_extendr_crate(path = path), - echo_cmd = echo, echo = echo ) diff --git a/R/license_note.R b/R/license_note.R index b1508c10..214ef961 100644 --- a/R/license_note.R +++ b/R/license_note.R @@ -4,8 +4,6 @@ #' recursive dependencies in Rust crate. #' #' @param path character scalar, the R package directory -#' @param echo logical scalar, whether to print cargo command and outputs to the -#' console (default is `TRUE`) #' @param quiet logical scalar, whether to signal successful writing of #' LICENSE.note (default is `FALSE`) #' @param force logical scalar, whether to regenerate LICENSE.note if @@ -21,11 +19,9 @@ #' } write_license_note <- function( path = ".", - echo = TRUE, quiet = FALSE, force = TRUE) { check_string(path, class = "rextendr_error") - check_bool(echo, class = "rextendr_error") check_bool(quiet, class = "rextendr_error") check_bool(force, class = "rextendr_error") @@ -42,8 +38,7 @@ write_license_note <- function( metadata <- run_cargo( args, wd = find_extendr_crate(path = path), - echo_cmd = echo, - echo = echo, + echo = FALSE, parse_json = TRUE ) diff --git a/R/read_cargo_metadata.R b/R/read_cargo_metadata.R index 30c13360..e3c9ffb7 100644 --- a/R/read_cargo_metadata.R +++ b/R/read_cargo_metadata.R @@ -1,10 +1,10 @@ #' Retrieve metadata for packages and workspaces #' #' @param path character scalar, the R package directory -#' @param dependencies logical scalar, whether to include all recursive -#' dependencies in stdout (default is FALSE) -#' @param echo logical scalar, should cargo command and outputs be printed to -#' console (default is TRUE) +#' @param dependencies Default `FALSE`. A logical scalar, whether to include +#' all recursive dependencies in stdout. +#' @param echo Default `FALSE`. A logical scalar, should cargo command and +#' outputs be printed to the console. #' #' @details #' For more details, see @@ -12,15 +12,16 @@ #' for `cargo-metadata`. See especially "JSON Format" to get a sense of what you #' can expect to find in the returned list. #' -#' @return `list`, including the following elements: -#' - "packages" -#' - "workspace_members" -#' - "workspace_default_members" -#' - "resolve" -#' - "target_directory" -#' - "version" -#' - "workspace_root" -#' - "metadata" +#' @returns +#' A `list` including the following elements: +#' - `packages` +#' - `workspace_members` +#' - `workspace_default_members` +#' - `resolve` +#' - `target_directory` +#' - `version` +#' - `workspace_root` +#' - `metadata` #' #' @export #' @@ -32,7 +33,7 @@ read_cargo_metadata <- function( path = ".", dependencies = FALSE, - echo = TRUE) { + echo = FALSE) { check_string(path, class = "rextendr_error") check_bool(dependencies, class = "rextendr_error") check_bool(echo, class = "rextendr_error") @@ -53,7 +54,6 @@ read_cargo_metadata <- function( run_cargo( args, wd = find_extendr_crate(path = path), - echo_cmd = echo, echo = echo, parse_json = TRUE ) diff --git a/R/run_cargo.R b/R/run_cargo.R index 4cbd11b0..2d521f17 100644 --- a/R/run_cargo.R +++ b/R/run_cargo.R @@ -6,21 +6,20 @@ #' @param args character vector, the Cargo subcommand and flags to be executed. #' @param wd character scalar, location of the Rust crate, (default is #' `find_extendr_crate()`). -#' @param error_on_status, logical scalar, whether to error if `status != 0`, -#' (default is `TRUE`). -#' @param echo_cmd logical scalar, whether to print Cargo subcommand and flags -#' to the console (default is `TRUE`). -#' @param echo logical scalar, whether to print standard output and error to the -#' console (default is `TRUE`). +#' @param error_on_status Default `TRUE`. A logical scalar, whether to error on a non-zero exist status. +#' @param echo_cmd Default `TRUE`. A logical scalar, whether to print Cargo subcommand and flags +#' to the console. +#' @param echo Default `TRUE`. Alogical scalar, whether to print standard output and error to the +#' console. #' @param env character vector, environment variables of the child process. -#' @param parse_json logical scalar, whether to parse JSON-structured standard -#' output (default is `FALSE`). -#' @param error_call call scalar, from rlang docs: "the defused call with which -#' the function running in the frame was invoked." (default is -#' `rlang::caller_call()`) -#' -#' @return if `parse_json = TRUE`, result of parsing JSON-structured standard -#' output; otherwise, standard output is returned as a character scalar. +#' @param parse_json Default `FALSE`. A logical scalar, whether to parse JSON-structured standard +#' output using [`jsonlite::parse_json()`] with `simplifyDataFrame = TRUE`. +#' @param error_call Default [`rlang::caller_call()`]. The defused call with which +#' the function running in the frame was invoked. +#' @param ... additional arguments passed to [`processx::run()`]. +#' @returns +#' A list with elements `status`, `stdout`, `stderr`, and `timeout`. See [`processx::run()`]. If `parse_json = TRUE`, result of parsing JSON-structured standard +#' output. #' #' @keywords internal #' @noRd @@ -28,15 +27,15 @@ run_cargo <- function( args, wd = find_extendr_crate(), error_on_status = TRUE, - echo_cmd = TRUE, echo = TRUE, env = get_cargo_envvars(), parse_json = FALSE, - error_call = rlang::caller_call()) { + error_call = rlang::caller_call(), + ... +) { check_character(args, call = error_call, class = "rextendr_error") check_string(wd, call = error_call, class = "rextendr_error") check_bool(error_on_status, call = error_call, class = "rextendr_error") - check_bool(echo_cmd, call = error_call, class = "rextendr_error") check_bool(echo, call = error_call, class = "rextendr_error") check_character(env, call = error_call, class = "rextendr_error") check_bool(parse_json, call = error_call, class = "rextendr_error") @@ -46,9 +45,10 @@ run_cargo <- function( args = args, error_on_status = error_on_status, wd = wd, - echo_cmd = echo_cmd, + echo_cmd = echo, echo = echo, - env = env + env = env, + ... ) stdout <- out[["stdout"]] @@ -62,8 +62,18 @@ run_cargo <- function( } if (parse_json) { - stdout <- jsonlite::parse_json(stdout, simplifyDataFrame = TRUE) + res <- rlang::try_fetch( + jsonlite::parse_json(stdout, simplifyDataFrame = TRUE), + error = function(cnd) { + cli::cli_abort( + c("Failed to {.code stdout} as json:", " " = "{stdout}"), + parent = cnd, + class = "rextendr_error" + ) + } + ) + return(res) } - stdout -} + out +} \ No newline at end of file diff --git a/R/use_crate.R b/R/use_crate.R index e8c492af..f249c4e3 100644 --- a/R/use_crate.R +++ b/R/use_crate.R @@ -99,7 +99,6 @@ use_crate <- function( run_cargo( args, wd = find_extendr_crate(path = path), - echo_cmd = echo, echo = echo ) diff --git a/man/read_cargo_metadata.Rd b/man/read_cargo_metadata.Rd index 61509c8b..16aea079 100644 --- a/man/read_cargo_metadata.Rd +++ b/man/read_cargo_metadata.Rd @@ -4,28 +4,28 @@ \alias{read_cargo_metadata} \title{Retrieve metadata for packages and workspaces} \usage{ -read_cargo_metadata(path = ".", dependencies = FALSE, echo = TRUE) +read_cargo_metadata(path = ".", dependencies = FALSE, echo = FALSE) } \arguments{ \item{path}{character scalar, the R package directory} -\item{dependencies}{logical scalar, whether to include all recursive -dependencies in stdout (default is FALSE)} +\item{dependencies}{Default \code{FALSE}. A logical scalar, whether to include +all recursive dependencies in stdout.} -\item{echo}{logical scalar, should cargo command and outputs be printed to -console (default is TRUE)} +\item{echo}{Default \code{FALSE}. A logical scalar, should cargo command and +outputs be printed to the console.} } \value{ -\code{list}, including the following elements: +A \code{list} including the following elements: \itemize{ -\item "packages" -\item "workspace_members" -\item "workspace_default_members" -\item "resolve" -\item "target_directory" -\item "version" -\item "workspace_root" -\item "metadata" +\item \code{packages} +\item \code{workspace_members} +\item \code{workspace_default_members} +\item \code{resolve} +\item \code{target_directory} +\item \code{version} +\item \code{workspace_root} +\item \code{metadata} } } \description{ diff --git a/man/vendor_pkgs.Rd b/man/vendor_pkgs.Rd index 98e75d57..8e133c19 100644 --- a/man/vendor_pkgs.Rd +++ b/man/vendor_pkgs.Rd @@ -30,8 +30,7 @@ compressed \code{vendor.tar.xz} which will be shipped with package itself. If you have modified your dependencies, you will need need to repackage } \examples{ - \dontrun{ - vendor_pkgs() +vendor_pkgs() } } diff --git a/man/write_license_note.Rd b/man/write_license_note.Rd index 56872675..6bf93d53 100644 --- a/man/write_license_note.Rd +++ b/man/write_license_note.Rd @@ -4,14 +4,11 @@ \alias{write_license_note} \title{Generate LICENSE.note file.} \usage{ -write_license_note(path = ".", echo = TRUE, quiet = FALSE, force = TRUE) +write_license_note(path = ".", quiet = FALSE, force = TRUE) } \arguments{ \item{path}{character scalar, the R package directory} -\item{echo}{logical scalar, whether to print cargo command and outputs to the -console (default is \code{TRUE})} - \item{quiet}{logical scalar, whether to signal successful writing of LICENSE.note (default is \code{FALSE})} diff --git a/tests/testthat/test-clean.R b/tests/testthat/test-clean.R index 3ee022ee..b20c8354 100644 --- a/tests/testthat/test-clean.R +++ b/tests/testthat/test-clean.R @@ -11,8 +11,15 @@ test_that("rextendr::clean() removes cargo target directory & binaries", { expect_equal(length(dir("src", pattern = "testpkg\\..*")), 1) expect_true(dir.exists(file.path("src", "rust", "target"))) + + # clean once clean() + # we expect an error the second time + expect_error(clean()) + + expect_error(clean(1L)) + expect_error(clean(echo = NULL)) expect_equal(length(dir("src", pattern = "testpkg\\..*")), 0) expect_false(dir.exists(file.path("src", "rust", "target"))) }) diff --git a/tests/testthat/test-license_note.R b/tests/testthat/test-license_note.R index 2785f534..1092e6ae 100644 --- a/tests/testthat/test-license_note.R +++ b/tests/testthat/test-license_note.R @@ -3,8 +3,15 @@ test_that("LICENSE.note is generated properly", { skip_if_cargo_unavailable(c("license", "--help")) local_package("testPackage") + + # try running write_license_note() when there is nothing present + dir.create(file.path("src", "rust"), recursive = TRUE) + expect_error(write_license_note()) + + # create license note for extendr package use_extendr() write_license_note() - expect_snapshot(cat_file("LICENSE.note")) + expect_error(write_license_note(path = NULL)) + expect_error(write_license_note(force = "yup")) }) diff --git a/tests/testthat/test-use_crate.R b/tests/testthat/test-use_crate.R index b3419279..8f7328ed 100644 --- a/tests/testthat/test-use_crate.R +++ b/tests/testthat/test-use_crate.R @@ -15,16 +15,15 @@ test_that("use_crate() adds dependency to package or workspace", { path = path ) - metadata <- read_cargo_metadata(path) + metadata <- read_cargo_metadata(path, echo = FALSE) dependency <- metadata[["packages"]][["dependencies"]][[1]] dependency <- dependency[dependency[["name"]] == "serde", ] expect_equal(dependency[["name"]], "serde") - expect_equal(dependency[["features"]][[1]], "derive") - expect_equal(dependency[["req"]], "^1.0.1") + }) test_that("use_crate() errors when user passes git and version arguments", { diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index bc45a226..8f975eeb 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -20,3 +20,16 @@ test_that("`try_exec_cmd()` returns stdout when command is available", { echo <- "This is an echo" expect_equal(try_exec_cmd("echo", echo), echo) }) + + +test_that("`replace_na()` respects type", { + x <- 1:5 + x[2] <- NA + expect_error(replace_na(x, "L")) +}) + +test_that("`replace_na()` replaces with the correct value", { + x <- 1:5 + x[2] <- NA_integer_ + expect_identical(replace_na(x, -99L), c(1L, -99L, 3L, 4L, 5L)) +}) From d46ee515c90ff7a86150c04d09b1a51a369d704c Mon Sep 17 00:00:00 2001 From: Josiah Parry Date: Thu, 21 Nov 2024 13:45:12 -0800 Subject: [PATCH 21/21] lintr --- R/run_cargo.R | 23 ++++++++++++----------- tests/testthat/test-license_note.R | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/R/run_cargo.R b/R/run_cargo.R index 2d521f17..1db8ed13 100644 --- a/R/run_cargo.R +++ b/R/run_cargo.R @@ -18,20 +18,21 @@ #' the function running in the frame was invoked. #' @param ... additional arguments passed to [`processx::run()`]. #' @returns -#' A list with elements `status`, `stdout`, `stderr`, and `timeout`. See [`processx::run()`]. If `parse_json = TRUE`, result of parsing JSON-structured standard -#' output. +#' A list with elements `status`, `stdout`, `stderr`, and `timeout`. +#' See [`processx::run()`]. If `parse_json = TRUE`, result of parsing +#' JSON-structured standard output. #' #' @keywords internal #' @noRd run_cargo <- function( - args, - wd = find_extendr_crate(), - error_on_status = TRUE, - echo = TRUE, - env = get_cargo_envvars(), - parse_json = FALSE, - error_call = rlang::caller_call(), - ... + args, + wd = find_extendr_crate(), + error_on_status = TRUE, + echo = TRUE, + env = get_cargo_envvars(), + parse_json = FALSE, + error_call = rlang::caller_call(), + ... ) { check_character(args, call = error_call, class = "rextendr_error") check_string(wd, call = error_call, class = "rextendr_error") @@ -76,4 +77,4 @@ run_cargo <- function( } out -} \ No newline at end of file +} diff --git a/tests/testthat/test-license_note.R b/tests/testthat/test-license_note.R index 1092e6ae..92bf73cb 100644 --- a/tests/testthat/test-license_note.R +++ b/tests/testthat/test-license_note.R @@ -8,7 +8,7 @@ test_that("LICENSE.note is generated properly", { dir.create(file.path("src", "rust"), recursive = TRUE) expect_error(write_license_note()) - # create license note for extendr package + # create license note for extendr package use_extendr() write_license_note() expect_snapshot(cat_file("LICENSE.note"))