Repository: extendr/extendr
Pull request #719 (2024-03-31 10:39:18)

`xtask` add more features
I've added a bunch of extra features to the `xtask`. There are many changes lines, but these should be complete, in the sense that they won't require too much exploration to review.

Also, I'm trying to out as a out to migrate changes from the "fork" to `master`.

- Added a `cargo extendr fmt` that uses `cargo fmt` / `rustfmt` on both the crates in the workspace, plus `tests/extendrtests/src/rust` as well. We rarely format that integration-test crate.
- Added `cargo extendr devtools-test -a` which will "accept" the newly generated macros from a given build. We will see the changes in the PR, but this will help contributors. The workflow is imagined to be: `cargo extendr devtools-test`, that then tests things, and a potential new `macro-snapshot.md` is generated. Then one is supposed to call `cargo extendr devtools-test -a` to accept the snapshot, and perform testing _again_, just in case.
- Added `cargo extendr document`, which is a feature to run `rextendr::document()` for the `tests/extendrtests`-package. In the fork, I've vendored the `rextendr` crate, which makes it possible to have a consistent contributor-experience, if one is changing `rextendr` simultaneously.


CGMossa: I could use some help to shape this PR into an acceptable version. If @Ilia-Kosenkov got time to go through it, that would be great honestly. 
CGMossa: I added a few checking mechanisms.


Patch:
diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml
index a67d44eb9..519396b62 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 11af2744d..868d15c6d 100644
--- a/xtask/README.md
+++ b/xtask/README.md
@@ -17,10 +17,12 @@ Usage: xtask <COMMAND>
 
 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/mod.rs b/xtask/src/cli/mod.rs
index a074082b0..a293479ad 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 6afa4fead..eead2eff2 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<RCmdCheckErrorOn> for ErrorOn {
-    fn into(self) -> RCmdCheckErrorOn {
-        match self {
+impl From<ErrorOn> 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 000000000..4a499a153
--- /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 ca0df58fd..ebcbd0714 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/generate_docs.rs b/xtask/src/commands/generate_docs.rs
index 2c4fec48f..62c427060 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 1c20dbbe6..39952e1fc 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 000000000..0effcd4c1
--- /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<dyn Error>> {
+    let _document_handle = swap_extendr_api_path(shell)?;
+
+    run_rextendr_document(shell)
+}
+
+fn run_rextendr_document(shell: &Shell) -> Result<(), Box<dyn Error>> {
+    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/main.rs b/xtask/src/main.rs
index a8a146278..b7085b617 100644
--- a/xtask/src/main.rs
+++ b/xtask/src/main.rs
@@ -21,6 +21,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             .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<dyn std::error::Error>> {
         )?,
         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(())


Test patch:
diff --git a/xtask/src/cli/devtools_test.rs b/xtask/src/cli/devtools_test.rs
new file mode 100644
index 000000000..7a2539c80
--- /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/commands/devtools_test.rs b/xtask/src/commands/devtools_test.rs
index c9ec66825..1ccdbb1c4 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<dyn Error>> {
+pub(crate) fn run(shell: &Shell, args: DevtoolsTestArg) -> Result<(), Box<dyn Error>> {
     let _document_handle = swap_extendr_api_path(shell)?;
 
-    run_tests(shell)?;
+    run_tests(shell, args)?;
 
     Ok(())
 }
 
-fn run_tests(shell: &Shell) -> Result<(), Box<dyn Error>> {
+fn run_tests(shell: &Shell, args: DevtoolsTestArg) -> Result<(), Box<dyn Error>> {
     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/extendrtests/with_absolute_path.rs b/xtask/src/extendrtests/with_absolute_path.rs
index 141ae0b04..706dc8c8a 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<u8>,
     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<DocumentHandle, Box<dyn Error>> {
+pub(crate) fn swap_extendr_api_path(shell: &Shell) -> Result<DocumentMutHandle, Box<dyn Error>> {
     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<DocumentHandle, Box
 
     let mut replacement = InlineTable::new();
 
-    let item = Value::from(get_replacement_path(&current_path));
+    let item = Value::from(get_replacement_path_extendr_api(&current_path));
     replacement.entry("path").or_insert(item);
     *extendr_api_entry = Value::InlineTable(replacement);
 
+    #[allow(non_snake_case)]
+    let libR_sys_entry = get_libR_sys_entry(&mut cargo_toml);
+    #[allow(non_snake_case)]
+    if let Some(libR_sys_entry) = libR_sys_entry {
+        let mut replacement = InlineTable::new();
+        if let Some(replacement_path) = get_replacement_path_libR_sys(&current_path) {
+            let item = Value::from(replacement_path);
+            replacement.entry("path").or_insert(item);
+            *libR_sys_entry = Value::InlineTable(replacement);
+        }
+    }
+
+    // save altered paths
     shell.write_file(CARGO_TOML, cargo_toml.to_string())?;
-    Ok(DocumentHandle {
+    Ok(DocumentMutHandle {
         document: original_cargo_toml_bytes,
         shell,
     })
 }
 
-fn get_replacement_path(path: &Path) -> 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<String> {
+    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<P: AsRef<Path>>(
     shell: &Shell,
     path: P,