Skip to content

Commit

Permalink
fix(core/shell): speedup Command.execute & fix extra new lines (#1299)
Browse files Browse the repository at this point in the history
* 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 tauri-apps/tauri#6519
ref: tauri-apps/tauri#7684 (comment)

* fmt

* dedup code
  • Loading branch information
amrbashir authored May 9, 2024
1 parent 5c1b791 commit eb1679b
Show file tree
Hide file tree
Showing 12 changed files with 197 additions and 94 deletions.
5 changes: 5 additions & 0 deletions .changes/shell-command-execute-extra-new-lines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"shell-js": "patch"
---

Fix `Command.execute` API including extra new lines.
6 changes: 6 additions & 0 deletions .changes/shell-command-execute-speed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"shell": "patch"
"shell-js": "patch"
---

Improve the speed of the JS `Command.execute` API
28 changes: 28 additions & 0 deletions examples/api/src-tauri/gen/schemas/desktop-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -2339,6 +2339,13 @@
"shell:allow-open"
]
},
{
"description": "shell:allow-spawn -> Enables the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"shell:allow-spawn"
]
},
{
"description": "shell:allow-stdin-write -> Enables the stdin_write command without any pre-configured scope.",
"type": "string",
Expand Down Expand Up @@ -2367,6 +2374,13 @@
"shell:deny-open"
]
},
{
"description": "shell:deny-spawn -> Denies the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"shell:deny-spawn"
]
},
{
"description": "shell:deny-stdin-write -> Denies the stdin_write command without any pre-configured scope.",
"type": "string",
Expand Down Expand Up @@ -5689,6 +5703,13 @@
"shell:allow-open"
]
},
{
"description": "shell:allow-spawn -> Enables the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"shell:allow-spawn"
]
},
{
"description": "shell:allow-stdin-write -> Enables the stdin_write command without any pre-configured scope.",
"type": "string",
Expand Down Expand Up @@ -5717,6 +5738,13 @@
"shell:deny-open"
]
},
{
"description": "shell:deny-spawn -> Denies the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"shell:deny-spawn"
]
},
{
"description": "shell:deny-stdin-write -> Denies the stdin_write command without any pre-configured scope.",
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion plugins/shell/api-iife.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion plugins/shell/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#[path = "src/scope_entry.rs"]
mod scope_entry;

const COMMANDS: &[&str] = &["execute", "stdin_write", "kill", "open"];
const COMMANDS: &[&str] = &["execute", "spawn", "stdin_write", "kill", "open"];

fn main() {
tauri_plugin::Builder::new(COMMANDS)
Expand Down
129 changes: 43 additions & 86 deletions plugins/shell/guest-js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,39 +99,6 @@ interface ChildProcess<O extends IOPayload> {
stderr: O;
}

/**
* Spawns a process.
*
* @ignore
* @param program The name of the scoped command.
* @param onEventHandler Event handler.
* @param args Program arguments.
* @param options Configuration for the process spawn.
* @returns A promise resolving to the process id.
*
* @since 2.0.0
*/
async function execute<O extends IOPayload>(
onEventHandler: (event: CommandEvent<O>) => void,
program: string,
args: string | string[] = [],
options?: InternalSpawnOptions,
): Promise<number> {
if (typeof args === "object") {
Object.freeze(args);
}

const onEvent = new Channel<CommandEvent<O>>();
onEvent.onmessage = onEventHandler;

return await invoke<number>("plugin:shell|execute", {
program,
args,
options,
onEvent,
});
}

/**
* @since 2.0.0
*/
Expand Down Expand Up @@ -513,27 +480,38 @@ class Command<O extends IOPayload> extends EventEmitter<CommandEvents> {
* @since 2.0.0
*/
async spawn(): Promise<Child> {
return await execute<O>(
(event) => {
switch (event.event) {
case "Error":
this.emit("error", event.payload);
break;
case "Terminated":
this.emit("close", event.payload);
break;
case "Stdout":
this.stdout.emit("data", event.payload);
break;
case "Stderr":
this.stderr.emit("data", event.payload);
break;
}
},
this.program,
this.args,
this.options,
).then((pid) => new Child(pid));
const program = this.program;
const args = this.args;
const options = this.options;

if (typeof args === "object") {
Object.freeze(args);
}

const onEvent = new Channel<CommandEvent<O>>();
onEvent.onmessage = (event) => {
switch (event.event) {
case "Error":
this.emit("error", event.payload);
break;
case "Terminated":
this.emit("close", event.payload);
break;
case "Stdout":
this.stdout.emit("data", event.payload);
break;
case "Stderr":
this.stderr.emit("data", event.payload);
break;
}
};

return await invoke<number>("plugin:shell|spawn", {
program,
args,
options,
onEvent,
}).then((pid) => new Child(pid));
}

/**
Expand All @@ -553,40 +531,19 @@ class Command<O extends IOPayload> extends EventEmitter<CommandEvents> {
* @since 2.0.0
*/
async execute(): Promise<ChildProcess<O>> {
return await new Promise((resolve, reject) => {
this.on("error", reject);

const stdout: O[] = [];
const stderr: O[] = [];
this.stdout.on("data", (line: O) => {
stdout.push(line);
});
this.stderr.on("data", (line: O) => {
stderr.push(line);
});

this.on("close", (payload: TerminatedPayload) => {
resolve({
code: payload.code,
signal: payload.signal,
stdout: this.collectOutput(stdout) as O,
stderr: this.collectOutput(stderr) as O,
});
});

this.spawn().catch(reject);
});
}
const program = this.program;
const args = this.args;
const options = this.options;

/** @ignore */
private collectOutput(events: O[]): string | Uint8Array {
if (this.options.encoding === "raw") {
return events.reduce<Uint8Array>((p, c) => {
return new Uint8Array([...p, ...(c as Uint8Array), 10]);
}, new Uint8Array());
} else {
return events.join("\n");
if (typeof args === "object") {
Object.freeze(args);
}

return await invoke<ChildProcess<O>>("plugin:shell|execute", {
program,
args,
options,
});
}
}

Expand Down
13 changes: 13 additions & 0 deletions plugins/shell/permissions/autogenerated/commands/spawn.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Automatically generated - DO NOT EDIT!

"$schema" = "../../schemas/schema.json"

[[permission]]
identifier = "allow-spawn"
description = "Enables the spawn command without any pre-configured scope."
commands.allow = ["spawn"]

[[permission]]
identifier = "deny-spawn"
description = "Denies the spawn command without any pre-configured scope."
commands.deny = ["spawn"]
2 changes: 2 additions & 0 deletions plugins/shell/permissions/autogenerated/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@
|`deny-kill`|Denies the kill command without any pre-configured scope.|
|`allow-open`|Enables the open command without any pre-configured scope.|
|`deny-open`|Denies the open command without any pre-configured scope.|
|`allow-spawn`|Enables the spawn command without any pre-configured scope.|
|`deny-spawn`|Denies the spawn command without any pre-configured scope.|
|`allow-stdin-write`|Enables the stdin_write command without any pre-configured scope.|
|`deny-stdin-write`|Denies the stdin_write command without any pre-configured scope.|
14 changes: 14 additions & 0 deletions plugins/shell/permissions/schemas/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,20 @@
"deny-open"
]
},
{
"description": "allow-spawn -> Enables the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"allow-spawn"
]
},
{
"description": "deny-spawn -> Denies the spawn command without any pre-configured scope.",
"type": "string",
"enum": [
"deny-spawn"
]
},
{
"description": "allow-stdin-write -> Enables the stdin_write command without any pre-configured scope.",
"type": "string",
Expand Down
86 changes: 80 additions & 6 deletions plugins/shell/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,15 @@ fn default_env() -> Option<HashMap<String, String>> {
Some(HashMap::default())
}

#[allow(clippy::too_many_arguments)]
#[tauri::command]
pub fn execute<R: Runtime>(
#[inline(always)]
fn prepare_cmd<R: Runtime>(
window: Window<R>,
shell: State<'_, Shell<R>>,
program: String,
args: ExecuteArgs,
on_event: Channel,
options: CommandOptions,
command_scope: CommandScope<crate::scope::ScopeAllowedCommand>,
global_scope: GlobalScope<crate::scope::ScopeAllowedCommand>,
) -> crate::Result<ChildId> {
) -> crate::Result<(crate::process::Command, EncodingWrapper)> {
let scope = crate::scope::ShellScope {
scopes: command_scope
.allows()
Expand Down Expand Up @@ -151,6 +148,7 @@ pub fn execute<R: Runtime>(
} else {
command = command.env_clear();
}

let encoding = match options.encoding {
Option::None => EncodingWrapper::Text(None),
Some(encoding) => match encoding.as_str() {
Expand All @@ -168,6 +166,82 @@ pub fn execute<R: Runtime>(
},
};

Ok((command, encoding))
}

#[derive(Serialize)]
enum Output {
String(String),
Raw(Vec<u8>),
}

#[derive(Serialize)]
pub struct ChildProcessReturn {
code: Option<i32>,
signal: Option<i32>,
#[serde(flatten)]
stdout: Output,
#[serde(flatten)]
stderr: Output,
}

#[allow(clippy::too_many_arguments)]
#[tauri::command]
pub fn execute<R: Runtime>(
window: Window<R>,
program: String,
args: ExecuteArgs,
options: CommandOptions,
command_scope: CommandScope<crate::scope::ScopeAllowedCommand>,
global_scope: GlobalScope<crate::scope::ScopeAllowedCommand>,
) -> crate::Result<ChildProcessReturn> {
let (command, encoding) =
prepare_cmd(window, program, args, options, command_scope, global_scope)?;

let mut command: std::process::Command = command.into();
let output = command.output()?;

let (stdout, stderr) = match encoding {
EncodingWrapper::Text(Some(encoding)) => (
Output::String(encoding.decode_with_bom_removal(&output.stdout).0.into()),
Output::String(encoding.decode_with_bom_removal(&output.stderr).0.into()),
),
EncodingWrapper::Text(None) => (
Output::String(String::from_utf8(output.stdout)?),
Output::String(String::from_utf8(output.stderr)?),
),
EncodingWrapper::Raw => (Output::Raw(output.stdout), Output::Raw(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,
})
}

#[allow(clippy::too_many_arguments)]
#[tauri::command]
pub fn spawn<R: Runtime>(
window: Window<R>,
shell: State<'_, Shell<R>>,
program: String,
args: ExecuteArgs,
on_event: Channel,
options: CommandOptions,
command_scope: CommandScope<crate::scope::ScopeAllowedCommand>,
global_scope: GlobalScope<crate::scope::ScopeAllowedCommand>,
) -> crate::Result<ChildId> {
let (command, encoding) =
prepare_cmd(window, program, args, options, command_scope, global_scope)?;

let (mut rx, child) = command.spawn()?;

let pid = child.pid();
Expand Down
3 changes: 3 additions & 0 deletions plugins/shell/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ pub enum Error {
/// JSON error.
#[error(transparent)]
Json(#[from] serde_json::Error),
/// Utf8 error.
#[error(transparent)]
Utf8(#[from] std::string::FromUtf8Error),
}

impl Serialize for Error {
Expand Down
Loading

0 comments on commit eb1679b

Please sign in to comment.