From 0294a0044d91d982fded53464e0352b7f8a33064 Mon Sep 17 00:00:00 2001 From: CGMossa Date: Wed, 3 Apr 2024 01:11:23 +0200 Subject: [PATCH] `xtask` add more features (#719) * fix: extendrtests points to libR-sys submodule updated `xtask` as well * xtask: added `extendrtests` to the `check-fmt`. Added a `cargo extendr fmt` that formats `extendr`-crates and `extendrtests`. * xtask: use `DocumentMut` instead of `Document` from toml_edit * xtask: add `--snapshot-accept` / `-a` option to subcommand `cargo extendr devtools-test` that helps accept newly formed macro-snapshots. * xtask: just pin `toml_edit` to a minor version, instead of an arbitrary patch-version. * xtask: added support for vendored / submodule libR-sys / rextendr, or installed ones. Co-authored-by: Ilia Kosenkov Co-authored-by: Ilia Kosenkov * xtask: added comment on which `rextendr` is being used * xtask: ` -a, --accept-snapshot Accept newly generated macro-snapshots` --------- Co-authored-by: Ilia Kosenkov --- xtask/Cargo.toml | 2 +- xtask/README.md | 19 ++++++- xtask/src/cli/devtools_test.rs | 12 +++++ xtask/src/cli/mod.rs | 12 +++-- xtask/src/cli/r_cmd_check.rs | 6 +-- xtask/src/commands/cargo_fmt.rs | 14 +++++ xtask/src/commands/cargo_fmt_check.rs | 15 ++++-- xtask/src/commands/devtools_test.rs | 18 +++++-- xtask/src/commands/generate_docs.rs | 3 +- xtask/src/commands/mod.rs | 2 + xtask/src/commands/rextendr_document.rs | 55 ++++++++++++++++++++ xtask/src/extendrtests/with_absolute_path.rs | 50 ++++++++++++++---- xtask/src/main.rs | 4 +- 13 files changed, 184 insertions(+), 28 deletions(-) create mode 100644 xtask/src/cli/devtools_test.rs create mode 100644 xtask/src/commands/cargo_fmt.rs create mode 100644 xtask/src/commands/rextendr_document.rs diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index a67d44eb9b..519396b621 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -11,5 +11,5 @@ publish = false [dependencies] clap = { version = "4.4.6", features = ["derive"] } -toml_edit = "0.22.6" +toml_edit = "0.22" xshell = "0.2.5" diff --git a/xtask/README.md b/xtask/README.md index 11af2744d8..868d15c6dc 100644 --- a/xtask/README.md +++ b/xtask/README.md @@ -17,10 +17,12 @@ Usage: xtask Commands: check-fmt Run cargo fmt on extendr + fmt Run `cargo fmt` on extendr crates r-cmd-check Run R CMD check on {extendrtests} doc Generate documentation for all features msrv Check that the specified rust-version is MSRV devtools-test Run devtools::test() on {extendrtests} + document Generate wrappers with `rextendr::document()` help Print this message or the help of the given subcommand(s) Options: @@ -35,9 +37,9 @@ Let's describe some of the features listed above. This checks if the rust code follows the specified `rustfmt`. It does not format the code. In order to do so, please run `cargo fmt`. -## `devtools-test` +## `fmt` -This performs `devtools::test()` in R, within the R-package `tests/extendrtests`. +This command calls `cargo fmt` within the workspace, ensuring that the contents of `tests/extendrtests/src/rust` folder are formatted as well. ## `r-cmd-check` @@ -48,6 +50,19 @@ Runs `R CMD check` tests in `tests/extendrtests`. Generates documentation as seen on [/extendr.github.io](https://extendr.github.io/extendr/extendr_api/), meaning it will enable feature `full-functionality`, which includes all the optional dependencies, plus all the additive features. +## `msrv` + +Performs Minimum Supported Rust Version (MSRV) check in the repo. + +## `devtools-test` + +This performs `devtools::test()` in R, within the R-package `tests/extendrtests`. If this call results in messages about updating +macro snapshots, one may run `cargo extendr devtools-test -a` to accept the newly generated snapshots. + +## `document` + +Use `cargo extendr document` to regenerate the wrappers for the integration-test package `tests/extendrtests`. This invokes `rextendr::document()`. + ## TODO In the following are features that could be added to `xtask`. diff --git a/xtask/src/cli/devtools_test.rs b/xtask/src/cli/devtools_test.rs new file mode 100644 index 0000000000..7a2539c80a --- /dev/null +++ b/xtask/src/cli/devtools_test.rs @@ -0,0 +1,12 @@ +use clap::Args; + +#[derive(Args, Debug)] +pub(crate) struct DevtoolsTestArg { + #[arg( + long, + short, + default_value = "false", + help = "Accept newly generated macro-snapshots" + )] + pub(crate) accept_snapshot: bool, +} diff --git a/xtask/src/cli/mod.rs b/xtask/src/cli/mod.rs index a074082b08..a293479add 100644 --- a/xtask/src/cli/mod.rs +++ b/xtask/src/cli/mod.rs @@ -1,9 +1,11 @@ use clap::{Parser, Subcommand}; -use crate::cli::r_cmd_check::RCmdCheckArg; - +pub(crate) mod devtools_test; pub(crate) mod r_cmd_check; +use devtools_test::DevtoolsTestArg; +use r_cmd_check::RCmdCheckArg; + #[derive(Parser, Debug)] #[command(author, version, about, long_about = None)] pub(crate) struct Cli { @@ -15,6 +17,8 @@ pub(crate) struct Cli { pub(crate) enum Commands { #[command(about = "Run cargo fmt on extendr")] CheckFmt, + #[command(about = "Run `cargo fmt` on extendr crates")] + Fmt, #[command(about = "Run R CMD check on {extendrtests}")] RCmdCheck(RCmdCheckArg), #[command(about = "Generate documentation for all features")] @@ -22,7 +26,9 @@ pub(crate) enum Commands { #[command(about = "Check that the specified rust-version is MSRV")] Msrv, #[command(about = "Run devtools::test() on {extendrtests}")] - DevtoolsTest, + DevtoolsTest(DevtoolsTestArg), + #[command(about = "Generate wrappers by `rextendr::document()`")] + Document, } pub(crate) fn parse() -> Cli { diff --git a/xtask/src/cli/r_cmd_check.rs b/xtask/src/cli/r_cmd_check.rs index 6afa4feade..eead2eff27 100644 --- a/xtask/src/cli/r_cmd_check.rs +++ b/xtask/src/cli/r_cmd_check.rs @@ -24,9 +24,9 @@ pub(crate) enum ErrorOn { Error, } -impl Into for ErrorOn { - fn into(self) -> RCmdCheckErrorOn { - match self { +impl From for RCmdCheckErrorOn { + fn from(val: ErrorOn) -> Self { + match val { ErrorOn::Never => RCmdCheckErrorOn::Never, ErrorOn::Note => RCmdCheckErrorOn::Note, ErrorOn::Warning => RCmdCheckErrorOn::Warning, diff --git a/xtask/src/commands/cargo_fmt.rs b/xtask/src/commands/cargo_fmt.rs new file mode 100644 index 0000000000..4a499a153a --- /dev/null +++ b/xtask/src/commands/cargo_fmt.rs @@ -0,0 +1,14 @@ +use xshell::{cmd, Error, Shell}; + +pub(crate) fn run(shell: &Shell) -> Result<(), Error> { + // extendr-api, extendr-macros, extendr-engine, xtask + cmd!(shell, "cargo fmt --all").run()?; + // extendrtests + cmd!( + shell, + "cargo fmt --all --manifest-path tests/extendrtests/src/rust/Cargo.toml" + ) + .run()?; + + Ok(()) +} diff --git a/xtask/src/commands/cargo_fmt_check.rs b/xtask/src/commands/cargo_fmt_check.rs index ca0df58fd6..ebcbd0714e 100644 --- a/xtask/src/commands/cargo_fmt_check.rs +++ b/xtask/src/commands/cargo_fmt_check.rs @@ -1,12 +1,19 @@ use xshell::{cmd, Error, Shell}; pub(crate) fn run(shell: &Shell) -> Result<(), Error> { - let check_result = cmd!(shell, "cargo fmt -- --check").run(); - if check_result.is_err() { - println!("Please run `cargo fmt --all`"); + let check_extendr = cmd!(shell, "cargo fmt --all -- --check").run(); + + let check_extendrtests = cmd!( + shell, + "cargo fmt --all --manifest-path tests/extendrtests/src/rust/Cargo.toml -- --check" + ) + .run(); + + if check_extendr.is_err() || check_extendrtests.is_err() { + println!("Please run `cargo extendr fmt`"); } else { println!("Success!"); } - check_result + check_extendr } diff --git a/xtask/src/commands/devtools_test.rs b/xtask/src/commands/devtools_test.rs index c9ec66825f..1ccdbb1c48 100644 --- a/xtask/src/commands/devtools_test.rs +++ b/xtask/src/commands/devtools_test.rs @@ -2,18 +2,28 @@ use std::error::Error; use xshell::{cmd, Shell}; -use crate::extendrtests::with_absolute_path::{swap_extendr_api_path, R_FOLDER_PATH}; +use crate::{ + cli::devtools_test::DevtoolsTestArg, + extendrtests::with_absolute_path::{swap_extendr_api_path, R_FOLDER_PATH}, +}; -pub(crate) fn run(shell: &Shell) -> Result<(), Box> { +pub(crate) fn run(shell: &Shell, args: DevtoolsTestArg) -> Result<(), Box> { let _document_handle = swap_extendr_api_path(shell)?; - run_tests(shell)?; + run_tests(shell, args)?; Ok(()) } -fn run_tests(shell: &Shell) -> Result<(), Box> { +fn run_tests(shell: &Shell, args: DevtoolsTestArg) -> Result<(), Box> { let _r_path = shell.push_dir(R_FOLDER_PATH); + if args.accept_snapshot { + cmd!( + shell, + "Rscript -e testthat::snapshot_accept(\"macro-snapshot\")" + ) + .run()?; + } cmd!(shell, "Rscript -e devtools::test()").run()?; Ok(()) diff --git a/xtask/src/commands/generate_docs.rs b/xtask/src/commands/generate_docs.rs index 2c4fec48f5..62c427060d 100644 --- a/xtask/src/commands/generate_docs.rs +++ b/xtask/src/commands/generate_docs.rs @@ -1,7 +1,8 @@ use xshell::{cmd, Error, Shell}; +/// Generates documentation like on the site extendr.github.io pub(crate) fn run(shell: &Shell) -> Result<(), Error> { - let _generate_docs = cmd!( + cmd!( shell, "cargo doc --workspace --no-deps --document-private-items --features full-functionality" ) diff --git a/xtask/src/commands/mod.rs b/xtask/src/commands/mod.rs index 1c20dbbe61..39952e1fcd 100644 --- a/xtask/src/commands/mod.rs +++ b/xtask/src/commands/mod.rs @@ -1,5 +1,7 @@ +pub(crate) mod cargo_fmt; pub(crate) mod cargo_fmt_check; pub(crate) mod cargo_msrv; pub(crate) mod devtools_test; pub(crate) mod generate_docs; pub(crate) mod r_cmd_check; +pub(crate) mod rextendr_document; diff --git a/xtask/src/commands/rextendr_document.rs b/xtask/src/commands/rextendr_document.rs new file mode 100644 index 0000000000..0effcd4c1e --- /dev/null +++ b/xtask/src/commands/rextendr_document.rs @@ -0,0 +1,55 @@ +//! This invokes `rextendr::document()` within `tests/extendrtests`. +//! +//! It uses the vendored `rextendr` in the repository as the source package. +//! +//! 1. Ensure that `git submodule update --init` was invoked once, as to setup +//! the vendored `rextendr` package. +//! 2. `devtools` must be installed on system. +//! +//! +//! The idea here is to be able to develop `rextendr` alongside `extendr`, +//! as well as ease the development of extendr. +//! +use std::error::Error; + +use crate::extendrtests::with_absolute_path::{swap_extendr_api_path, R_FOLDER_PATH}; +use xshell::{cmd, Shell}; + +pub(crate) fn run(shell: &Shell) -> Result<(), Box> { + let _document_handle = swap_extendr_api_path(shell)?; + + run_rextendr_document(shell) +} + +fn run_rextendr_document(shell: &Shell) -> Result<(), Box> { + let _r_path = shell.push_dir(R_FOLDER_PATH); + + let rextendr_submodule = std::path::Path::new(".../../rextendr"); + let rextendr_submodule = matches!(rextendr_submodule.try_exists(), Ok(true)); + if rextendr_submodule { + println!("Loading vendored `{{rextendr}}`"); + cmd!(shell, "Rscript") + .args([ + "-e", + r#"requireNamespace("devtools")"#, + "-e", + r#"devtools::load_all("../../rextendr")"#, + "-e", + r#"rextendr::document()"#, + ]) + .run()?; + } else { + // check if rextendr is installed and use that instead + println!("Using installed `{{rextendr}}`"); + cmd!(shell, "Rscript") + .args([ + "-e", + r#"requireNamespace("rextendr")"#, + "-e", + r#"rextendr::document()"#, + ]) + .run()?; + } + + Ok(()) +} diff --git a/xtask/src/extendrtests/with_absolute_path.rs b/xtask/src/extendrtests/with_absolute_path.rs index 141ae0b046..706dc8c8ab 100644 --- a/xtask/src/extendrtests/with_absolute_path.rs +++ b/xtask/src/extendrtests/with_absolute_path.rs @@ -1,7 +1,7 @@ use std::error::Error; use std::path::Path; -use toml_edit::{Document, InlineTable, Value}; +use toml_edit::{DocumentMut, InlineTable, Value}; use xshell::Shell; use crate::extendrtests::path_helper::RCompatiblePath; @@ -12,12 +12,12 @@ const RUST_FOLDER_PATH: &str = "tests/extendrtests/src/rust"; const CARGO_TOML: &str = "Cargo.toml"; #[derive(Debug)] -pub(crate) struct DocumentHandle<'a> { +pub(crate) struct DocumentMutHandle<'a> { document: Vec, shell: &'a Shell, } -impl<'a> Drop for DocumentHandle<'a> { +impl<'a> Drop for DocumentMutHandle<'a> { fn drop(&mut self) { let _rust_folder = self.shell.push_dir(RUST_FOLDER_PATH); self.shell @@ -26,13 +26,14 @@ impl<'a> Drop for DocumentHandle<'a> { } } -pub(crate) fn swap_extendr_api_path(shell: &Shell) -> Result> { +pub(crate) fn swap_extendr_api_path(shell: &Shell) -> Result> { let current_path = shell.current_dir(); let _rust_folder = shell.push_dir(RUST_FOLDER_PATH); let original_cargo_toml_bytes = read_file_with_line_ending(shell, CARGO_TOML)?; - let original_cargo_toml: Document = std::str::from_utf8(&original_cargo_toml_bytes)?.parse()?; + let original_cargo_toml: DocumentMut = + std::str::from_utf8(&original_cargo_toml_bytes)?.parse()?; let mut cargo_toml = original_cargo_toml.clone(); @@ -41,24 +42,46 @@ pub(crate) fn swap_extendr_api_path(shell: &Shell) -> Result String { +fn get_replacement_path_extendr_api(path: &Path) -> String { let path = path.adjust_for_r(); format!("{path}/extendr-api") } -fn get_extendr_api_entry(document: &mut Document) -> Option<&mut Value> { +#[allow(non_snake_case)] +fn get_replacement_path_libR_sys(path: &Path) -> Option { + let path = path.adjust_for_r(); + #[allow(non_snake_case)] + let libR_sys_path = format!("{path}/libR-sys"); + let valid_path = std::path::Path::new(&libR_sys_path); + matches!(valid_path.try_exists(), Ok(true)).then_some(libR_sys_path) +} + +fn get_extendr_api_entry(document: &mut DocumentMut) -> Option<&mut Value> { document .get_mut("patch")? .get_mut("crates-io")? @@ -66,6 +89,15 @@ fn get_extendr_api_entry(document: &mut Document) -> Option<&mut Value> { .as_value_mut() } +#[allow(non_snake_case)] +fn get_libR_sys_entry(document: &mut DocumentMut) -> Option<&mut Value> { + document + .get_mut("patch")? + .get_mut("crates-io")? + .get_mut("libR-sys")? + .as_value_mut() +} + fn read_file_with_line_ending>( shell: &Shell, path: P, diff --git a/xtask/src/main.rs b/xtask/src/main.rs index a8a1462786..b7085b6176 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -21,6 +21,7 @@ fn main() -> Result<(), Box> { .canonicalize()?, ); match cli.command { + cli::Commands::Fmt => commands::cargo_fmt::run(&shell)?, cli::Commands::CheckFmt => commands::cargo_fmt_check::run(&shell)?, cli::Commands::RCmdCheck(RCmdCheckArg { no_build_vignettes, @@ -35,7 +36,8 @@ fn main() -> Result<(), Box> { )?, cli::Commands::Doc => commands::generate_docs::run(&shell)?, cli::Commands::Msrv => commands::cargo_msrv::run(&shell)?, - cli::Commands::DevtoolsTest => commands::devtools_test::run(&shell)?, + cli::Commands::DevtoolsTest(args) => commands::devtools_test::run(&shell, args)?, + cli::Commands::Document => commands::rextendr_document::run(&shell)?, }; Ok(())