On Sunday, February 24, 2013 13:35:31 Dmitry Olshansky wrote:
> 24-Feb-2013 13:17, Jonathan M Davis пишет:
> > On Sunday, February 24, 2013 13:06:00 Dmitry Olshansky wrote:
> >> 24-Feb-2013 12:05, Andrei Alexandrescu пишет:
> >>> On 2/24/13 6:26 AM, Andrej Mitrovic wrote:
> >>>> Phobos modules which already use std.process would have to be changed to directly import std.process1 or std.process2.
> >>> 
> >>> This is problematic as has been discussed. I think we could address immediate needs by attaching an attribute to import, e.g.:
> >>> 
> >>> @"v2.070+" import std.process;
> >>> 
> >>> or similar. By default code would import the old library.
> >> 
> >> The same could be achieved by simply using old version of compiler+druntime+phobos to compile old project.
> >> 
> >> I don't get the desire to keep old junk forever. A year or two - maybe. More then this is just insane.
> > 
> > I agree, but it _is_ more than a question of keeping old junk around in this case. We need a we to transition cleanly, and the only way at present that that means not breaking code is to put the new std.process somewhere other than std.process, since if we put it in std.process, it would mean breaking a lot of code which uses the current std.process, forcing everyone to stick with the old compiler until they'd updated their code. And we don't want that.
> I suppose it's easy to translate any active project (as in maintained) to the new std.process, as it's supposed to have easier/richer interface.
> 
> Then any old cruft that just works and there is no need to touch it can just use the older version of _compiler_. In any case the phrase "old code that needs to use new std.process" is kind of perplexing.

It's the fact that you're forced to update your code when you update the compiler rather than giving you time to update it which is the problem. I'm not completely against the idea of saying that we have the old std.process in one version and the new std.process in another - especially if we had more of a major-minor versioning scheme where it was expected that code would break at major version changes, but we don't really have that right now, and I really don't think that Walter would go for it anyway.

Long term, code should definitely be ported over to the new std.process, and it's probably not all that big a deal in this case (though other modules in similar situations could be a much bigger deal - e.g. std.date -> std.datetime was a very large change), but the question is whether it's okay to force people to change their code immediately. If it's not (and given Walter's attitude and our current versioning scheme, I don't think that it is), then we need to come up with a new name for the new std.process module rather than replacing it in-place. But it _is_ this sort of change which would make it desirable to adjust our versioning scheme.

> > Whether the old std.process sticks around for more than a year or two is
> > therefore a separate matter from what we name the new std.process unless
> > we
> > add a feature like Andrei is suggesting.
> 
> Maybe I'm missing something but I don't see this feature solving anything yet.

It makes it so that you can have both the old and new std.process be name std.process. However, given that @"v2.064+" or whatever would have to be there to use the new std.process and the fact that you'd presumably would want to remove it once the old std.process was actually gone, it's ultimately not all that different from naming it std.process2 and then renaming it to std.process later. It's just an attribute rather than a 2 at the end of its name.

- Jonathan M Davis

Am 23.02.2013 12:31, schrieb 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?)
> 
> Lars

I haven't read all responses (sorry), but considering that there don't seem to be API conflicts between the old and new std.process, why don't we just keep the old C style functions and deprecate them? No need to rename or create a separate module AFAICS.

1, What about support for nonblocking wait(). It would be very nice not to block the main thread if you really don't care about waiting for the sub process but just want to be nice and not create zombies.

2, What about nonblocking read/writes or support for timing out reads/writes at least. On linux you can select() on the file descriptor but that is not supported on windows.

I believe that support for nonblocking operation is important since spawning external processes  is a common way to parallelize work and std.concurrency could take advantage of this as well.

/Jonas

On 2/24/13 11:06 AM, Dmitry Olshansky wrote:
> 24-Feb-2013 12:05, Andrei Alexandrescu пишет:
>> On 2/24/13 6:26 AM, Andrej Mitrovic wrote:
>>> Phobos modules which already use std.process would have to be changed
>>> to directly import std.process1 or std.process2.
>>
>> This is problematic as has been discussed. I think we could address
>> immediate needs by attaching an attribute to import, e.g.:
>>
>> @"v2.070+" import std.process;
>>
>> or similar. By default code would import the old library.
>
> The same could be achieved by simply using old version of
> compiler+druntime+phobos to compile old project.

That's quite different I'd say.

Andrei

I apologise if this gets posted twice, but my newsgroup client is acting up.

On Saturday, 23 February 2013 at 16:44:30 UTC, H. S. Teoh wrote:
> I just looked over the docs. Looks very good!

Thanks! :)

> Just a few minor comments:
>
> - wait():
>    - Some code examples would be nice.

The wait() documentation says "Examples: See the spawnProcess documentation", and provides a link.  I didn't see any point in duplicating the examples there just for the sake of it.


>    - 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?

Steve already answered this, but yes, the number returned by wait() has already been processed by these macros.  wait() therefore has the same result on all POSIX systems.


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

I will write a separate post about this shortly.


> - 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.)?

Again, Steve answered this, but let me clarify:  There are three layers of functionality in this module.

1. spawnProcess() gives you detailed control over process creation, in a cross-platform manner.

2. pipeProcess()/pipeShell() are convenience functions that take care of creating pipes to the child process for you, as this is a common use case.  Use this if you need detailed control over what goes in and out of the process.

3. execute()/shell() are a second layer of convenience, if you just want a simple way to execute a process and retrieve its output.  If there is any chance of the process producing vast amounts of output, you really should use pipeProcess() or even spawnProcess().

Of course, it is a simple matter to add an optional maxOutputSize parameter to execute() and an "overflow" flag to the returned tuple, but I think it adds more API clutter than it is worth.  I'd love to hear others' opinions about it, though.


> - 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)?

Well, no.  The thing is, ProcessException may be associated with an errno code, with a Windows GetLastError() code, or it may simply be a pure "D error" with no relation at all to the underlying C APIs.

Personally, I believe that the distinction between ProcessException (for process management errors) and StdioException (for I/O errors, mostly related to pipes) provides enough granularity for most use cases.  I believe it's the best we can do in a cross-platform manner without inventing our own error codes and mapping them to errno/GetLastError() codes.  If anyone needs the underlying OS error code, they can still retrieve them using the system-specific APIs.

Again, I am of course open for discussion about this.

Lars

On Sunday, 24 February 2013 at 04:54:13 UTC, Jonathan M Davis wrote:
> On Saturday, February 23, 2013 19:47:54 Nathan M. Swan wrote:
>> Why not just deprecate everything currently in std.process and drop in
>> the new stuff? It might be a bit ugly, but it prevents both code
>> breakage _and_ a proliferation of "std.module2"s.
>
> That only works if there are no conflicts and none of the functions' behaviors
> are changed in a manner which would be incompatible with how they currently
> work.
> [...]
> I don't know how much the new std.process
> conflicts with the old one, but since the main fellow behind the new one is the
> same one who did the new std.path, I suspect that they conflict to much to be
> able to do the transition within a single module. I'd have to compare them to
> be sure though.

Let me save you the trouble.  There are three points of incompatibility between the old and the new APIs, that prevent them from coexisting:

shell():

The old shell() captures the standard output, returns only a string containing the output, and throws an exception if the program returns with a nonzero exit code.

The new shell() only throws if there was an error with actually running the process, it captures both the standard output AND standard error streams, and it returns a tuple which contains both the output and the exit code.

environment.opIndex():

The new version no longer throws if the environment variable does not exist, it simply returns null.  This is a very subtle change that will not cause a compilation error.

It was I who wrote the old 'environment' as well, and the idea was for it to have the exact same interface as the built-in associative arrays.  Therefore, at the time, it seemed like a good idea to throw from opIndex().  However, having actually used it for quite some time, I have found that I *never* want that functionality.  I *always* end up calling environment.get() instead, to avoid the exception.  Thus, I decided use this opportunity to change it.

environment.get():

To distinguish it from opIndex(), the second argument (the default value if the variable doesn't exist) is no longer optional.  This, however, will cause a compilation error in the cases where the second argument has been left out, and all other cases will work like they used to.


I would absolutely *hate* to have to change the name of shell(), or, worse, revert it to the old version.  I am reluctant to revert environment.opIndex() too, as I would really like to get it right this time.  I am not going to be as adamant on this, however.  environment.get() is not a big deal, but if environment.opIndex() changes, this might as well do too.

Lars

On Sun, 24 Feb 2013 08:03:24 -0500, Jonas Drewsen <jdrewsen@nospam.com> wrote:

>
> 1, What about support for nonblocking wait(). It would be very nice not to block the main thread if you really don't care about waiting for the sub process but just want to be nice and not create zombies.

Non-blocking wait was brought up.  I think we can add it.  It would be non-blocking wait on specific processes though, I think doing a wait for *any* process is a more difficult problem to solve, and is not supported in the current std.process.  It may be something added later.

> 2, What about nonblocking read/writes or support for timing out reads/writes at least. On linux you can select() on the file descriptor but that is not supported on windows.

This is not an issue with std.process, but rather with File.  If File doesn't support non-blocking read/write, that is an issue, but we should solve it for all streams, not just pipes.

> I believe that support for nonblocking operation is important since spawning external processes  is a common way to parallelize work and std.concurrency could take advantage of this as well.

I agree, and would love to see File support non-blocking operations.

-Steve

On Sunday, 24 February 2013 at 00:11:42 UTC, H. S. Teoh wrote:
> 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.

I agree, it's not ideal, but "unforgivably ugly" is taking it a bit far. :)

Anyway, to be honest, I named it std.process2 because I got tired of merge conflicts whenever someone made changes in Phobos master that either directly or indirectly involved the current std.process.

Whether it should finally be named std.process or std.process2 is open for debate, IMO, but I have to admit that I am to an increasing degree starting to understand Walter's point of view on these matters...

Lars

On Saturday, 23 February 2013 at 11:31:21 UTC, 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.
>
> 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?)


Ok, there have been several posts about changes and additions to wait(), and I believe Steve has answered them all, so let me just summarise:


Non-blocking wait:

This is a good idea, and it should be simple to implement.  I'll do that.


Wait for all processes:

This would certainly be convenient, and it is simple to implement.  It would require a static __gshared Pid[int] of all processes created by spawnProcess(), and we'd simply call wait() on all of those.

The question, is this simple enough that we can just leave it to the user, thereby avoiding the need for a global Pid cache?  (I think so.)

Note that Windows has built-in functionality for waiting for a set of processes, namely WaitForMultipleObjects().  std.process2 would take advantage of that as a minor optimisation, but my guess is that the benefit would be negligible.


Wait for any process:

This one is a problem.  POSIX supports it natively, but as has been pointed out by others, it would necessarily also affect processes *not* created through std.process.  I don't see any obvious solution to this.

On Windows, this is also supported through WaitForMultipleObjects(), but again, it would require a global Pid cache.

I think this one is a no-go.

Lars

On Sunday, 24 February 2013 at 10:23:36 UTC, Sönke Ludwig wrote:
> I haven't read all responses (sorry), but considering that there don't
> seem to be API conflicts between the old and new std.process, why don't
> we just keep the old C style functions and deprecate them? No need to
> rename or create a separate module AFAICS.

There are API conflicts.  See my reply to Jonathan.

Lars