Issue 11650: Process::new and dynamic_library::open_external should take &[u8]s, not Paths or ~strs

[aturon]

Process API

The existing APIs for spawning processes took strings for the command
and arguments, but the underlying system may not impose utf8 encoding,
so this is overly limiting.

The assumption we actually want to make is just that the command and
arguments are viewable as [u8] slices with no interior NULLs, i.e., as
CStrings. The ToCStr trait is a handy bound for types that meet this
requirement (such as &str and Path).

However, since the commands and arguments are often a mixture of
strings and paths, it would be inconvenient to take a slice with a
single T: ToCStr bound. So this patch revamps the process creation API
to instead use a builder-style interface, called Command, allowing
arguments to be added one at a time with differing ToCStr
implementations for each.

The initial cut of the builder API has some drawbacks that can be
addressed once issue #13851 (libstd as a facade) is closed. These are
detailed as FIXMEs.

Dynamic library API

std::unstable::dynamic_library::open_external currently takes a
Path, but because Paths produce normalized strings, this can
change the semantics of lookups in a given environment. This patch
generalizes the function to take a ToCStr-bounded type, which
includes both Paths and strs.

ToCStr API

Adds ToCStr impl for &Path and ~str. This is a stopgap until DST (#12938) lands.

Until DST lands, we cannot decompose &str into & and str, so we cannot
usefully take ToCStr arguments by reference (without forcing an
additional & around &str). So we are instead temporarily adding an
instance for &Path and ~str, so that we can take ToCStr as owned. When
DST lands, the &Path instance should be removed, the string instances
should be revisted, and arguments bound by ToCStr should be passed by
reference.

FIXMEs have been added accordingly.

Tickets closed

Closes #11650.
Closes #7928.

[alexcrichton]

Awesome patch, this looks better than I thought it would!

[alexcrichton]

cc @carlhuda, @wycats, @carllerche - I've seen tons of process stuff in cargo, you may be interested in this change, and have comments of your own!

[liigo]

ToCStr VS. AsciiStr ?
2014年5月6日 上午8:59于 "Alex Crichton" notifications@github.com写道：

Awesome patch, this looks better than I thought it would!

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/13954#issuecomment-42258167
.

[aturon]

@alexcrichton Pushed new patches addressing all the issues you pointed out. In addition, I realized that the Command builder interface can actually work entirely with borrowed pointers, which means that we don't need convenience methods like push_arg. Luckily, given the current rules for lifetimes of temporaries, patterns like

Command::new("echo").arg("hello").spawn()

still work just fine. For bigger uses of builders, you have to do:

let mut cmd = Command::new("echo");
cmd.arg("hello");
cmd.arg("world");
cmd.spawn();

which we were generally doing in places like link.rs anyway.

Also, note that io::process::Command now exposes no fields and serializes into rtio::ProcessConfig.

[aturon]

@liigo We want ToCStr here: valid program paths and arguments may be any array of bytes (like a [u8] slice) with no interior nulls. See issue #11650, and for example this post on stackexchange

[liigo]

Perhaps ToCStr is a little wired in a context out of ffi.
2014年5月7日 上午11:22于 "Aaron Turon" notifications@github.com写道：

@liigo https://github.com/liigo We want ToCStr here: valid program
paths and arguments may be any array of bytes (like a [u8] slice) with no
interior nulls. See issue #11650#11650,
and for example this post on stackexchangehttp://unix.stackexchange.com/questions/2089/what-charset-encoding-is-used-for-filenames-and-paths-on-linux

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/13954#issuecomment-42385875
.

[alexcrichton]

(travis failure and needs a rebase)

[aturon]

@liigo That's a fair point. But what we want here is really something like: something viewable as a &[u8] containing no 0 values. Right now, I think ToCStr is the best type to represent that constraint, but this could be revisited later on.

[liigo]

That's OK
2014年5月8日 上午3:06于 "Aaron Turon" notifications@github.com写道：

@liigo https://github.com/liigo That's a fair point. But what we want
here is really something like: something viewable as a &[u8] containing no
0 values. Right now, I think ToCStr is the best type to represent that
constraint, but this could be revisited later on.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/13954#issuecomment-42469423
.

[alexcrichton]

\o/

[pnkfelix]

hmm this is almost certainly going to require me to rebase #14000. But that's okay. :)

[tamird]

@aturon this comment is still in the codebase, even though ToCStr is gone and to_os_str is an inherent method on Path. Should this comment just be removed?

[tamird]

cc @alexcrichton

[alexcrichton]

Yeah this can just be removed now, we've since dealt with the underlying issue in one way or another!