Repository: extendr/extendr
Pull request #648 (2023-10-25 22:48:18)

`xtask`: Add `r-cmd-check`
Prototype implementation based on calling `rcmdcheck::rcmdcheck()`. We can do it manually, but it requires using a temp dir, copying files there, running `R CMD build` and only then `R CMD check`. Let's resort to using a proven R package for now.

Reuses the same Cargo rewriting mechanism as `devtools-test`, which is now improved & stores original Cargo.toml as bytes (no cr/lf issue).

Ilia-Kosenkov: I might consider calling this task on CI instead of the weird magic that we do right now.
CGMossa: Output:
```


❯ checking package dependencies ... ERROR
  Packages suggested but not available: 'patrick', 'lobstr'

  The suggested packages are required for a complete check.
  Checking can be attempted without them by setting the environment
  variable _R_CHECK_FORCE_SUGGESTS_ to a false value.

  See section 'The DESCRIPTION file' in the 'Writing R Extensions'
  manual.

❯ checking CRAN incoming feasibility ... [11s] NOTE
  Maintainer: 'Claus O. Wilke <wilke@austin.utexas.edu>'

  New submission

  Found the following (possibly) invalid URLs:
    URL: https://opensource.org/licenses/MIT (moved to https://opensource.org/license/mit/)
      From: README.md
      Status: 200
      Message: OK
```

CGMossa: code to install all packages from a `DESCRIPTION` file..

```r
require("attachment")
attachment::install_from_description("tests/extendrtests/DESCRIPTION")
```


Ilia-Kosenkov: I rolled back CI/CD changes here, I will introduce them separately. It seems `xtask` is working smoothly on CI.

Patch:
diff --git a/xtask/src/commands/r_cmd_check.rs b/xtask/src/commands/r_cmd_check.rs
index e6ffe7938..8310257a5 100644
--- a/xtask/src/commands/r_cmd_check.rs
+++ b/xtask/src/commands/r_cmd_check.rs
@@ -1,17 +1,31 @@
-use xshell::{Error, Shell};
-
-pub(crate) fn run(shell: &Shell, no_build_vignettes: bool) -> Result<(), Error> {
-    let mut cmd = shell
-        .cmd("R")
-        .arg("CMD")
-        .arg("check")
-        .arg("tests/extendrtests")
-        .arg("--no-manual")
-        .arg("--as-cran")
-        .arg("--force-multiarch");
+use std::error::Error;
 
+use xshell::Shell;
+
+use crate::extendrtests::with_absolute_path::{swap_extendr_api_path, R_FOLDER_PATH};
+
+pub(crate) fn run(shell: &Shell, no_build_vignettes: bool) -> Result<(), Box<dyn Error>> {
+    let _document_handle = swap_extendr_api_path(shell)?;
+
+    run_r_cmd_check(shell, no_build_vignettes)
+}
+
+fn run_r_cmd_check(shell: &Shell, no_build_vignettes: bool) -> Result<(), Box<dyn Error>> {
+    let _r_path = shell.push_dir(R_FOLDER_PATH);
+    let mut args = vec!["'--as-cran'", "'--no-manual'"];
     if no_build_vignettes {
-        cmd = cmd.arg("--no-build-vignettes");
+        args.push("'--no-build-vignettes'");
     }
-    cmd.run()
+
+    let args = format!("c({0})", args.join(", "));
+
+    shell
+        .cmd("Rscript")
+        .arg("-e")
+        .arg(format!(
+            "rcmdcheck::rcmdcheck(args = {args}, error_on = 'warning')"
+        ))
+        .run()?;
+
+    Ok(())
 }
diff --git a/xtask/src/main.rs b/xtask/src/main.rs
index d0ab97ee6..53bcd3851 100644
--- a/xtask/src/main.rs
+++ b/xtask/src/main.rs
@@ -6,6 +6,7 @@ use xshell::Shell;
 
 mod cli;
 mod commands;
+mod extendrtests;
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let cli = cli::parse();


Test patch:
diff --git a/xtask/src/commands/devtools_test.rs b/xtask/src/commands/devtools_test.rs
index 2e7aa0748..c9ec66825 100644
--- a/xtask/src/commands/devtools_test.rs
+++ b/xtask/src/commands/devtools_test.rs
@@ -1,12 +1,8 @@
 use std::error::Error;
-use std::path::Path;
 
-use toml_edit::{Document, InlineTable, Value};
 use xshell::{cmd, Shell};
 
-const RUST_FOLDER_PATH: &str = "tests/extendrtests/src/rust";
-const R_FOLDER_PATH: &str = "tests/extendrtests";
-const CARGO_TOML: &str = "Cargo.toml";
+use crate::extendrtests::with_absolute_path::{swap_extendr_api_path, R_FOLDER_PATH};
 
 pub(crate) fn run(shell: &Shell) -> Result<(), Box<dyn Error>> {
     let _document_handle = swap_extendr_api_path(shell)?;
@@ -16,94 +12,9 @@ pub(crate) fn run(shell: &Shell) -> Result<(), Box<dyn Error>> {
     Ok(())
 }
 
-#[derive(Debug, Clone)]
-struct DocumentHandle<'a> {
-    document: Document,
-    is_crlf: bool,
-    shell: &'a Shell,
-}
-
-impl<'a> Drop for DocumentHandle<'a> {
-    fn drop(&mut self) {
-        let _rust_folder = self.shell.push_dir(RUST_FOLDER_PATH);
-        write_file_preserve_line_ending(self.shell, CARGO_TOML, &self.document, self.is_crlf)
-            .expect("Failed to restore Cargo.toml");
-    }
-}
-
 fn run_tests(shell: &Shell) -> Result<(), Box<dyn Error>> {
     let _r_path = shell.push_dir(R_FOLDER_PATH);
     cmd!(shell, "Rscript -e devtools::test()").run()?;
 
     Ok(())
 }
-
-fn swap_extendr_api_path(shell: &Shell) -> Result<DocumentHandle, Box<dyn Error>> {
-    let current_path = shell.current_dir();
-    let _rust_folder = shell.push_dir(RUST_FOLDER_PATH);
-
-    let (original_cargo_toml, is_crlf) = read_file_with_line_ending(shell, CARGO_TOML)?;
-
-    let original_cargo_toml: Document = original_cargo_toml.parse()?;
-
-    let mut cargo_toml = original_cargo_toml.clone();
-
-    let extendr_api_entry =
-        get_extendr_api_entry(&mut cargo_toml).ok_or("`extendr-api` not found in Cargo.toml")?;
-
-    let mut replacement = InlineTable::new();
-
-    let item = Value::from(get_replacement_path(&current_path));
-    replacement.entry("path").or_insert(item);
-    *extendr_api_entry = Value::InlineTable(replacement);
-
-    write_file_preserve_line_ending(shell, CARGO_TOML, &cargo_toml, is_crlf)?;
-    Ok(DocumentHandle {
-        document: original_cargo_toml,
-        is_crlf,
-        shell,
-    })
-}
-
-fn get_replacement_path(path: &Path) -> String {
-    let path = path.to_string_lossy();
-    let path = if cfg!(target_os = "windows") && path.starts_with(r"\\?\") {
-        path[4..].replace('\\', "/")
-    } else {
-        path.to_string()
-    };
-
-    format!("{}/extendr-api", path)
-}
-
-fn get_extendr_api_entry(document: &mut Document) -> Option<&mut Value> {
-    document
-        .get_mut("patch")?
-        .get_mut("crates-io")?
-        .get_mut("extendr-api")?
-        .as_value_mut()
-}
-
-fn read_file_with_line_ending<P: AsRef<Path>>(
-    shell: &Shell,
-    path: P,
-) -> Result<(String, bool), Box<dyn Error>> {
-    let file_contents = shell.read_binary_file(path)?;
-    let file_contents = String::from_utf8(file_contents)?;
-    let is_crlf = file_contents.contains("\r\n");
-    Ok((file_contents, is_crlf))
-}
-
-fn write_file_preserve_line_ending<P: AsRef<Path>>(
-    shell: &Shell,
-    path: P,
-    contents: &Document,
-    is_crlf: bool,
-) -> Result<(), Box<dyn Error>> {
-    let mut file_contents = contents.to_string();
-    if is_crlf {
-        file_contents = file_contents.replace('\n', "\r\n");
-    }
-    shell.write_file(path, file_contents)?;
-    Ok(())
-}
diff --git a/xtask/src/extendrtests/mod.rs b/xtask/src/extendrtests/mod.rs
new file mode 100644
index 000000000..90f9995cc
--- /dev/null
+++ b/xtask/src/extendrtests/mod.rs
@@ -0,0 +1,1 @@
+pub(crate) mod with_absolute_path;
diff --git a/xtask/src/extendrtests/with_absolute_path.rs b/xtask/src/extendrtests/with_absolute_path.rs
new file mode 100644
index 000000000..4c2dbc275
--- /dev/null
+++ b/xtask/src/extendrtests/with_absolute_path.rs
@@ -0,0 +1,78 @@
+use std::error::Error;
+use std::path::Path;
+
+use toml_edit::{Document, InlineTable, Value};
+use xshell::Shell;
+
+pub(crate) const R_FOLDER_PATH: &str = "tests/extendrtests";
+
+const RUST_FOLDER_PATH: &str = "tests/extendrtests/src/rust";
+const CARGO_TOML: &str = "Cargo.toml";
+
+#[derive(Debug)]
+pub(crate) struct DocumentHandle<'a> {
+    document: Vec<u8>,
+    shell: &'a Shell,
+}
+
+impl<'a> Drop for DocumentHandle<'a> {
+    fn drop(&mut self) {
+        let _rust_folder = self.shell.push_dir(RUST_FOLDER_PATH);
+        self.shell
+            .write_file(CARGO_TOML, &self.document)
+            .expect("Failed to restore Cargo.toml");
+    }
+}
+
+pub(crate) fn swap_extendr_api_path(shell: &Shell) -> Result<DocumentHandle, 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 mut cargo_toml = original_cargo_toml.clone();
+
+    let extendr_api_entry =
+        get_extendr_api_entry(&mut cargo_toml).ok_or("`extendr-api` not found in Cargo.toml")?;
+
+    let mut replacement = InlineTable::new();
+
+    let item = Value::from(get_replacement_path(&current_path));
+    replacement.entry("path").or_insert(item);
+    *extendr_api_entry = Value::InlineTable(replacement);
+
+    shell.write_file(CARGO_TOML, cargo_toml.to_string())?;
+    Ok(DocumentHandle {
+        document: original_cargo_toml_bytes,
+        shell,
+    })
+}
+
+fn get_replacement_path(path: &Path) -> String {
+    let path = path.to_string_lossy();
+    let path = if cfg!(target_os = "windows") && path.starts_with(r"\\?\") {
+        path[4..].replace('\\', "/")
+    } else {
+        path.to_string()
+    };
+
+    format!("{}/extendr-api", path)
+}
+
+fn get_extendr_api_entry(document: &mut Document) -> Option<&mut Value> {
+    document
+        .get_mut("patch")?
+        .get_mut("crates-io")?
+        .get_mut("extendr-api")?
+        .as_value_mut()
+}
+
+fn read_file_with_line_ending<P: AsRef<Path>>(
+    shell: &Shell,
+    path: P,
+) -> Result<Vec<u8>, Box<dyn Error>> {
+    let file_contents = shell.read_binary_file(path)?;
+    Ok(file_contents)
+}