Jump to page: 1 2
Thread overview
[phobos] popen/pclose and bug 3157
Sep 08, 2010
Brad Roberts
Sep 08, 2010
Brad Roberts
Sep 09, 2010
Brad Roberts
September 08, 2010
I just updated the patch that Lars created to address a major problem with popen support in std.stdio.  His version handles the first problem, the requirement that a popened fd be closed via pclose rather than fclose.

His patch doesn't address my second issue with popen/pclose, which is getting the exit code from pclose back to the caller.  I don't like what I just did.  In fact, I hesitated quite a bit before sending out this email.  It's, well, fugly, but works until a better solution can be found.

I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.

   http://d.puremagic.com/issues/show_bug.cgi?id=3157

Thoughts?

Thanks,
Brad

September 08, 2010
On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
> I just updated the patch that Lars created to address a major problem with popen support in std.stdio.  His version handles the first problem, the requirement that a popened fd be closed via pclose rather than fclose.
> 
> His patch doesn't address my second issue with popen/pclose, which is getting the exit code from pclose back to the caller.  I don't like what I just did.  In fact, I hesitated quite a bit before sending out this email.  It's, well, fugly, but works until a better solution can be found.
> 
> I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.

The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.

So I guess the next steps will be
  1. Wait for next DMD release, which will contain aforementioned fix.
  2. Finish up Windows version.
  3. Code review and hopefully acceptance in time for following release.
[ 4. Deprecation and subsequent death of File.popen(). ;) ]

-Lars

September 08, 2010
I just reopened that issue.  Walter reverted the patch because it caused an infinite recursion in the compiler.

There's an updated patch, that needs to be added in order to be able to do the windows development.

BTW, I have a windows version that compiles, I just can't test it, because the workaround for that bug creates a module static constructor cycle.

-Steve



----- Original Message ----
> From: Lars Tandle Kyllingstad <lars at kyllingen.net>
> To: Discuss the phobos library for D <phobos at puremagic.com>
> Sent: Wed, September 8, 2010 5:48:50 AM
> Subject: Re: [phobos] popen/pclose and bug 3157
> 
> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
> > I just updated  the patch that Lars created to address a major problem with
>popen
> >  support in std.stdio.  His version handles the first problem, the
>requirement
> > that a popened fd be closed via pclose rather than  fclose.
> > 
> > His patch doesn't address my second issue with  popen/pclose, which is
>getting
> > the exit code from pclose back to the  caller.  I don't like what I just
>did.  In
> > fact, I hesitated  quite a bit before sending out this email.  It's, well,
>fugly,
> > but  works until a better solution can be found.
> > 
> > I know that a major  rework of std.process is a work in progress.  What's
the
> > state of  it?  I have a need, right now, for being able to execute a command
>and
> > getting back both it's output and its exit code.. and it needs to  work on
>all
> > platforms.
> 
> The current status is that the POSIX  version works, and has done so for a while.  Its incorporation in  Phobos, and development of the Windows implementation, has been blocked by a  DMD bug which is now fixed in SVN.
> 
> So I guess the next steps will  be
>   1. Wait for next DMD release, which will contain aforementioned  fix.
>   2. Finish up Windows version.
>   3. Code review and  hopefully acceptance in time for following release.
> [ 4. Deprecation and  subsequent death of File.popen(). ;)  ]
> 
> -Lars
> 
> _______________________________________________
> phobos  mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
> 



September 08, 2010
On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:

> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
> > I just updated the patch that Lars created to address a major problem with popen support in std.stdio.  His version handles the first problem, the requirement that a popened fd be closed via pclose rather than fclose.
> > 
> > His patch doesn't address my second issue with popen/pclose, which is getting the exit code from pclose back to the caller.  I don't like what I just did.  In fact, I hesitated quite a bit before sending out this email.  It's, well, fugly, but works until a better solution can be found.
> > 
> > I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.
> 
> The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
> 
> So I guess the next steps will be
>   1. Wait for next DMD release, which will contain aforementioned fix.
>   2. Finish up Windows version.
>   3. Code review and hopefully acceptance in time for following release.
> [ 4. Deprecation and subsequent death of File.popen(). ;) ]
> 
> -Lars

Would you point me to the docs for the new version?

Thanks,
Brad

September 09, 2010
On Wed, 2010-09-08 at 13:52 -0700, Brad Roberts wrote:
> On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:
> > On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
> > > I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.
> > 
> > The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
> > 
> > So I guess the next steps will be
> >   1. Wait for next DMD release, which will contain aforementioned fix.
> >   2. Finish up Windows version.
> >   3. Code review and hopefully acceptance in time for following release.
> > [ 4. Deprecation and subsequent death of File.popen(). ;) ]
> > 
> > -Lars
> 
> Would you point me to the docs for the new version?


Sure:

  http://kyllingen.net/code/ltk/doc/process.html

-Lars

September 09, 2010
On 9/9/2010 12:59 AM, Lars Tandle Kyllingstad wrote:
> On Wed, 2010-09-08 at 13:52 -0700, Brad Roberts wrote:
>> On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:
>>> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
>>>> I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.
>>>
>>> The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
>>>
>>> So I guess the next steps will be
>>>   1. Wait for next DMD release, which will contain aforementioned fix.
>>>   2. Finish up Windows version.
>>>   3. Code review and hopefully acceptance in time for following release.
>>> [ 4. Deprecation and subsequent death of File.popen(). ;) ]
>>>
>>> -Lars
>>
>> Would you point me to the docs for the new version?
> 
> 
> Sure:
> 
>   http://kyllingen.net/code/ltk/doc/process.html
> 
> -Lars
> 

Thanks.. I like the looks of it.  spawnProcess looks like exactly what I'd love to have right now. :)

I didn't read super closely, but some things that I noticed:

In enum Redirect, you might want to change 'all' to 'allstd' or something. Also, you might want to generally comment about what's done with fd's NOT in the 0..2 (inclusive) range.

You have such a rich set for spawnProcess that the omission of a form of pipeProcess, pipeShell, execute, and shell that also takes an env array stands out.

Thanks for building this,
Brad
September 09, 2010
On Thu, 2010-09-09 at 01:22 -0700, Brad Roberts wrote:
> On 9/9/2010 12:59 AM, Lars Tandle Kyllingstad wrote:
> > On Wed, 2010-09-08 at 13:52 -0700, Brad Roberts wrote:
> >> On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:
> >>> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
> >>>> I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.
> >>>
> >>> The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
> >>>
> >>> So I guess the next steps will be
> >>>   1. Wait for next DMD release, which will contain aforementioned fix.
> >>>   2. Finish up Windows version.
> >>>   3. Code review and hopefully acceptance in time for following release.
> >>> [ 4. Deprecation and subsequent death of File.popen(). ;) ]
> >>>
> >>> -Lars
> >>
> >> Would you point me to the docs for the new version?
> > 
> > 
> > Sure:
> > 
> >   http://kyllingen.net/code/ltk/doc/process.html
> > 
> > -Lars
> > 
> 
> Thanks.. I like the looks of it.  spawnProcess looks like exactly what I'd love to have right now. :)
> 
> I didn't read super closely, but some things that I noticed:
> 
> In enum Redirect, you might want to change 'all' to 'allstd' or something. Also, you might want to generally comment about what's done with fd's NOT in the 0..2 (inclusive) range.

It is mentioned in the spawnProcess() docs -- see the 'Note' section.


> You have such a rich set for spawnProcess that the omission of a form of pipeProcess, pipeShell, execute, and shell that also takes an env array stands out.

I did consider it, but for completeness that would mean not just one,
but two more forms of pipeProcess() and execute().  Example:

  // Already there:
  execute(command)
  execute(exe, args)

  // Add these?
  execute(command, env)
  execute(exe, args, env)

It just seemed a bit too much for what I believed to be a rare need.  If there is a real need for this, though, I don't have any problems with adding it.

-Lars

September 09, 2010
I think we need one more function - shellQuote. That would help people quote their arguments properly to avoid trouble with filenames containing spaces etc.

I suspect on Unix in general it suffices to prepend and append a single quote ' and to replace any single quotes with '"'"'. Thus,

It's wonderful

becomes

'It'"'"'s wonderful'

Oh, and one more thing - shouldn't we template some functions on the string type?


Andrei

On 9/9/10 6:43 CDT, Lars Tandle Kyllingstad wrote:
> On Thu, 2010-09-09 at 01:22 -0700, Brad Roberts wrote:
>> On 9/9/2010 12:59 AM, Lars Tandle Kyllingstad wrote:
>>> On Wed, 2010-09-08 at 13:52 -0700, Brad Roberts wrote:
>>>> On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:
>>>>> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
>>>>>> I know that a major rework of std.process is a work in progress.  What's the state of it?  I have a need, right now, for being able to execute a command and getting back both it's output and its exit code.. and it needs to work on all platforms.
>>>>>
>>>>> The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
>>>>>
>>>>> So I guess the next steps will be
>>>>>    1. Wait for next DMD release, which will contain aforementioned fix.
>>>>>    2. Finish up Windows version.
>>>>>    3. Code review and hopefully acceptance in time for following release.
>>>>> [ 4. Deprecation and subsequent death of File.popen(). ;) ]
>>>>>
>>>>> -Lars
>>>>
>>>> Would you point me to the docs for the new version?
>>>
>>>
>>> Sure:
>>>
>>>    http://kyllingen.net/code/ltk/doc/process.html
>>>
>>> -Lars
>>>
>>
>> Thanks.. I like the looks of it.  spawnProcess looks like exactly what I'd love to have right now. :)
>>
>> I didn't read super closely, but some things that I noticed:
>>
>> In enum Redirect, you might want to change 'all' to 'allstd' or something. Also, you might want to generally comment about what's done with fd's NOT in the 0..2 (inclusive) range.
>
> It is mentioned in the spawnProcess() docs -- see the 'Note' section.
>
>
>> You have such a rich set for spawnProcess that the omission of a form of pipeProcess, pipeShell, execute, and shell that also takes an env array stands out.
>
> I did consider it, but for completeness that would mean not just one,
> but two more forms of pipeProcess() and execute().  Example:
>
>    // Already there:
>    execute(command)
>    execute(exe, args)
>
>    // Add these?
>    execute(command, env)
>    execute(exe, args, env)
>
> It just seemed a bit too much for what I believed to be a rare need.  If there is a real need for this, though, I don't have any problems with adding it.
>
> -Lars
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
September 09, 2010
On 9/9/10 6:43 CDT, Lars Tandle Kyllingstad wrote:
> I did consider it, but for completeness that would mean not just one,
> but two more forms of pipeProcess() and execute().  Example:
>
>    // Already there:
>    execute(command)
>    execute(exe, args)
>
>    // Add these?
>    execute(command, env)
>    execute(exe, args, env)
>
> It just seemed a bit too much for what I believed to be a rare need.  If there is a real need for this, though, I don't have any problems with adding it.

How would people who do have such a need achieve it without the added primitives? If they can't, probably you need to add them.

Perhaps you can consolidate the functions by using a default argument for env.


Andrei
September 09, 2010
The way we solve this is with a version that accepts the arguments as an array. Then the arguments are passed to the shell after being properly quoted and escaped.

-Steve



----- Original Message ----
From: Andrei Alexandrescu <andrei at erdani.com>
To: Discuss the phobos library for D <phobos at puremagic.com>
Sent: Thu, September 9, 2010 10:03:43 AM
Subject: Re: [phobos] popen/pclose and bug 3157

I think we need one more function - shellQuote. That would help people quote their arguments properly to avoid trouble with filenames containing spaces etc.

I suspect on Unix in general it suffices to prepend and append a single quote ' and to replace any single quotes with '"'"'. Thus,

It's wonderful

becomes

'It'"'"'s wonderful'

Oh, and one more thing - shouldn't we template some functions on the string type?


Andrei

On 9/9/10 6:43 CDT, Lars Tandle Kyllingstad wrote:
> On Thu, 2010-09-09 at 01:22 -0700, Brad Roberts wrote:
>> On 9/9/2010 12:59 AM, Lars Tandle Kyllingstad wrote:
>>> On Wed, 2010-09-08 at 13:52 -0700, Brad Roberts wrote:
>>>> On Wed, 8 Sep 2010, Lars Tandle Kyllingstad wrote:
>>>>> On Wed, 2010-09-08 at 02:14 -0700, Brad Roberts wrote:
>>>>>> I know that a major rework of std.process is a work in progress.  What's
>>the
>>>>>> state of it?  I have a need, right now, for being able to execute a command
>>>>>and
>>>>>> getting back both it's output and its exit code.. and it needs to work on
>>>all
>>>>>> platforms.
>>>>>
>>>>> The current status is that the POSIX version works, and has done so for a while.  Its incorporation in Phobos, and development of the Windows implementation, has been blocked by a DMD bug which is now fixed in SVN.
>>>>>
>>>>> So I guess the next steps will be
>>>>>    1. Wait for next DMD release, which will contain aforementioned fix.
>>>>>    2. Finish up Windows version.
>>>>>    3. Code review and hopefully acceptance in time for following release.
>>>>> [ 4. Deprecation and subsequent death of File.popen(). ;) ]
>>>>>
>>>>> -Lars
>>>>
>>>> Would you point me to the docs for the new version?
>>>
>>>
>>> Sure:
>>>
>>>    http://kyllingen.net/code/ltk/doc/process.html
>>>
>>> -Lars
>>>
>>
>> Thanks.. I like the looks of it.  spawnProcess looks like exactly what I'd
>love
>> to have right now. :)
>>
>> I didn't read super closely, but some things that I noticed:
>>
>> In enum Redirect, you might want to change 'all' to 'allstd' or something.
>>Also,
>> you might want to generally comment about what's done with fd's NOT in the
>0..2
>> (inclusive) range.
>
> It is mentioned in the spawnProcess() docs -- see the 'Note' section.
>
>
>> You have such a rich set for spawnProcess that the omission of a form of pipeProcess, pipeShell, execute, and shell that also takes an env array stands
>>out.
>
> I did consider it, but for completeness that would mean not just one,
> but two more forms of pipeProcess() and execute().  Example:
>
>    // Already there:
>    execute(command)
>    execute(exe, args)
>
>    // Add these?
>    execute(command, env)
>    execute(exe, args, env)
>
> It just seemed a bit too much for what I believed to be a rare need.  If there is a real need for this, though, I don't have any problems with adding it.
>
> -Lars
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
_______________________________________________
phobos mailing list
phobos at puremagic.com
http://lists.puremagic.com/mailman/listinfo/phobos




« First   ‹ Prev
1 2