The new std.process is ready for review

[Vladimir Panteleev]

On Sunday, 24 February 2013 at 14:43:50 UTC, Lars T. Kyllingstad 
wrote:
> [snip]

Hi Lars,

First of all, about environment. I think the old behavior makes 
more sense.

I think you had a good point about making it behave like an 
associative array. I would expect using opIndex with an 
inexisting key to throw. Subtle deviations of behavior for types 
that generally behave like well-known types can introduce latent 
bugs.

The danger is even more potent in the case of environment 
variables, as those are often used for constructing command-lines 
and such. If attempting to get the value of an inexisting 
variable now returns null, which is used to build a command line, 
unexpected things can happen.

For example, let's say that you're writing a program for 
analyzing malware, which expects $BINDUMP to be set to the path 
of some analysis tool. So it runs environment["BINDUMP"] ~ 
args[1] - however, if BINDUMP is unset, the program runs the 
malware executable itself.

For another example, here's this classic catastrophic bug in 
shell scripts:

rm -rf $FOO/$BAR

What happens if $FOO and $BAR are unset?

One thing that I think is missing from the environment object is 
opIn_r. Implementing opIn_r would allow users to more safely 
explicitly check if a variable is set or not, and is more 
readable than environment.get("FOO", null).

And of course, there's the issue of people migrating code from 
the old module version to the new one: if they relied on the old 
behavior, the code can break in unexpected ways after the 
migration.

What are your specific reasons for changing environment's 
behavior?

Speaking of shells, I noticed you hardcode cmd.exe in 
std.process2. That's another bug, it should look at the COMSPEC 
variable.

Also, about the shell function, I noticed this recent bug report:
http://d.puremagic.com/issues/show_bug.cgi?id=9444

Maybe it somehow makes the transition to the new function easier? 
:)

If not, since you're adamant about not changing the name, can we 
overload the function (e.g. make the new one return some results 
in "out" parameters), and deprecate the original overload?

Finally, I'd just like to sum up that we seem to have two 
decisions on the scales: somehow solving the API 
incompatibilities, or introducing the new version as an entirely 
new module. The latter is a mess we really don't want to get 
into, so it'd need to be justified, and IMHO the 
incompatibilities don't seem to be as severe and unresolvable to 
warrant that mess.