fix(core/shell): speedup Command.execute & fix extra new lines (#9706)

* fix(core/shell): speedup `Command.execute` & fix extra new lines The speed gains comes from running the Command in Rust fully and returning the result in one go instead of using events. The extra new lines was a regression from #6519 ref: #7684 (comment) * fix unix build * clippy * cleanup --------- Co-authored-by: Lucas Nogueira <[email protected]>
tauri-apps · May 9, 2024 · 7f885bd · 7f885bd
1 parent 2eb2137
commit 7f885bd
Show file tree

Hide file tree

Showing 5 changed files with 218 additions and 155 deletions.
diff --git a/.changes/shell-execute-extra-newline.md b/.changes/shell-execute-extra-newline.md
@@ -0,0 +1,6 @@
+---
+"@tauri-apps/api": "patch:bug"
+---
+
+Fix The JS `Command.execute` API from `shell` module including extra new lines.
+
diff --git a/.changes/shell-execute-performance.md b/.changes/shell-execute-performance.md
@@ -0,0 +1,7 @@
+---
+"tauri": "patch:enhance"
+"@tauri-apps/api": "patch:enhance"
+---
+
+Enhance the speed of The JS `Command.execute` API from `shell` module.
+
diff --git a/core/tauri/scripts/bundle.global.js b/core/tauri/scripts/bundle.global.js
diff --git a/core/tauri/src/endpoints/shell.rs b/core/tauri/src/endpoints/shell.rs
@@ -8,7 +8,7 @@ use super::InvokeContext;
 use crate::{api::ipc::CallbackFn, Runtime};
 #[cfg(shell_scope)]
 use crate::{Manager, Scopes};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use tauri_macros::{command_enum, module_command_handler, CommandModule};
 
 #[cfg(shell_scope)]
@@ -63,6 +63,15 @@ pub struct CommandOptions {
 #[derive(Deserialize, CommandModule)]
 #[serde(tag = "cmd", rename_all = "camelCase")]
 pub enum Cmd {
+  /// The execute and return script API.
+  #[cmd(shell_script, "shell > execute or shell > sidecar")]
+  #[serde(rename_all = "camelCase")]
+  ExecuteAndReturn {
+    program: String,
+    args: ExecuteArgs,
+    #[serde(default)]
+    options: CommandOptions,
+  },
   /// The execute script API.
   #[cmd(shell_script, "shell > execute or shell > sidecar")]
   #[serde(rename_all = "camelCase")]
@@ -81,101 +90,88 @@ pub enum Cmd {
   Open { path: String, with: Option<String> },
 }
 
+#[derive(Serialize)]
+#[cfg(any(shell_execute, shell_sidecar))]
+struct ChildProcessReturn {
+  code: Option<i32>,
+  signal: Option<i32>,
+  stdout: String,
+  stderr: String,
+}
+
 impl Cmd {
   #[module_command_handler(shell_script)]
-  #[allow(unused_variables)]
+  fn execute_and_return<R: Runtime>(
+    context: InvokeContext<R>,
+    program: String,
+    args: ExecuteArgs,
+    options: CommandOptions,
+  ) -> super::Result<ChildProcessReturn> {
+    let encoding = options
+      .encoding
+      .as_ref()
+      .and_then(|encoding| crate::api::process::Encoding::for_label(encoding.as_bytes()));
+    let command = prepare_cmd(&context, &program, args, options)?;
+
+    let mut command: std::process::Command = command.into();
+    let output = command.output()?;
+
+    let (stdout, stderr) = match encoding {
+      Some(encoding) => (
+        encoding.decode_with_bom_removal(&output.stdout).0.into(),
+        encoding.decode_with_bom_removal(&output.stderr).0.into(),
+      ),
+      None => (
+        String::from_utf8(output.stdout)?,
+        String::from_utf8(output.stderr)?,
+      ),
+    };
+
+    #[cfg(unix)]
+    use std::os::unix::process::ExitStatusExt;
+
+    Ok(ChildProcessReturn {
+      code: output.status.code(),
+      #[cfg(windows)]
+      signal: None,
+      #[cfg(unix)]
+      signal: output.status.signal(),
+      stdout,
+      stderr,
+    })
+  }
+
+  #[module_command_handler(shell_script)]
   fn execute<R: Runtime>(
     context: InvokeContext<R>,
     program: String,
     args: ExecuteArgs,
     on_event_fn: CallbackFn,
     options: CommandOptions,
   ) -> super::Result<ChildId> {
-    let mut command = if options.sidecar {
-      #[cfg(not(shell_sidecar))]
-      return Err(crate::Error::ApiNotAllowlisted("shell > sidecar".to_string()).into_anyhow());
-      #[cfg(shell_sidecar)]
-      {
-        let program = PathBuf::from(program);
-        let program_as_string = program.display().to_string();
-        let program_no_ext_as_string = program.with_extension("").display().to_string();
-        let configured_sidecar = context
-          .config
-          .tauri
-          .bundle
-          .external_bin
-          .as_ref()
-          .map(|bins| {
-            bins
-              .iter()
-              .find(|b| b == &&program_as_string || b == &&program_no_ext_as_string)
-          })
-          .unwrap_or_default();
-        if let Some(sidecar) = configured_sidecar {
-          context
-            .window
-            .state::<Scopes>()
-            .shell
-            .prepare_sidecar(&program.to_string_lossy(), sidecar, args)
-            .map_err(crate::error::into_anyhow)?
-        } else {
-          return Err(crate::Error::SidecarNotAllowed(program).into_anyhow());
-        }
-      }
-    } else {
-      #[cfg(not(shell_execute))]
-      return Err(crate::Error::ApiNotAllowlisted("shell > execute".to_string()).into_anyhow());
-      #[cfg(shell_execute)]
-      match context
-        .window
-        .state::<Scopes>()
-        .shell
-        .prepare(&program, args)
-      {
-        Ok(cmd) => cmd,
-        Err(e) => {
-          #[cfg(debug_assertions)]
-          eprintln!("{e}");
-          return Err(crate::Error::ProgramNotAllowed(PathBuf::from(program)).into_anyhow());
-        }
-      }
-    };
-    #[cfg(any(shell_execute, shell_sidecar))]
-    {
-      if let Some(cwd) = options.cwd {
-        command = command.current_dir(cwd);
-      }
-      if let Some(env) = options.env {
-        command = command.envs(env);
-      } else {
-        command = command.env_clear();
-      }
-      if let Some(encoding) = options.encoding {
-        if let Some(encoding) = crate::api::process::Encoding::for_label(encoding.as_bytes()) {
-          command = command.encoding(encoding);
-        } else {
-          return Err(anyhow::anyhow!(format!("unknown encoding {encoding}")));
-        }
-      }
-      let (mut rx, child) = command.spawn()?;
+    use std::future::Future;
+    use std::pin::Pin;
 
-      let pid = child.pid();
-      command_child_store().lock().unwrap().insert(pid, child);
+    let command = prepare_cmd(&context, &program, args, options)?;
 
-      crate::async_runtime::spawn(async move {
-        while let Some(event) = rx.recv().await {
-          if matches!(event, crate::api::process::CommandEvent::Terminated(_)) {
-            command_child_store().lock().unwrap().remove(&pid);
-          }
-          let js = crate::api::ipc::format_callback(on_event_fn, &event)
-            .expect("unable to serialize CommandEvent");
+    let (mut rx, child) = command.spawn()?;
 
-          let _ = context.window.eval(js.as_str());
+    let pid = child.pid();
+    command_child_store().lock().unwrap().insert(pid, child);
+
+    crate::async_runtime::spawn(async move {
+      while let Some(event) = rx.recv().await {
+        if matches!(event, crate::api::process::CommandEvent::Terminated(_)) {
+          command_child_store().lock().unwrap().remove(&pid);
         }
-      });
+        let js = crate::api::ipc::format_callback(on_event_fn, &event)
+          .expect("unable to serialize CommandEvent");
 
-      Ok(pid)
-    }
+        let _ = context.window.eval(js.as_str());
+      }
+    });
+
+    Ok(pid)
   }
 
   #[module_command_handler(shell_script)]
@@ -226,6 +222,81 @@ impl Cmd {
   }
 }
 
+fn prepare_cmd<R: Runtime>(
+  context: &InvokeContext<R>,
+  program: &String,
+  args: ExecuteArgs,
+  options: CommandOptions,
+) -> super::Result<crate::api::process::Command> {
+  let mut command = if options.sidecar {
+    #[cfg(not(shell_sidecar))]
+    return Err(crate::Error::ApiNotAllowlisted("shell > sidecar".to_string()).into_anyhow());
+    #[cfg(shell_sidecar)]
+    {
+      let program = PathBuf::from(program);
+      let program_as_string = program.display().to_string();
+      let program_no_ext_as_string = program.with_extension("").display().to_string();
+      let configured_sidecar = context
+        .config
+        .tauri
+        .bundle
+        .external_bin
+        .as_ref()
+        .map(|bins| {
+          bins
+            .iter()
+            .find(|b| b == &&program_as_string || b == &&program_no_ext_as_string)
+        })
+        .unwrap_or_default();
+      if let Some(sidecar) = configured_sidecar {
+        context
+          .window
+          .state::<Scopes>()
+          .shell
+          .prepare_sidecar(&program.to_string_lossy(), sidecar, args)
+          .map_err(crate::error::into_anyhow)
+      } else {
+        Err(crate::Error::SidecarNotAllowed(program).into_anyhow())
+      }
+    }
+  } else {
+    #[cfg(not(shell_execute))]
+    return Err(crate::Error::ApiNotAllowlisted("shell > execute".to_string()).into_anyhow());
+    #[cfg(shell_execute)]
+    match context
+      .window
+      .state::<Scopes>()
+      .shell
+      .prepare(program, args)
+    {
+      Ok(cmd) => Ok(cmd),
+      Err(e) => {
+        #[cfg(debug_assertions)]
+        eprintln!("{e}");
+        Err(crate::Error::ProgramNotAllowed(PathBuf::from(program)).into_anyhow())
+      }
+    }
+  }?;
+
+  if let Some(cwd) = options.cwd {
+    command = command.current_dir(cwd);
+  }
+  if let Some(env) = options.env {
+    command = command.envs(env);
+  } else {
+    command = command.env_clear();
+  }
+  if let Some(encoding) = &options.encoding {
+    if let Some(encoding) = crate::api::process::Encoding::for_label(encoding.as_bytes()) {
+      command = command.encoding(encoding);
+    } else {
+      return Err(anyhow::anyhow!(format!("unknown encoding {encoding}")));
+    }
+  }
+
+  Ok(command)
+}
+
 #[cfg(test)]
 mod tests {
   use super::{Buffer, ChildId, CommandOptions, ExecuteArgs};