Hello,

I have decided to take on Review Manager for std.process and hopefully will keep it up and get through the review queue. This is slightly ignoring a "first come, first serve" approach, but that is mostly because I didn't want to look into it. (So don't be too upset).

std.process has been under review for some time having discussion on the pull request and this thread:
http://forum.dlang.org/thread/stxxtfwfrwllkcpunhue@forum.dlang.org

I would like to propose the Formal review is one week (and one week voting). If there is objection speak up.

Docs:
http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process.html

Source:
https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d

Pull:
https://github.com/D-Programming-Language/phobos/pull/1151

Oops, forgot the project description:

std.process is improvements to the existing std.process and is a complete change to the API. The original API remains but these will be going through deprecation.

The major change is support for redirecting stdin/stdout/stderr

For those interested I took the unittests from the original std.process and modified them to the new API (back when it was under std.process2) so this diff gives an idea for changes (not done for Windows sorry).

https://gist.github.com/JesseKPhillips/5298688/revisions

On 4/4/13, Jesse Phillips <Jesse.K.Phillips+D@gmail.com> wrote:
> Source: https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d

Dead link.

> Docs: http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process.html

<quote>
Unless a directory is specified in args[0] or program, spawnProcess
will search for the program in the directories listed in the PATH
environment variable. To run an executable in the current directory,
use "./executable_name".
</quote>

This does not apply for Windows, so it should state that.

On 4/4/13, Jesse Phillips <Jesse.K.Phillips+D@gmail.com> wrote:
> Docs: http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process.html

<quote>
Win32-specific warning: The mechanisms for process termination are
incredibly badly specified in Win32. This function may therefore
produce unexpected results, and should be used with the utmost care.
</quote>

It then links to a note about XP. So does this only apply to XP or all Windows systems? "win32" isn't very specific, does it not apply for 64bit systems?

Actual Code location.

Source:
https://github.com/kyllingstad/phobos/blob/std-process2/std/process.d

On Thu, 04 Apr 2013 12:50:01 -0400, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:

> On 4/4/13, Jesse Phillips <Jesse.K.Phillips+D@gmail.com> wrote:
>> Source:
>> https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
>
> Dead link.

https://github.com/kyllingstad/phobos/blob/std-process2/std/process.d

At the last minute I insisted we change std.process2 to std.process since it was agreed to incorporate the original API instead of redesigning it slightly.

-Steve

On Thu, Apr 04, 2013 at 01:04:52PM -0400, Steven Schveighoffer wrote:
> On Thu, 04 Apr 2013 12:50:01 -0400, Andrej Mitrovic <andrej.mitrovich@gmail.com> wrote:
> 
> >On 4/4/13, Jesse Phillips <Jesse.K.Phillips+D@gmail.com> wrote:
> >>Source: https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
> >
> >Dead link.
> 
> https://github.com/kyllingstad/phobos/blob/std-process2/std/process.d
> 
> At the last minute I insisted we change std.process2 to std.process since it was agreed to incorporate the original API instead of redesigning it slightly.
[...]

Hooray! Looking forward to *not* having "std.process2". Didn't like that name from the start. I'll take a closer look at the code/docs later, for the review.


T

-- 
Государство делает вид, что платит нам зарплату, а мы делаем вид, что работаем.

On Thursday, 4 April 2013 at 16:50:15 UTC, Andrej Mitrovic wrote:
> <quote>
> Unless a directory is specified in args[0] or program, spawnProcess
> will search for the program in the directories listed in the PATH
> environment variable. To run an executable in the current directory,
> use "./executable_name".
> </quote>
>
> This does not apply for Windows, so it should state that.

It does apply to Windows.  It just isn't necessary to do it within std.process, as Windows' own CreateProcess() takes care of it.

It is a bit imprecise, though, because CreateProcess() does in fact search a lot of other directories first (current working dir, parent process dir, system32, etc.).  In fact, PATH is last on that list. ;)  I'll clarify this in the documentation.

Lars

On Thursday, 4 April 2013 at 16:57:58 UTC, Andrej Mitrovic wrote:
> On 4/4/13, Jesse Phillips <Jesse.K.Phillips+D@gmail.com> wrote:
>> Docs:
>> http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process.html
>
> <quote>
> Win32-specific warning: The mechanisms for process termination are
> incredibly badly specified in Win32. This function may therefore
> produce unexpected results, and should be used with the utmost care.
> </quote>
>
> It then links to a note about XP. So does this only apply to XP or all
> Windows systems? "win32" isn't very specific, does it not apply for
> 64bit systems?

Did you read the article? In the second paragraph it says:

"Many of the details of how processes exit are left unspecified in Win32, so different Win32 implementations can follow different mechanisms. For example, Win32s, Windows 95, and Windows NT all shut down processes differently."

BTW, this is written by one of Microsoft's Windows developers, so I'm assuming it to be authoritative.

I don't know whether it applies to Win64, though.  I've tried to investigate, but I couldn't find anything.  I agree the docs should probably be changed to "Windows APIs" rather than "Win32".

Lars

On 4/4/13, Lars T. Kyllingstad <public@kyllingen.net> wrote:
> Did you read the article?

I did some years ago.

There seem to be /some/ workarounds in various places on the web, with alternatives to TerminateProcess, but it's usually hacky.

I don't see this as a blocker personally, we can investigate for a safer implementation later.