It's been years in the coming, but we finally got it done. :)  The upshot is that the module has actually seen active use over those years, both by yours truly and others, so hopefully the worst wrinkles are already ironed out.

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

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

Documentation:
http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process2.html

I hope we can get it reviewed in time for the next release.  (The wiki page indicates that both std.benchmark and std.uni are currently being reviewed, but I fail to find any "official" review threads on the forum.  Is the wiki just out of date?)

Lars

23-Feb-2013 15:31, Lars T. Kyllingstad пишет:
> It's been years in the coming, but we finally got it done. :) The upshot
> is that the module has actually seen active use over those years, both
> by yours truly and others, so hopefully the worst wrinkles are already
> ironed out.
>
> Pull request:
> https://github.com/D-Programming-Language/phobos/pull/1151
>
> Code:
> https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
>
> Documentation:
> http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process2.html
>
>
> I hope we can get it reviewed in time for the next release.  (The wiki
> page indicates that both std.benchmark and std.uni are currently being
> reviewed, but I fail to find any "official" review threads on the
> forum.  Is the wiki just out of date?)

Cool. I was about to suggest that now, since release is out we can hopefully start the review process.

As far as std.uni & std.benchmark goes:

Some time ago std.benchmark was again collectively destroyed and/or Andrei hadn't enough time to incorporate feedback. This was back in 2012 IRC. This is the last thread I can dig up:
http://forum.dlang.org/thread/mailman.73.1347916419.5162.digitalmars-d@puremagic.com

Then (in 2013) std.uni seen an informal destruction, had its docs re-written and was proposed again. Not much comments received after that, as people were fighting over @property stuff.

This is the first thread about it:
http://forum.dlang.org/thread/kcppa1$30b9$1@digitalmars.com

The second one:
http://forum.dlang.org/thread/ke9gat$rg0$1@digitalmars.com

I'd suggest we start with anything that's ready and the sooner the better. Now with std.process proposed for review I'm aware of 2 modules being ready, any others?

-- 
Dmitry Olshansky

On Sat, Feb 23, 2013 at 12:31:19PM +0100, Lars T. Kyllingstad wrote:
> It's been years in the coming, but we finally got it done. :)  The upshot is that the module has actually seen active use over those years, both by yours truly and others, so hopefully the worst wrinkles are already ironed out.

Finally!!! *applause*


> Pull request: https://github.com/D-Programming-Language/phobos/pull/1151
> 
> Code: https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
> 
> Documentation: http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process2.html

I just looked over the docs. Looks very good!

Just a few minor comments:

- wait():
   - Some code examples would be nice.

   - For the POSIX-specific version, I thought the Posix standard
     specifies that the actual return code / signal number should be
     extracted by means of system-specific macros (in C anyway)?
     Wouldn't it be better to encapsulate this in a POD struct or
     something instead of exposing the implementation-specific values to
     the user?

   - How do I wait for *any* child process to terminate, not just a
     specific Pid?

- execute() and shell(): I'm a bit concerned about returning the
  *entire* output of a process as a string. What if the output generates
  too much output to store in a string? Would it be better to return a
  range instead (either a range of chars or range of lines maybe)? Or is
  this what pipeProcess was intended for? In any case, would it make
  sense to specify some kind of upper limit to the size of the output so
  that the program won't be vulnerable to bad subprocess behaviour
  (generate infinite output, etc.)?

- ProcessException: are there any specific methods to help user code
  extract information about the error? Or is the user expected to check
  errno himself (on Posix; or whatever it is on Windows)?


> I hope we can get it reviewed in time for the next release.  (The wiki page indicates that both std.benchmark and std.uni are currently being reviewed, but I fail to find any "official" review threads on the forum.  Is the wiki just out of date?)
[...]

I was *intending* to re-review std.uni, it's been sitting in my inbox for a few weeks already, but alas, I keep getting distracted by other things. I'll see if I can get to it today. Sighh... so many fun things to do, so little time...


T

-- 
If I were two-faced, would I be wearing this one? -- Abraham Lincoln

On Sat, 23 Feb 2013 11:42:26 -0500, H. S. Teoh <hsteoh@quickfur.ath.cx> wrote:

> - wait():
>    - Some code examples would be nice.
>
>    - For the POSIX-specific version, I thought the Posix standard
>      specifies that the actual return code / signal number should be
>      extracted by means of system-specific macros (in C anyway)?
>      Wouldn't it be better to encapsulate this in a POD struct or
>      something instead of exposing the implementation-specific values to
>      the user?

We handle the extraction as an implementation detail, the result should be cross-platform (at least on signal-using platforms).  I don't know what a POD struct would get you, maybe you could elaborate what you mean?

>
>    - How do I wait for *any* child process to terminate, not just a
>      specific Pid?

I don't think we have a method to do that.  It would be complex, especially if posix wait() returned a pid that we are not handling!

I suppose what you could do is call posix wait (I have a feeling we may need to change our global wait function, or eliminate it), and then map the result back to a Pid you are tracking.

You have any ideas how this could be implemented?  I'd prefer not to keep a global cache of child process objects...

> - execute() and shell(): I'm a bit concerned about returning the
>   *entire* output of a process as a string. What if the output generates
>   too much output to store in a string? Would it be better to return a
>   range instead (either a range of chars or range of lines maybe)? Or is
>   this what pipeProcess was intended for? In any case, would it make
>   sense to specify some kind of upper limit to the size of the output so
>   that the program won't be vulnerable to bad subprocess behaviour
>   (generate infinite output, etc.)?

Yes, pipeProcess gives you File objects for each of the streams for those cases where you expect lots of data to be returned, or want to process it as it comes.  This is the use case I expect most people will use.

There is no doubt good use cases for execute/shell, we have a lot of non-generic string processing functions in phobos, and a lot of command line tools on an OS produce a concise output that can be used.

In general, for input streams, ranges are not a good interface.  Output ranges are good for output though, and I think File is a valid output range.

> - ProcessException: are there any specific methods to help user code
>   extract information about the error? Or is the user expected to check
>   errno himself (on Posix; or whatever it is on Windows)?

This is a good idea.  Right now, ProcessException converts the errno to a string message, but we could easily store the errno.

I say we, but I really mean Lars, he has done almost all the work :)

-Steve

On Sat, Feb 23, 2013 at 03:15:26PM -0500, Steven Schveighoffer wrote:
> On Sat, 23 Feb 2013 11:42:26 -0500, H. S. Teoh <hsteoh@quickfur.ath.cx> wrote:
> 
> >- wait():
> >   - Some code examples would be nice.
> >
> >   - For the POSIX-specific version, I thought the Posix standard
> >     specifies that the actual return code / signal number should be
> >     extracted by means of system-specific macros (in C anyway)?
> >     Wouldn't it be better to encapsulate this in a POD struct or
> >     something instead of exposing the implementation-specific values
> >     to the user?
> 
> We handle the extraction as an implementation detail, the result should be cross-platform (at least on signal-using platforms).  I don't know what a POD struct would get you, maybe you could elaborate what you mean?

Oh, I thought the return value was just straight from the syscall, which requires WIFEXITED, WEXITSTATUS, WCOREDUMP, etc., to interpret. If it has already been suitably interpreted in std.process, then I guess it's OK.

Otherwise, I was thinking of encapsulating these macros in some kind of POD struct, that provides methods like .ifExited, .exitStatus, .coreDump, etc. so that the user code doesn't have to directly play with the exact values returned by the specific OS.


> >   - How do I wait for *any* child process to terminate, not just a
> >     specific Pid?
> 
> I don't think we have a method to do that.  It would be complex, especially if posix wait() returned a pid that we are not handling!
> 
> I suppose what you could do is call posix wait (I have a feeling we may need to change our global wait function, or eliminate it), and then map the result back to a Pid you are tracking.
> 
> You have any ideas how this could be implemented?  I'd prefer not to keep a global cache of child process objects...

Why not?  On Posix at least, you get SIGCHLD, etc., for all child processes anyway, so a global cache doesn't seem to be out-of-place.

But you do have a point about pids that we aren't managing, e.g. if the user code is doing some fork()s on its own. But the way I see it, std.process is supposed to alleviate the need to do such things directly, so in my mind, if everything is going through std.process anyway, might as well just manage all child processes there. OTOH, this may cause problems if the D program links in C/C++ libraries that manage their own child processes.

Still, it would be nice to have some way of waiting for a set of child Pids, not just a single one. It would be a pain if user code had to manually manage child processes all the time when there's more than one of them running at a time.

Hmm. The more I think about it, the more it makes sense to just have std.process manage all child process related stuff. It's too painful to deal with multiple child processes otherwise. Maybe provide an opt-out in case you need to link in some C/C++ libraries that need their own child process handling, but the default, IMO, should be to manage everything through std.process.


> >- execute() and shell(): I'm a bit concerned about returning the
> >  *entire* output of a process as a string. What if the output
> >  generates too much output to store in a string? Would it be better
> >  to return a range instead (either a range of chars or range of
> >  lines maybe)? Or is this what pipeProcess was intended for? In any
> >  case, would it make sense to specify some kind of upper limit to
> >  the size of the output so that the program won't be vulnerable to
> >  bad subprocess behaviour (generate infinite output, etc.)?
> 
> Yes, pipeProcess gives you File objects for each of the streams for those cases where you expect lots of data to be returned, or want to process it as it comes.  This is the use case I expect most people will use.
> 
> There is no doubt good use cases for execute/shell, we have a lot of non-generic string processing functions in phobos, and a lot of command line tools on an OS produce a concise output that can be used.

True.


> In general, for input streams, ranges are not a good interface. Output ranges are good for output though, and I think File is a valid output range.

True.


> >- ProcessException: are there any specific methods to help user code
> >  extract information about the error? Or is the user expected to
> >  check errno himself (on Posix; or whatever it is on Windows)?
> 
> This is a good idea.  Right now, ProcessException converts the errno to a string message, but we could easily store the errno.
> 
> I say we, but I really mean Lars, he has done almost all the work :)
[...]

I never liked the design of errno in C... its being a global makes keeping track of errors a pain. It would be nice if the value of errno were saved in the Exception object at the time the error was encountered, instead of arbitrary amounts of code after, which may have changed its value.


T

-- 
GEEK = Gatherer of Extremely Enlightening Knowledge

On Sat, 23 Feb 2013 17:46:04 -0500, H. S. Teoh <hsteoh@quickfur.ath.cx> wrote:

> On Sat, Feb 23, 2013 at 03:15:26PM -0500, Steven Schveighoffer wrote:
>> On Sat, 23 Feb 2013 11:42:26 -0500, H. S. Teoh
>> <hsteoh@quickfur.ath.cx> wrote:
>>
>> >- wait():
>> >   - Some code examples would be nice.
>> >
>> >   - For the POSIX-specific version, I thought the Posix standard
>> >     specifies that the actual return code / signal number should be
>> >     extracted by means of system-specific macros (in C anyway)?
>> >     Wouldn't it be better to encapsulate this in a POD struct or
>> >     something instead of exposing the implementation-specific values
>> >     to the user?
>>
>> We handle the extraction as an implementation detail, the result
>> should be cross-platform (at least on signal-using platforms).  I
>> don't know what a POD struct would get you, maybe you could elaborate
>> what you mean?
>
> Oh, I thought the return value was just straight from the syscall, which
> requires WIFEXITED, WEXITSTATUS, WCOREDUMP, etc., to interpret. If it
> has already been suitably interpreted in std.process, then I guess it's
> OK.

I think that's the case. (double checking) yes.

>
> Otherwise, I was thinking of encapsulating these macros in some kind of
> POD struct, that provides methods like .ifExited, .exitStatus,
> .coreDump, etc. so that the user code doesn't have to directly play with
> the exact values returned by the specific OS.

All we look at is WIFEXITED and WIFSIGNALED.  I think the others are Linux specific.  I don't think std.process2 should expose all the vagaries of the OS it's on, this is a cross-platform library.  Doing signals was easy because we embedded it in the int.

There is always pid.osHandle, which you can use to do whatever you want.

>> >   - How do I wait for *any* child process to terminate, not just a
>> >     specific Pid?
>>
>> I don't think we have a method to do that.  It would be complex,
>> especially if posix wait() returned a pid that we are not handling!
>>
>> I suppose what you could do is call posix wait (I have a feeling we
>> may need to change our global wait function, or eliminate it), and
>> then map the result back to a Pid you are tracking.
>>
>> You have any ideas how this could be implemented?  I'd prefer not to
>> keep a global cache of child process objects...
>
> Why not?  On Posix at least, you get SIGCHLD, etc., for all child
> processes anyway, so a global cache doesn't seem to be out-of-place.
>
> But you do have a point about pids that we aren't managing, e.g. if the
> user code is doing some fork()s on its own. But the way I see it,
> std.process is supposed to alleviate the need to do such things
> directly, so in my mind, if everything is going through std.process
> anyway, might as well just manage all child processes there. OTOH, this
> may cause problems if the D program links in C/C++ libraries that manage
> their own child processes.

Well, there is always the possibility of a child creating a child, and exiting, the grandchild then becomes our child.  There is no way to predict or plan for that.

If we expose the general wait call, then we will be subject to odd cases, and I think at that point, it's a specialized application.  We provide a way to get back to OS-specific land via osHandle.

> Still, it would be nice to have some way of waiting for a set of child
> Pids, not just a single one. It would be a pain if user code had to
> manually manage child processes all the time when there's more than one
> of them running at a time.

This is not possible on Linux/OSX (you can't specify the process subset to wait for), but possible on Windows.  We chose not to expose that because it's very application specific, and we are trying to write a cross platform library.

You can always write a function that does this.  Simple example:

Pid[int] processes;
// create processes, storing them by OS pid
int pid;
while(pid = .wait())
{
   Pid *p = processes[pid];
   if(p)
   {
      // handle child exiting
   }
}

> Hmm. The more I think about it, the more it makes sense to just have
> std.process manage all child process related stuff. It's too painful to
> deal with multiple child processes otherwise. Maybe provide an opt-out
> in case you need to link in some C/C++ libraries that need their own
> child process handling, but the default, IMO, should be to manage
> everything through std.process.

I can imagine that a ProcessManager singleton class could be written that collects all exited children, and does anything you need.  But I don't know if it's a necessary component for std.process to go out the door.  We currently have no such feature, so I would push for std.process to be reviewed and accepted without that, and then consider that an enhancement request.

Now, one thing we could probably do quickly and easily is add a wait function that returns immediately if the process is not done.  I'm pretty sure that is supported on all platforms.

-Steve

On Saturday, February 23, 2013 14:46:04 H. S. Teoh wrote:
> I never liked the design of errno in C... its being a global makes keeping track of errors a pain. It would be nice if the value of errno were saved in the Exception object at the time the error was encountered, instead of arbitrary amounts of code after, which may have changed its value.

That's what std.exception.errnoEnforce and ErrnoException doe. The ErrnoException gets the error code when it's constructed. std.file.FileException does the same thing at least some of the time (depending on what caused the error). In the case of FileException, we should probably have subclasses for the main error codes that you might want to handle specially (so catching the explicitly would be useful), but we don't have that yet.

I don't know what the new std.process is doing (I haven't look at it yet), but if it's throwing exceptions based on errno, it needs to at least put the error code in the exception and maybe have specific exception types if it would make sense to be catching exceptions from std.process based on what exactly went wrong. Get the value of errno after catching the exception is just asking for it, since who knows what code ran after the exception was originally thrown.

- Jonathan M Davis

On Sat, Feb 23, 2013 at 06:55:19PM -0500, Steven Schveighoffer wrote:
> On Sat, 23 Feb 2013 17:46:04 -0500, H. S. Teoh <hsteoh@quickfur.ath.cx> wrote:
> 
> >On Sat, Feb 23, 2013 at 03:15:26PM -0500, Steven Schveighoffer wrote:
[...]
> All we look at is WIFEXITED and WIFSIGNALED.  I think the others are Linux specific.  I don't think std.process2 should expose all the vagaries of the OS it's on, this is a cross-platform library.  Doing signals was easy because we embedded it in the int.

Fair enough, I think WIFEXITED and WIFSIGNALED probably covers 99.5% of the common use cases anyway, so it's probably not worth sweating over.

BTW, is "std.process2" just the temporary name, or are we seriously going to put in a "std.process2" into Phobos? I'm hoping the former, as the latter is unforgivably ugly.


> There is always pid.osHandle, which you can use to do whatever you want.

True.


> >>>   - How do I wait for *any* child process to terminate, not just a
> >>>     specific Pid?
> >>
> >>I don't think we have a method to do that.  It would be complex, especially if posix wait() returned a pid that we are not handling!
[...]
> >Hmm. The more I think about it, the more it makes sense to just have std.process manage all child process related stuff. It's too painful to deal with multiple child processes otherwise. Maybe provide an opt-out in case you need to link in some C/C++ libraries that need their own child process handling, but the default, IMO, should be to manage everything through std.process.
> 
> I can imagine that a ProcessManager singleton class could be written that collects all exited children, and does anything you need.  But I don't know if it's a necessary component for std.process to go out the door.  We currently have no such feature, so I would push for std.process to be reviewed and accepted without that, and then consider that an enhancement request.
[...]

Fair enough, we do want the new std.process to get in ASAP.


> Now, one thing we could probably do quickly and easily is add a wait function that returns immediately if the process is not done.  I'm pretty sure that is supported on all platforms.
[...]

Excellent! I think if we had this, it would address most of my concerns above.  A non-blocking wait would allow user code to do things like monitor the progress of child processes, etc., and basically implement whatever OS-specific stuff it may need to, without adding too much complication into std.process. I vote for putting this in, and just leave the handling of multiple child processes to user code.


T

-- 
What are you when you run out of Monet? Baroque.

On Sat, 23 Feb 2013 18:59:05 -0500, Jonathan M Davis <jmdavisProg@gmx.com> wrote:

> I don't know what the new std.process is doing (I haven't look at it yet), but
> if it's throwing exceptions based on errno, it needs to at least put the error
> code in the exception and maybe have specific exception types if it would make
> sense to be catching exceptions from std.process based on what exactly went
> wrong. Get the value of errno after catching the exception is just asking for
> it, since who knows what code ran after the exception was originally thrown.

It uses strerror to get the errno string representation, and uses that as the message.

I think it should also save the error code.

In a past life, I had a SystemException type (this was C++), and anything that died because of an errno error would throw a derivative of that, containing an errno copy.  If we already have a base ErrnoException, we ProcessException probably should derive from that.

-Steve

On Saturday, 23 February 2013 at 22:48:04 UTC, H. S. Teoh wrote:
> On Sat, Feb 23, 2013 at 03:15:26PM -0500, Steven Schveighoffer wrote:
>> On Sat, 23 Feb 2013 11:42:26 -0500, H. S. Teoh
>> <hsteoh@quickfur.ath.cx> wrote:
>> 
>> >- wait():
>> >   - Some code examples would be nice.
>> >
>> >   - For the POSIX-specific version, I thought the Posix
>> >     standard specifies that the actual return code /
>> >     signal number should be extracted by means of
>> >     system-specific macros (in C anyway)?
>> >     Wouldn't it be better to encapsulate this in a POD
>> >     struct or something instead of exposing the
>> >     implementation-specific values to the user?
>> 
>> We handle the extraction as an implementation detail, the result should be cross-platform (at least on signal-using
>> platforms). I don't know what a POD struct would get you,
>> maybe you could elaborate what you mean?
>
> Oh, I thought the return value was just straight from the syscall, which requires WIFEXITED, WEXITSTATUS, WCOREDUMP,
> etc., to interpret. If it has already been suitably interpreted
> in std.process, then I guess it's OK.
>
> Otherwise, I was thinking of encapsulating these macros in
> some kind of POD struct, that provides methods like
> .ifExited, .exitStatus, .coreDump, etc. so that the user
> code doesn't have to directly play with the exact values returned by the specific OS.
>
>> >   - How do I wait for *any* child process to terminate, not
>> >     just a specific Pid?
>> 
>> I don't think we have a method to do that.  It would be complex, especially if posix wait() returned a pid that we
>> are not handling!
>> 
>> I suppose what you could do is call posix wait (I have a feeling we may need to change our global wait function, or
>> eliminate it), and then map the result back to a Pid you
>> are tracking.
>> 
>> You have any ideas how this could be implemented?  I'd prefer not to keep a global cache of child process objects...

Btw on windows a simple array with the hProcesses is all you
need to do this, the code below uses AA for reverse lookup,
really simple stuff;

https://github.com/Wallbraker/Unicorn/blob/master/src/uni/util/cmd.d#L354

The code was written before before I know of the new
std.process, but it solved exactly this problem.

>
> Why not?  On Posix at least, you get SIGCHLD, etc., for all child processes anyway, so a global cache doesn't seem to be out-of-place.
>
> But you do have a point about pids that we aren't managing, e.g. if the user code is doing some fork()s on its own. But
> the way I see it, std.process is supposed to alleviate the
> need to do such things directly, so in my mind, if everything
> is going through std.process anyway, might as well just manage
> all child processes there. OTOH, this may cause problems if the
> D program links in C/C++ libraries that manage their own chil
> processes.

But as you state above, it only works for a single CmdGroup on
posix and will probably interact badly with code using wait.

I never got so far because the code works for my limited case,
but supposedly process groups might help out, but the seem to
interfere with signals. More reading is required.

http://linux.die.net/man/7/credentials

>
> Still, it would be nice to have some way of waiting for a set of child Pids, not just a single one. It would be a pain if
> user code had to manually manage child processes all the time
> when there's more than one of them running at a time.
>
> Hmm. The more I think about it, the more it makes sense to just have std.process manage all child process related stuff. It's
> too painful to deal with multiple child processes otherwise.
> Maybe provide an opt-out in case you need to link in some
> C/C++ libraries that need their own child process handling, but
> the default, IMO, should be to manage everything through
> std.process.

This needs to be opt-in, because this stuff should not break
libraries using fork/wait by default.

Cheers, Jakob.