From eb1679b99780e5d2b867f5649a1ccc2f3f70ab56 Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Thu, 9 May 2024 18:15:03 +0300 Subject: [PATCH] fix(core/shell): speedup `Command.execute` & fix extra new lines (#1299) * 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 https://github.com/tauri-apps/tauri/pull/6519 ref: https://github.com/tauri-apps/tauri/issues/7684#issuecomment-2100897383 * fmt * dedup code --- .../shell-command-execute-extra-new-lines.md | 5 + .changes/shell-command-execute-speed.md | 6 + .../src-tauri/gen/schemas/desktop-schema.json | 28 ++++ plugins/shell/api-iife.js | 2 +- plugins/shell/build.rs | 2 +- plugins/shell/guest-js/index.ts | 129 ++++++------------ .../autogenerated/commands/spawn.toml | 13 ++ .../permissions/autogenerated/reference.md | 2 + plugins/shell/permissions/schemas/schema.json | 14 ++ plugins/shell/src/commands.rs | 86 +++++++++++- plugins/shell/src/error.rs | 3 + plugins/shell/src/lib.rs | 1 + 12 files changed, 197 insertions(+), 94 deletions(-) create mode 100644 .changes/shell-command-execute-extra-new-lines.md create mode 100644 .changes/shell-command-execute-speed.md create mode 100644 plugins/shell/permissions/autogenerated/commands/spawn.toml diff --git a/.changes/shell-command-execute-extra-new-lines.md b/.changes/shell-command-execute-extra-new-lines.md new file mode 100644 index 000000000..c233a6d12 --- /dev/null +++ b/.changes/shell-command-execute-extra-new-lines.md @@ -0,0 +1,5 @@ +--- +"shell-js": "patch" +--- + +Fix `Command.execute` API including extra new lines. diff --git a/.changes/shell-command-execute-speed.md b/.changes/shell-command-execute-speed.md new file mode 100644 index 000000000..8e99b5960 --- /dev/null +++ b/.changes/shell-command-execute-speed.md @@ -0,0 +1,6 @@ +--- +"shell": "patch" +"shell-js": "patch" +--- + +Improve the speed of the JS `Command.execute` API diff --git a/examples/api/src-tauri/gen/schemas/desktop-schema.json b/examples/api/src-tauri/gen/schemas/desktop-schema.json index a19c5db8d..c00d481bd 100644 --- a/examples/api/src-tauri/gen/schemas/desktop-schema.json +++ b/examples/api/src-tauri/gen/schemas/desktop-schema.json @@ -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", @@ -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", @@ -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", @@ -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", diff --git a/plugins/shell/api-iife.js b/plugins/shell/api-iife.js index 971f25802..40c33e380 100644 --- a/plugins/shell/api-iife.js +++ b/plugins/shell/api-iife.js @@ -1 +1 @@ -if("__TAURI__"in window){var __TAURI_PLUGIN_SHELL__=function(t){"use strict";function e(t,e,s,n){if("a"===s&&!n)throw new TypeError("Private accessor was defined without a getter");if("function"==typeof e?t!==e||!n:!e.has(t))throw new TypeError("Cannot read private member from an object whose class did not declare it");return"m"===s?n:"a"===s?n.call(t):n?n.value:e.get(t)}function s(t,e,s,n,i){if("function"==typeof e?t!==e||!i:!e.has(t))throw new TypeError("Cannot write private member to an object whose class did not declare it");return e.set(t,s),s}var n,i,r;"function"==typeof SuppressedError&&SuppressedError;class o{constructor(){this.__TAURI_CHANNEL_MARKER__=!0,n.set(this,(()=>{})),i.set(this,0),r.set(this,{}),this.id=function(t,e=!1){return window.__TAURI_INTERNALS__.transformCallback(t,e)}((({message:t,id:o})=>{if(o===e(this,i,"f")){s(this,i,o+1),e(this,n,"f").call(this,t);const a=Object.keys(e(this,r,"f"));if(a.length>0){let t=o+1;for(const s of a.sort()){if(parseInt(s)!==t)break;{const i=e(this,r,"f")[s];delete e(this,r,"f")[s],e(this,n,"f").call(this,i),t+=1}}s(this,i,t)}}else e(this,r,"f")[o.toString()]=t}))}set onmessage(t){s(this,n,t)}get onmessage(){return e(this,n,"f")}toJSON(){return`__CHANNEL__:${this.id}`}}async function a(t,e={},s){return window.__TAURI_INTERNALS__.invoke(t,e,s)}n=new WeakMap,i=new WeakMap,r=new WeakMap;class h{constructor(){this.eventListeners=Object.create(null)}addListener(t,e){return this.on(t,e)}removeListener(t,e){return this.off(t,e)}on(t,e){return t in this.eventListeners?this.eventListeners[t].push(e):this.eventListeners[t]=[e],this}once(t,e){const s=n=>{this.removeListener(t,s),e(n)};return this.addListener(t,s)}off(t,e){return t in this.eventListeners&&(this.eventListeners[t]=this.eventListeners[t].filter((t=>t!==e))),this}removeAllListeners(t){return t?delete this.eventListeners[t]:this.eventListeners=Object.create(null),this}emit(t,e){if(t in this.eventListeners){const s=this.eventListeners[t];for(const t of s)t(e);return!0}return!1}listenerCount(t){return t in this.eventListeners?this.eventListeners[t].length:0}prependListener(t,e){return t in this.eventListeners?this.eventListeners[t].unshift(e):this.eventListeners[t]=[e],this}prependOnceListener(t,e){const s=n=>{this.removeListener(t,s),e(n)};return this.prependListener(t,s)}}class c{constructor(t){this.pid=t}async write(t){await a("plugin:shell|stdin_write",{pid:this.pid,buffer:"string"==typeof t?t:Array.from(t)})}async kill(){await a("plugin:shell|kill",{cmd:"killChild",pid:this.pid})}}class u extends h{constructor(t,e=[],s){super(),this.stdout=new h,this.stderr=new h,this.program=t,this.args="string"==typeof e?[e]:e,this.options=s??{}}static create(t,e=[],s){return new u(t,e,s)}static sidecar(t,e=[],s){const n=new u(t,e,s);return n.options.sidecar=!0,n}async spawn(){return await async function(t,e,s=[],n){"object"==typeof s&&Object.freeze(s);const i=new o;return i.onmessage=t,await a("plugin:shell|execute",{program:e,args:s,options:n,onEvent:i})}((t=>{switch(t.event){case"Error":this.emit("error",t.payload);break;case"Terminated":this.emit("close",t.payload);break;case"Stdout":this.stdout.emit("data",t.payload);break;case"Stderr":this.stderr.emit("data",t.payload)}}),this.program,this.args,this.options).then((t=>new c(t)))}async execute(){return await new Promise(((t,e)=>{this.on("error",e);const s=[],n=[];this.stdout.on("data",(t=>{s.push(t)})),this.stderr.on("data",(t=>{n.push(t)})),this.on("close",(e=>{t({code:e.code,signal:e.signal,stdout:this.collectOutput(s),stderr:this.collectOutput(n)})})),this.spawn().catch(e)}))}collectOutput(t){return"raw"===this.options.encoding?t.reduce(((t,e)=>new Uint8Array([...t,...e,10])),new Uint8Array):t.join("\n")}}return t.Child=c,t.Command=u,t.EventEmitter=h,t.open=async function(t,e){await a("plugin:shell|open",{path:t,with:e})},t}({});Object.defineProperty(window.__TAURI__,"shell",{value:__TAURI_PLUGIN_SHELL__})} +if("__TAURI__"in window){var __TAURI_PLUGIN_SHELL__=function(e){"use strict";function t(e,t,s,n){if("a"===s&&!n)throw new TypeError("Private accessor was defined without a getter");if("function"==typeof t?e!==t||!n:!t.has(e))throw new TypeError("Cannot read private member from an object whose class did not declare it");return"m"===s?n:"a"===s?n.call(e):n?n.value:t.get(e)}function s(e,t,s,n,i){if("function"==typeof t?e!==t||!i:!t.has(e))throw new TypeError("Cannot write private member to an object whose class did not declare it");return t.set(e,s),s}var n,i,r;"function"==typeof SuppressedError&&SuppressedError;class o{constructor(){this.__TAURI_CHANNEL_MARKER__=!0,n.set(this,(()=>{})),i.set(this,0),r.set(this,{}),this.id=function(e,t=!1){return window.__TAURI_INTERNALS__.transformCallback(e,t)}((({message:e,id:o})=>{if(o===t(this,i,"f")){s(this,i,o+1),t(this,n,"f").call(this,e);const a=Object.keys(t(this,r,"f"));if(a.length>0){let e=o+1;for(const s of a.sort()){if(parseInt(s)!==e)break;{const i=t(this,r,"f")[s];delete t(this,r,"f")[s],t(this,n,"f").call(this,i),e+=1}}s(this,i,e)}}else t(this,r,"f")[o.toString()]=e}))}set onmessage(e){s(this,n,e)}get onmessage(){return t(this,n,"f")}toJSON(){return`__CHANNEL__:${this.id}`}}async function a(e,t={},s){return window.__TAURI_INTERNALS__.invoke(e,t,s)}n=new WeakMap,i=new WeakMap,r=new WeakMap;class h{constructor(){this.eventListeners=Object.create(null)}addListener(e,t){return this.on(e,t)}removeListener(e,t){return this.off(e,t)}on(e,t){return e in this.eventListeners?this.eventListeners[e].push(t):this.eventListeners[e]=[t],this}once(e,t){const s=n=>{this.removeListener(e,s),t(n)};return this.addListener(e,s)}off(e,t){return e in this.eventListeners&&(this.eventListeners[e]=this.eventListeners[e].filter((e=>e!==t))),this}removeAllListeners(e){return e?delete this.eventListeners[e]:this.eventListeners=Object.create(null),this}emit(e,t){if(e in this.eventListeners){const s=this.eventListeners[e];for(const e of s)e(t);return!0}return!1}listenerCount(e){return e in this.eventListeners?this.eventListeners[e].length:0}prependListener(e,t){return e in this.eventListeners?this.eventListeners[e].unshift(t):this.eventListeners[e]=[t],this}prependOnceListener(e,t){const s=n=>{this.removeListener(e,s),t(n)};return this.prependListener(e,s)}}class c{constructor(e){this.pid=e}async write(e){await a("plugin:shell|stdin_write",{pid:this.pid,buffer:"string"==typeof e?e:Array.from(e)})}async kill(){await a("plugin:shell|kill",{cmd:"killChild",pid:this.pid})}}class l extends h{constructor(e,t=[],s){super(),this.stdout=new h,this.stderr=new h,this.program=e,this.args="string"==typeof t?[t]:t,this.options=s??{}}static create(e,t=[],s){return new l(e,t,s)}static sidecar(e,t=[],s){const n=new l(e,t,s);return n.options.sidecar=!0,n}async spawn(){const e=this.program,t=this.args,s=this.options;"object"==typeof t&&Object.freeze(t);const n=new o;return n.onmessage=e=>{switch(e.event){case"Error":this.emit("error",e.payload);break;case"Terminated":this.emit("close",e.payload);break;case"Stdout":this.stdout.emit("data",e.payload);break;case"Stderr":this.stderr.emit("data",e.payload)}},await a("plugin:shell|spawn",{program:e,args:t,options:s,onEvent:n}).then((e=>new c(e)))}async execute(){const e=this.program,t=this.args,s=this.options;return"object"==typeof t&&Object.freeze(t),await a("plugin:shell|execute",{program:e,args:t,options:s})}}return e.Child=c,e.Command=l,e.EventEmitter=h,e.open=async function(e,t){await a("plugin:shell|open",{path:e,with:t})},e}({});Object.defineProperty(window.__TAURI__,"shell",{value:__TAURI_PLUGIN_SHELL__})} diff --git a/plugins/shell/build.rs b/plugins/shell/build.rs index 6814fbbc7..6a7285700 100644 --- a/plugins/shell/build.rs +++ b/plugins/shell/build.rs @@ -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) diff --git a/plugins/shell/guest-js/index.ts b/plugins/shell/guest-js/index.ts index cd1aa94be..5772df649 100644 --- a/plugins/shell/guest-js/index.ts +++ b/plugins/shell/guest-js/index.ts @@ -99,39 +99,6 @@ interface ChildProcess { 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( - onEventHandler: (event: CommandEvent) => void, - program: string, - args: string | string[] = [], - options?: InternalSpawnOptions, -): Promise { - if (typeof args === "object") { - Object.freeze(args); - } - - const onEvent = new Channel>(); - onEvent.onmessage = onEventHandler; - - return await invoke("plugin:shell|execute", { - program, - args, - options, - onEvent, - }); -} - /** * @since 2.0.0 */ @@ -513,27 +480,38 @@ class Command extends EventEmitter { * @since 2.0.0 */ async spawn(): Promise { - return await execute( - (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>(); + 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("plugin:shell|spawn", { + program, + args, + options, + onEvent, + }).then((pid) => new Child(pid)); } /** @@ -553,40 +531,19 @@ class Command extends EventEmitter { * @since 2.0.0 */ async execute(): Promise> { - return await new Promise((resolve, reject) => { - this.on("error", reject); + const program = this.program; + const args = this.args; + const options = this.options; - 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); - }); - } - - /** @ignore */ - private collectOutput(events: O[]): string | Uint8Array { - if (this.options.encoding === "raw") { - return events.reduce((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>("plugin:shell|execute", { + program, + args, + options, + }); } } diff --git a/plugins/shell/permissions/autogenerated/commands/spawn.toml b/plugins/shell/permissions/autogenerated/commands/spawn.toml new file mode 100644 index 000000000..a3802d2a2 --- /dev/null +++ b/plugins/shell/permissions/autogenerated/commands/spawn.toml @@ -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"] diff --git a/plugins/shell/permissions/autogenerated/reference.md b/plugins/shell/permissions/autogenerated/reference.md index 27a2ace7b..8138c686f 100644 --- a/plugins/shell/permissions/autogenerated/reference.md +++ b/plugins/shell/permissions/autogenerated/reference.md @@ -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.| diff --git a/plugins/shell/permissions/schemas/schema.json b/plugins/shell/permissions/schemas/schema.json index 788f5fedd..3ad11a47d 100644 --- a/plugins/shell/permissions/schemas/schema.json +++ b/plugins/shell/permissions/schemas/schema.json @@ -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", diff --git a/plugins/shell/src/commands.rs b/plugins/shell/src/commands.rs index ab00a64eb..77aee6998 100644 --- a/plugins/shell/src/commands.rs +++ b/plugins/shell/src/commands.rs @@ -94,18 +94,15 @@ fn default_env() -> Option> { Some(HashMap::default()) } -#[allow(clippy::too_many_arguments)] -#[tauri::command] -pub fn execute( +#[inline(always)] +fn prepare_cmd( window: Window, - shell: State<'_, Shell>, program: String, args: ExecuteArgs, - on_event: Channel, options: CommandOptions, command_scope: CommandScope, global_scope: GlobalScope, -) -> crate::Result { +) -> crate::Result<(crate::process::Command, EncodingWrapper)> { let scope = crate::scope::ShellScope { scopes: command_scope .allows() @@ -151,6 +148,7 @@ pub fn execute( } else { command = command.env_clear(); } + let encoding = match options.encoding { Option::None => EncodingWrapper::Text(None), Some(encoding) => match encoding.as_str() { @@ -168,6 +166,82 @@ pub fn execute( }, }; + Ok((command, encoding)) +} + +#[derive(Serialize)] +enum Output { + String(String), + Raw(Vec), +} + +#[derive(Serialize)] +pub struct ChildProcessReturn { + code: Option, + signal: Option, + #[serde(flatten)] + stdout: Output, + #[serde(flatten)] + stderr: Output, +} + +#[allow(clippy::too_many_arguments)] +#[tauri::command] +pub fn execute( + window: Window, + program: String, + args: ExecuteArgs, + options: CommandOptions, + command_scope: CommandScope, + global_scope: GlobalScope, +) -> crate::Result { + 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( + window: Window, + shell: State<'_, Shell>, + program: String, + args: ExecuteArgs, + on_event: Channel, + options: CommandOptions, + command_scope: CommandScope, + global_scope: GlobalScope, +) -> crate::Result { + let (command, encoding) = + prepare_cmd(window, program, args, options, command_scope, global_scope)?; + let (mut rx, child) = command.spawn()?; let pid = child.pid(); diff --git a/plugins/shell/src/error.rs b/plugins/shell/src/error.rs index c8af2343b..99b13cfd3 100644 --- a/plugins/shell/src/error.rs +++ b/plugins/shell/src/error.rs @@ -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 { diff --git a/plugins/shell/src/lib.rs b/plugins/shell/src/lib.rs index 012d9e96e..1f92c6ccd 100644 --- a/plugins/shell/src/lib.rs +++ b/plugins/shell/src/lib.rs @@ -81,6 +81,7 @@ pub fn init() -> TauriPlugin> { .js_init_script(include_str!("init-iife.js").to_string()) .invoke_handler(tauri::generate_handler![ commands::execute, + commands::spawn, commands::stdin_write, commands::kill, commands::open