Lack of API mutating args at std::process::Command

Sure, the Command can be recreated from scratch, there are workarounds for this issue. Implementing 'Clone' would do the job too.

The point here is that 'Command' is already a builder and can be used for multiple instances with one of the execution triggering methods. The other attributes (Oops as I am writing this I haven't checked for Stdio) do have API's to refine them before firing one execution again, only the 'args' does not. I'd call this an inconsistency in the API.

IMO it would be valuable to add this, to my understanding it won't cost much and won't break anything, just making things more consistent and easier to use.

clear_args() seems like the simplest thing that could work. More than that goes onto a slippery slope of "why not < insert any random Vec method > too?"

That's why I put other API's up to discussion. A 'reset' function is the bare minimum required (i'd suggest reset instead clear, because it should preserve argv[0]).

Additionally I'd opt for the 'set' function that replaces an existing component by index (dropping or returning the old one). Because it is common that one wants to prepare a command to be executed and then run it over different filenames for example.

argv[0] is specified using Command::new(), and can't be set using .arg(), so I think it's fine to present an API as if the "args" meant argv[1..] only, so clear() clears [1..].

Set by index would raise question why not implement Index and command[n] = arg. And if there's set, why not remove, and if there's remove, why not insert, retain, and so on.

Question is answered by that the 'args' is not implemented as single Vec, nor are all the methods Vec offers needed in this case.

My aim is for an usable ergonomic API. Take a look at 'env' which has env_clear() and env_remove().

Having an 'args_clear()' that clears [1..] is Ok. (while sometimes one wants to chage argv[0]) but that is unrelated to this topic, if need arises this can be discussed again under another topic.

Looking at env, values can be updated with the env() and envs() method, since that is an associative store. For args it is very common that one wants to update an argument at a certain position. I am thinking this really deserves to have an API, not for efficiency (which is irrelevant when we talking about spawning processes), but for ergonomic reasons with something like:

let mut command = Command::new("dosomething").args(["FILE"]);
somefiles.iter().for_each(|file| command.arg_set(1, file)?.spawn());

There are certain use cases where one wants to remove or insert more arguments, but that may only fatten the API, in that case it is Ok to clean and rebuild the argv from scratch.

1 Like

Just some observation:

arg_set() should return a Result<Command, Error>

Which surprises me now since the env() functions don't return a Result but a Command. But there are certain ways which would fail setting an environment variable (illegal characters in its name). Is this intentional or a defect.

Any errors will be returned when you call spawn, etc.

I assume they aren't tested any earlier because it would have made the builder less ergonomic back before we had the ? operator.

Actually errors are not handled in that case: (testing on linux here)

fn main() {
    use std::process::Command;

    let out = Command::new("bash")
        .env("FOO=BAR", "BAZ")
        .args(["-c", "/bin/echo $FOO"])
        .output()
        .unwrap()
        .stdout;
    println!("Surprise: {}", String::from_utf8(out).unwrap());
}

this results in "BAR=BAZ" as if one did .env("FOO", "BAR=BAZ") ... which isnt unexpected when you know how the underlying representation in the libc is.....

1 Like

Ah, interesting! I assumed it would do some testing when building the variables but only a null check is done.

Now I am puzzled about how to fix that. Other OSes would certainly put other restrictions on characters a env var name can hold (and possibly different rules on the value as well). Changing the return into a Result<> would be a breaking change, nogo for the stdlib. Validating the env before spawning seems to be costly (ymmw). Possibly one could store the env validation state in Command, dirty it every time the env is mutated and validate it when dirty -> set it to Ok or Error on spawn time, returning Error perhaps.

See this thread for more on env validation.

Huh, Unix Rust seems to use a saw_null variable as a way to signal an error.

I think Command::env() things should follow whats there (PR) finalized. But I have no idea how to deal with a breaking change in rusts stdlib (feature?). Anyway this is going off-topic here (the 'args' thing). Shall we open a new thread on this forum about the 'env' issue or file a ticket straight away about a possible defect? I am the new kid on the block here and don't know the rust process on these things.

I think just mentioning that Command::env also care in that thread should be enough. Easier than splitting up the tasks associated with whatever comes out of it.

The Command env methods are implemented internally using the private construct CommandEnv (rust/process.rs at 456a03227e3c81a51631f87ec80cac301e5fa6d7 · rust-lang/rust · GitHub), which has methods like clear, set, remove. An alternative would be exposing CommandEnv and introducing an equivalent CommandArg (possibly with a clearer name like CommandEnvStore?).

For environment variables, remove makes sense because the variables are like a map, with key and value, and the variables do not rely on order. But arguments may depend on order: an argument can have an effect on the next arguments, so removing a single argument can be much more complicated than removing an environment variable.

In summary, while there are both env_clear() and env_remove(key), and it looks harmless to have a similar arg_clear(), a method arg_remove(index) looks very tricky and error prone.

Also, env_clear and env_remove are necessary because Command::new inherits the current process environment, while there is no such thing for arguments. So even adding arg_clear is not really equivalent to env_clear.

My saying, still clearing the argument list for recreating it would be the most minimal functionality needed and is lacking currently. I would like to have an API that allows the mutation of an existing argument as shown above, because that is very common.

About the env-mess I'd wait and see what the solution on Why should std::env::var panic? will look like finally and then follow that route. Note that I put a comment there about that env keys may get their distinct type, dunno how feasible that is (to me it boils down to "will rust want to make things correct even when it introduces a breaking change or staying bug by bug compatible").

1 Like

I have written many programs that needed to do things like this (not in Rust) and I would like to explicitly vote for the addition of .arg_set(index, string). It can easily be quite a bit hairier than @cehteh's example, e.g. from a Python script I wrote for work

proc = subprocess.Popen([BRO, "-b", "-C", BRO_SCRIPT,
                         "-r", input_file, "-w", "-"],
                        stdout=subprocess.PIPE, ...)

where the argument that changes for each file to be processed is third-to-last, and has to be there because of the way the process being run parses its arguments. In Python it's no big deal to create a scratch argument array for every invocation, but in Rust it would be quite a bit longer I think.

What would be the way to progress here (only the 'arg' part)? Opening an issue, Writing an RFC or sending a PR?

I haven't done such yet and don't know the process. I can offer to do the coding for Linux/Unix, but don't know Windows and other platforms. Would be nice if someone cold lend me a hand.