From a2fbdd338588a9574e1506db7e61ea360d2e08f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Fri, 23 Jul 2021 19:16:46 +0200 Subject: [PATCH 1/7] implement `std::process::Command::args_clear()` see #87379 Only unix implementation yet. --- library/std/src/lib.rs | 1 + library/std/src/process.rs | 32 +++++++++++++++++++ library/std/src/process/tests.rs | 26 +++++++++++++++ .../src/sys/unix/process/process_common.rs | 9 ++++++ 4 files changed, 68 insertions(+) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 67846e7883570..6d612abcc98a2 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -306,6 +306,7 @@ #![feature(min_specialization)] #![feature(mixed_integer_ops)] #![feature(must_not_suspend)] +#![feature(mutate_command_args)] #![feature(needs_panic_runtime)] #![feature(negative_impls)] #![feature(never_type)] diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 4e9fd51f28229..7e39952ffc44f 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -648,6 +648,38 @@ impl Command { self } + /// Clears the argument array. + /// + /// When one wants to restart a Command again with different + /// arguments the argument list can to be cleared first. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ```no_run + /// #![feature(mutate_command_args)] + /// use std::process::Command; + /// + /// let mut lsdir = Command::new("ls"); + /// + /// lsdir + /// .arg("target") + /// .spawn() + /// .expect("ls command failed to start"); + /// + /// lsdir + /// .args_clear() + /// .arg("tests") + /// .spawn() + /// .expect("ls command failed to start"); + /// ``` + #[unstable(feature = "mutate_command_args", issue = "87379")] + pub fn args_clear(&mut self) -> &mut Command { + self.inner.args_clear(); + self + } + /// Inserts or updates an environment variable mapping. /// /// Note that environment variable names are case-insensitive (but case-preserving) on Windows, diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index 67b747e410732..838ec8a12c40a 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -211,6 +211,32 @@ fn test_wait_with_output_once() { assert_eq!(stderr, Vec::new()); } +#[test] +fn test_args_clear() { + let mut prog = Command::new("echo"); + + if cfg!(target_os = "windows") { + prog.args(&["/C", "echo reset_me"]) + } else { + prog.arg("reset_me") + }; + + prog.args_clear(); + + if cfg!(target_os = "windows") { + prog.args(&["/C", "echo hello"]); + } else { + prog.arg("hello"); + }; + + let Output { status, stdout, stderr } = prog.output().unwrap(); + let output_str = str::from_utf8(&stdout).unwrap(); + + assert!(status.success()); + assert_eq!(output_str.trim().to_string(), "hello"); + assert_eq!(stderr, Vec::new()); +} + #[cfg(all(unix, not(target_os = "android")))] pub fn env_cmd() -> Command { Command::new("env") diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index 7ac2f9d8af75a..daf787de0b5ff 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -190,6 +190,15 @@ impl Command { self.args.push(arg); } + pub fn args_clear(&mut self) { + // resize `argv` to 2 (argv[0] and NULL). + self.argv.0.truncate(2); + self.argv.0[1] = ptr::null(); + + // drop all excess args. + self.args.truncate(1); + } + pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(os2c(dir, &mut self.saw_nul)); } From c420fdc64a12bcff66e45b4131866a7ae9ade0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sat, 24 Jul 2021 02:52:13 +0200 Subject: [PATCH 2/7] implement `std::process::Command::arg_set()` Only unix implementation yet. --- library/std/src/process.rs | 34 ++++++++++++++++++ library/std/src/process/tests.rs | 36 +++++++++++++++++++ .../src/sys/unix/process/process_common.rs | 7 ++++ 3 files changed, 77 insertions(+) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 7e39952ffc44f..ac608cb0a67a4 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -680,6 +680,40 @@ impl Command { self } + /// Sets the argument at index to a new value. + /// + /// When one wants to restart a Command again with different + /// arguments the argument list can to be cleared first. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ```no_run + /// #![feature(mutate_command_args)] + /// use std::process::Command; + /// + /// // Prepare a command + /// let mut command = Command::new("ls"); + /// command.args(["-l", "-a", "FILE"]); + /// + /// // Run it with mutated 3rd parameter + /// ["foo", "bar", "baz"].iter() + /// .for_each( + /// |file| { + /// command + /// .arg_set(3, file) + /// .spawn() + /// .unwrap(); + /// } + /// ); + /// ``` + #[unstable(feature = "mutate_command_args", issue = "87379")] + pub fn arg_set<S: AsRef<OsStr>>(&mut self, index: usize, value: S) -> &mut Command { + self.inner.arg_set(index, value.as_ref()); + self + } + /// Inserts or updates an environment variable mapping. /// /// Note that environment variable names are case-insensitive (but case-preserving) on Windows, diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index 838ec8a12c40a..f46b3cfc7730e 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -237,6 +237,42 @@ fn test_args_clear() { assert_eq!(stderr, Vec::new()); } +#[test] +fn test_arg_set() { + let mut prog = Command::new("echo"); + + if cfg!(target_os = "windows") { + prog.args(&["/C", "echo set_me"]) + } else { + prog.arg("set_me") + }; + + if cfg!(target_os = "windows") { + prog.arg_set(2, "echo hello"); + } else { + prog.arg_set(1, "hello"); + }; + + let Output { status, stdout, stderr } = prog.output().unwrap(); + let output_str = str::from_utf8(&stdout).unwrap(); + + assert!(status.success()); + assert_eq!(output_str.trim().to_string(), "hello"); + assert_eq!(stderr, Vec::new()); +} + +#[test] +#[should_panic] +fn test_arg_set_fail() { + let mut prog = Command::new("echo"); + + if cfg!(target_os = "windows") { + prog.arg_set(1, "echo hello"); + } else { + prog.arg_set(1, "hello"); + }; +} + #[cfg(all(unix, not(target_os = "android")))] pub fn env_cmd() -> Command { Command::new("env") diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index daf787de0b5ff..d0568ff38eaef 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -199,6 +199,13 @@ impl Command { self.args.truncate(1); } + pub fn arg_set(&mut self, index: usize, arg: &OsStr) { + debug_assert!(index >= 1 && index < self.args.len(), "Index out of range"); + let arg = os2c(arg, &mut self.saw_nul); + self.argv.0[index] = arg.as_ptr(); + self.args[index] = arg; + } + pub fn cwd(&mut self, dir: &OsStr) { self.cwd = Some(os2c(dir, &mut self.saw_nul)); } From 6a10a48e17ed086752efd9a5c3cdeebc18d36dac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sat, 24 Jul 2021 03:07:52 +0200 Subject: [PATCH 3/7] implement `std::process::Command::args_new()` --- library/std/src/process.rs | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index ac608cb0a67a4..0d984203edf53 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -714,6 +714,48 @@ impl Command { self } + /// Reset the argument list to new content + /// + /// Allows to maintain the argument list in a Vec or anything else that can provide + /// a suitable iterator. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ```no_run + /// #![feature(mutate_command_args)] + /// use std::process::Command; + /// + /// // Prepare a command + /// let mut command = Command::new("ls"); + /// let mut my_args = Vec::from(["-l", "foo"]); + /// + /// command + /// .args_new(&my_args) + /// .spawn() + /// .unwrap(); + /// + /// // Mutate my_args + /// my_args.insert(1, "-a"); + /// my_args.pop(); + /// my_args.push("bar"); + /// + /// // Run command again with the mutated arguments + /// command + /// .args_new(&my_args) + /// .spawn() + /// .unwrap(); + /// ``` + #[unstable(feature = "mutate_command_args", issue = "87379")] + pub fn args_new<I, S>(&mut self, args: I) -> &mut Command + where + I: IntoIterator<Item = S>, + S: AsRef<OsStr>, + { + self.args_clear().args(args) + } + /// Inserts or updates an environment variable mapping. /// /// Note that environment variable names are case-insensitive (but case-preserving) on Windows, From 816e4ec6a569eea023fc4e70fb61095770b21810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sat, 24 Jul 2021 03:33:21 +0200 Subject: [PATCH 4/7] FIX: assert >= 1 in process::command::arg_set() We do not want to set argv[0] here. This needs to be asserted at any time. Assertion for the index within range is only a debug_assert becasue its a mere helpful error message, indexing later would assert on out of range access anyway. --- library/std/src/sys/unix/process/process_common.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/process/process_common.rs b/library/std/src/sys/unix/process/process_common.rs index d0568ff38eaef..62e60c26e7a17 100644 --- a/library/std/src/sys/unix/process/process_common.rs +++ b/library/std/src/sys/unix/process/process_common.rs @@ -200,7 +200,8 @@ impl Command { } pub fn arg_set(&mut self, index: usize, arg: &OsStr) { - debug_assert!(index >= 1 && index < self.args.len(), "Index out of range"); + assert!(index >= 1, "Index must be >= 1"); + debug_assert!(index < self.args.len(), "Index out of range"); let arg = os2c(arg, &mut self.saw_nul); self.argv.0[index] = arg.as_ptr(); self.args[index] = arg; From ca62a9754eb50bb5aa106b3f8dbb3b55440d2bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sat, 24 Jul 2021 03:36:38 +0200 Subject: [PATCH 5/7] Provide the windows implementation of args_clear() and arg_set() --- library/std/src/sys/windows/process.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 66b210ce1bfb3..bf3dad8d0086b 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -208,6 +208,14 @@ impl Command { pub fn arg(&mut self, arg: &OsStr) { self.args.push(Arg::Regular(arg.to_os_string())) } + pub fn args_clear(&mut self) { + self.args.truncate(1); + } + pub fn arg_set(&mut self, index: usize, arg: &OsStr) { + assert!(index >= 1, "Index must be >= 1"); + debug_assert!(index < self.args.len(), "Index out of range"); + self.args[index] = Arg::Regular(arg.to_os_string()); + } pub fn env_mut(&mut self) -> &mut CommandEnv { &mut self.env } From 9d69f08572d34d65734e3aff72f0e9babfb9d6f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sat, 24 Jul 2021 04:13:58 +0200 Subject: [PATCH 6/7] DOCFIX: arg_set() lacked doc/panics --- library/std/src/process.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 0d984203edf53..6a9ced9d73417 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -682,8 +682,11 @@ impl Command { /// Sets the argument at index to a new value. /// - /// When one wants to restart a Command again with different - /// arguments the argument list can to be cleared first. + /// An existing argument at the given index will be replaced with a new value. + /// + /// # Panics + /// + /// May panic if the index is 0 or out of bounds. /// /// # Examples /// From 45d17805195994d9471653258a86ba94a89c9a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= <ct@pipapo.org> Date: Sun, 24 Oct 2021 17:14:56 +0200 Subject: [PATCH 7/7] Revert "implement `std::process::Command::args_new()`" This reverts commit 626d0e71b1e82c4804cec1a2bd03c3c53e4368ad. * removes 'args_new()' as requested in https://github.com/rust-lang/rust/pull/87420#issuecomment-933123951 * still disallow argv0 mutation as I explained in https://github.com/rust-lang/rust/pull/87420#issuecomment-950275301 --- library/std/src/process.rs | 42 -------------------------------------- 1 file changed, 42 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 6a9ced9d73417..a629a94d496a2 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -717,48 +717,6 @@ impl Command { self } - /// Reset the argument list to new content - /// - /// Allows to maintain the argument list in a Vec or anything else that can provide - /// a suitable iterator. - /// - /// # Examples - /// - /// Basic usage: - /// - /// ```no_run - /// #![feature(mutate_command_args)] - /// use std::process::Command; - /// - /// // Prepare a command - /// let mut command = Command::new("ls"); - /// let mut my_args = Vec::from(["-l", "foo"]); - /// - /// command - /// .args_new(&my_args) - /// .spawn() - /// .unwrap(); - /// - /// // Mutate my_args - /// my_args.insert(1, "-a"); - /// my_args.pop(); - /// my_args.push("bar"); - /// - /// // Run command again with the mutated arguments - /// command - /// .args_new(&my_args) - /// .spawn() - /// .unwrap(); - /// ``` - #[unstable(feature = "mutate_command_args", issue = "87379")] - pub fn args_new<I, S>(&mut self, args: I) -> &mut Command - where - I: IntoIterator<Item = S>, - S: AsRef<OsStr>, - { - self.args_clear().args(args) - } - /// Inserts or updates an environment variable mapping. /// /// Note that environment variable names are case-insensitive (but case-preserving) on Windows,