Removes popen3 deprecated warning

I tested this on windows by adding a call to Popen (just opening notepad) at the start of lilypond-book.  This is why we need the conditional test for fds-close - windows does not allow this to be True when used with the stdin, etc., options.  This is running the python instance that is delivered with the current lilypond.  I have also run make, make doc and confirmed that the deprecation warning is taken out.

[PhilEHolmes, 2012-08-18 13:50:13]

Please review

[Reinhold, 2012-08-18 14:12:43]

IIRC, the problem was not the Popen wouldn't be working on Windows. Rather, our
GUB installer somehow does not include the msvcrt python  module in its python
build, which is needed on Windows to provide the Popen call. So, running
lilypond-book fromself-compiled binaries has always been working, only the
downloads from lilypond.org didn't...

[Graham Percival, 2012-08-18 14:23:09]

Hmm.  I'm not really qualified to comment on python+windows+GUB stuff, but are
you certain that this won't re-open
http://code.google.com/p/lilypond/issues/detail?id=1933
?

[mail_philholmes.net, 2012-08-18 14:31:12]

----- Original Message ----- 
From: <reinhold.kainhofer@gmail.com>
To: <PhilEHolmes@googlemail.com>; <graham@percival-music.ca>; 
<john.mandereau@gmail.com>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Saturday, August 18, 2012 3:12 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)


> IIRC, the problem was not the Popen wouldn't be working on Windows.
> Rather, our GUB installer somehow does not include the msvcrt python
> module in its python build, which is needed on Windows to provide the
> Popen call. So, running lilypond-book fromself-compiled binaries has
> always been working, only the downloads from lilypond.org didn't...
>
> http://codereview.appspot.com/6471043/


My understanding of this problem was that it appeared that Popen (at some 
point in the past) would not work with our delivered Python (as created by 
Gub).  I also believed that the fix to 1933 also probably removed this 
limitation.  So I took lilypond-book and ran it using the python delivered 
with 2.15.95 (I only have lilypond-delivered python on my machine).  I then 
added a call to Popen, and apart from the need to fix close_fds it ran 
without problem.  So I concluded that the limitation is no longer there in 
our delivered python.

FWIW, the place where Popen is called is a pretty arcane bit of the 
lilypond-book code anyway - I couldn't find a way to exercise it directly on 
my Windows box, so I would be surprised if any windows users actually have 
this problem.  If it's anyone, I'd guess it could be Trevor, so would 
appreciate his testing this.  However, as I say above, I'd be surprised if 
it fails.

--
Phil Holmes 

[email_philholmes.net, 2012-08-18 14:31:55]

----- Original Message ----- 
From: <graham@percival-music.ca>
To: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>; 
<reinhold.kainhofer@gmail.com>
Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com>
Sent: Saturday, August 18, 2012 3:23 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)


> Hmm.  I'm not really qualified to comment on python+windows+GUB stuff,
> but are you certain that this won't re-open
> http://code.google.com/p/lilypond/issues/detail?id=1933
> ?
>
>
> http://codereview.appspot.com/6471043/

See my reply to Reinhold, but I think it's likely the other way round.

--
Phil Holmes 

[Graham Percival, 2012-08-18 14:39:29]

On Sat, Aug 18, 2012 at 03:31:06PM +0100, Phil Holmes wrote:
> My understanding of this problem was that it appeared that Popen (at
> some point in the past) would not work with our delivered Python (as
> created by Gub).  I also believed that the fix to 1933 also probably
> removed this limitation.

No; the fix to 1933 added a special case for win32 to avoid using
subprocess.  See:
  git show ab8dfc

As of 2012 Jan 12, python files produced via GUB could not run
subprocess.  I'm not aware of any work on GUB since then which
might make this possible.  Now, you could look at ab8dfc for
inspiration on how to write a special-case to use the old
os.popen3 version for windows and use subprocess for other
operating systems.  That would remove the deprecation warning when
building on lilydev.

- Graham

[mail_philholmes.net, 2012-08-18 15:14:01]

----- Original Message ----- 
From: "Graham Percival" <graham@percival-music.ca>
To: "Phil Holmes" <mail@philholmes.net>
Cc: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>; 
<reinhold.kainhofer@gmail.com>; <lilypond-devel@gnu.org>; 
<reply@codereview-hr.appspotmail.com>
Sent: Saturday, August 18, 2012 3:39 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)


> On Sat, Aug 18, 2012 at 03:31:06PM +0100, Phil Holmes wrote:
>> My understanding of this problem was that it appeared that Popen (at
>> some point in the past) would not work with our delivered Python (as
>> created by Gub).  I also believed that the fix to 1933 also probably
>> removed this limitation.
>
> No; the fix to 1933 added a special case for win32 to avoid using
> subprocess.  See:
>  git show ab8dfc
>
> As of 2012 Jan 12, python files produced via GUB could not run
> subprocess.  I'm not aware of any work on GUB since then which
> might make this possible.  Now, you could look at ab8dfc for
> inspiration on how to write a special-case to use the old
> os.popen3 version for windows and use subprocess for other
> operating systems.  That would remove the deprecation warning when
> building on lilydev.
>
> - Graham
>

OK - so from my bog-standard box, with only lilypond versions of Python:

C:\Users\Phil>path
PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\Windows\System32\WindowsPowerShell\v1.0\;
c:\Program Files (x86)\LilyPondV2.15.95\usr\bin

As you see, the only way python is run is from 2.15.95\usr\bin

C:\Users\Phil>python
Python 2.4.5 (#1, Feb 28 2012, 00:07:03)
[GCC 4.1.1] on mingw32
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> p = subprocess.Popen("notepad", shell=True, stdin=subprocess.PIPE, 
>>> stdout=su
bprocess.PIPE, stderr=subprocess.PIPE, close_fds=False)
>>>

And python on my box happily runs Popen.  So either we've done something in 
the long-lost mists to get round this problem, or it wasn't a problem at 
all.

Now checking back to emails related to 1933, I wrote:

"As an ugly side-effect, it imported subprocess, which caused our 
installation to fail as missing msvcrt. For a reason I don't understand, we 
started shipping msvcrt in the last few releases, "

So we could try to find out when we re-started shipping msvcrt, but you'll 
see I concluded in January that we do, and I still believe that to be true, 
based on the above.  Sorry, this is a bit of a rambling response, but I've 
just done the same Python test on an earlier lilypond shipped python:

C:\Users\Phil>path
PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\Windows\System32\WindowsPowerShell\v1.0\;
c:\Program Files (x86)\LilyPondV2.13.31\usr\bin

C:\Users\Phil>python
Python 2.4.5 (#1, Aug  6 2010, 09:30:22)
[GCC 4.1.1] on mingw32
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File "c:\Program Files 
(x86)\LilyPondV2.13.31\usr\lib\python2.4\subprocess.py"
, line 352, in ?
    import msvcrt
ImportError: No module named msvcrt
>>>

So we have fixed it.  You want I should find out when?

--
Phil Holmes 

[email_philholmes.net, 2012-08-18 16:23:42]

----- Original Message ----- 
From: "Phil Holmes" <mail@philholmes.net>
To: "Graham Percival" <graham@percival-music.ca>
Cc: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>; 
<reinhold.kainhofer@gmail.com>; <lilypond-devel@gnu.org>; 
<reply@codereview-hr.appspotmail.com>
Sent: Saturday, August 18, 2012 4:13 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)



> So we have fixed it.  You want I should find out when?

Between 2.15.18 and 2.15.21

--
Phil Holmes 

[email_philholmes.net, 2012-08-18 17:06:35]

----- Original Message ----- 
From: "Phil Holmes" <email@philholmes.net>
To: "Phil Holmes" <mail@philholmes.net>; "Graham Percival" 
<graham@percival-music.ca>
Cc: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>; 
<reinhold.kainhofer@gmail.com>; <lilypond-devel@gnu.org>; 
<reply@codereview-hr.appspotmail.com>
Sent: Saturday, August 18, 2012 5:23 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)


> ----- Original Message ----- 
> From: "Phil Holmes" <mail@philholmes.net>
> To: "Graham Percival" <graham@percival-music.ca>
> Cc: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>; 
> <reinhold.kainhofer@gmail.com>; <lilypond-devel@gnu.org>; 
> <reply@codereview-hr.appspotmail.com>
> Sent: Saturday, August 18, 2012 4:13 PM
> Subject: Re: Removes popen3 deprecated warning (issue 6471043)
>
>
>
>> So we have fixed it.  You want I should find out when?
>
> Between 2.15.18 and 2.15.21
>
> --
> Phil Holmes

And a bit of detective work shows that C:\Program Files 
(x86)\LilyPond\usr\lib\python2.4\config\config.c was changed between those 2 
builds.

Earlier version:

extern void init_symtable(void);
extern void initxxsubtype(void);

/* -- ADDMODULE MARKER 1 -- */

Later version:

extern void init_symtable(void);
extern void initxxsubtype(void);
extern void init_subprocess(void);
extern void initmsvcrt(void);

/* -- ADDMODULE MARKER 1 -- */

Dunno how, though.  Still looking.

--
Phil Holmes 

[Graham Percival, 2012-08-18 18:34:36]

On Sat, Aug 18, 2012 at 05:23:17PM +0100, Phil Holmes wrote:
> ----- Original Message ----- From: "Phil Holmes"
> <mail@philholmes.net>
> To: "Graham Percival" <graham@percival-music.ca>
> Cc: <PhilEHolmes@googlemail.com>; <john.mandereau@gmail.com>;
> >So we have fixed it.  You want I should find out when?
> 
> Between 2.15.18 and 2.15.21

Given that 2.15.21 was released on 2011 Dec 6, why were we working
on issue 1933 on 2012 Jan 12 ?  was it just a misunderstanding
about which GUB versions had the problem?

I have the same feeling as I do for the LILYPOND_RELOCATE_PREFIX.
I'm not totally opposed to the patch going into master, but I
would not be at all surprised if it broke something.  I'd feel a
lot better if you made some binaries and got some user feedback
from people using them on windows (ideally with at least 3
different versions of windows).

- Graham

[t.daniels_treda.co.uk, 2012-08-18 21:03:54]

Graham Percival wrote Saturday, August 18, 2012 7:34 PM


> On Sat, Aug 18, 2012 at 05:23:17PM +0100, Phil Holmes wrote:
>
>> Between 2.15.18 and 2.15.21
> 
> Given that 2.15.21 was released on 2011 Dec 6, why were we working
> on issue 1933 on 2012 Jan 12 ?  was it just a misunderstanding
> about which GUB versions had the problem?

Issue 1933 was originally reported just before 25 Sep 2011 as a bug 
in 2.15.12, although I believe it first appeared in 2.15.10, following a 
patch from Reinhold.

It lay dormant as a critical issue until 30 Dec 2011, when Carl began trying
to get GUB to work in order to investigate it.  That came to nothing.  

On 11 Jan 2012 Graham began building a series of special releases which I
tested in order to isolate the problems.  There were several, including the
msvcrt one, which were eventually all fixed in 
ab8dfc6e8f3de3cefc8fab09a4993acaec460720 committed on 12 Jan 2012.  

This fix first appeared in 2.15.25.  AFAIK it's been ok since then.  I've
used lilypond-book from several recent releases, including 2.15.95, under
Windows Vista without any problems, not even the one Phil is trying to
fix.

I don't know what Phil thinks was fixed between 2.15.18 and 2.15.21,
but I do suspect the symptoms of the problem or problems changed 
from release to release, and may even depend on the Windows setup.
 
> I have the same feeling as I do for the LILYPOND_RELOCATE_PREFIX.
> I'm not totally opposed to the patch going into master, but I
> would not be at all surprised if it broke something.  I'd feel a
> lot better if you made some binaries and got some user feedback
> from people using them on windows (ideally with at least 3
> different versions of windows).

Happy to test a binary Phil, if you can build one.

Trevor

[t.daniels_treda.co.uk, 2012-08-18 21:37:17]

Trevor Daniels wrote Saturday, August 18, 2012 10:03 PM


> I don't know what Phil thinks was fixed between 2.15.18 and 2.15.21,
> but I do suspect the symptoms of the problem or problems changed 
> from release to release, and may even depend on the Windows setup.

I've just turned up an email from me to Carl dated 4 Jan 2012 which 
suggests the problem /did/ change from the originally reported msvcrt 
one to this in 2.15.23:

>  File "c:\Program Files\LilyPond\usr\lib\python2.4\subprocess.py", line 
> 549, in __init__
>    self.stdout = os.fdopen(c2pread, 'rU', bufsize)
> OSError: [Errno 22] Invalid argument
> Lilypond-book returned code 1

Then we found problems with subprocess.Popen and subprocess.PIPE,
with sleep (which was a Solaris hack), and with the setting for 
universal_newlines.  And the definition of cmd was wrong for Windows.

All these were fixed by Graham's patch, which was actually pushed to
git on 15 Jan.

Trevor

[email_philholmes.net, 2012-08-19 09:34:36]

----- Original Message ----- 
From: "Trevor Daniels" <t.daniels@treda.co.uk>
To: "Graham Percival" <graham@percival-music.ca>; "Phil Holmes" 
<email@philholmes.net>
Cc: <reply@codereview-hr.appspotmail.com>; <PhilEHolmes@googlemail.com>; 
<lilypond-devel@gnu.org>; <reinhold.kainhofer@gmail.com>
Sent: Saturday, August 18, 2012 10:03 PM
Subject: Re: Removes popen3 deprecated warning (issue 6471043)


>
> Graham Percival wrote Saturday, August 18, 2012 7:34 PM
>
>
>> On Sat, Aug 18, 2012 at 05:23:17PM +0100, Phil Holmes wrote:
>>
>>> Between 2.15.18 and 2.15.21
>>
>> Given that 2.15.21 was released on 2011 Dec 6, why were we working
>> on issue 1933 on 2012 Jan 12 ?  was it just a misunderstanding
>> about which GUB versions had the problem?
>
> Issue 1933 was originally reported just before 25 Sep 2011 as a bug
> in 2.15.12, although I believe it first appeared in 2.15.10, following a
> patch from Reinhold.
>
> It lay dormant as a critical issue until 30 Dec 2011, when Carl began 
> trying
> to get GUB to work in order to investigate it.  That came to nothing.
>
> On 11 Jan 2012 Graham began building a series of special releases which I
> tested in order to isolate the problems.  There were several, including 
> the
> msvcrt one, which were eventually all fixed in
> ab8dfc6e8f3de3cefc8fab09a4993acaec460720 committed on 12 Jan 2012.
>
> This fix first appeared in 2.15.25.  AFAIK it's been ok since then.  I've
> used lilypond-book from several recent releases, including 2.15.95, under
> Windows Vista without any problems, not even the one Phil is trying to
> fix.
>
> I don't know what Phil thinks was fixed between 2.15.18 and 2.15.21,
> but I do suspect the symptoms of the problem or problems changed
> from release to release, and may even depend on the Windows setup.

See my other note - if you have stashes of your installs, have a look at

C:\Program Files (x86)\LilyPond\usr\lib\python2.4\config\config.c

You'll see that at 15.18 there's no mention of msvcrt but in 15.21 it's 
there.  So something (but I don't know what) caused us to start delivering 
msvcrt-linked python at that time.  There's no binary needed to test this 
one - you could update/download an updated book_snippets.py, or I could send 
you one, if you want?

>> I have the same feeling as I do for the LILYPOND_RELOCATE_PREFIX.
>> I'm not totally opposed to the patch going into master, but I
>> would not be at all surprised if it broke something.  I'd feel a
>> lot better if you made some binaries and got some user feedback
>> from people using them on windows (ideally with at least 3
>> different versions of windows).
>
> Happy to test a binary Phil, if you can build one.


It wouldn't help, in my opinion - this one is not a windows problem, and 
unless you have an environment variable LILYPOND_RELOCATE_PREFIX set to a 
specific directory, then you won't be affected.  You could check by typing 
SET into a command prompt.

--
Phil Holmes 

[t.daniels_treda.co.uk, 2012-08-19 20:09:50]

Phil Holmes

> See my other note - if you have stashes of your installs, have a look at
> 
> C:\Program Files (x86)\LilyPond\usr\lib\python2.4\config\config.c
> 
> You'll see that at 15.18 there's no mention of msvcrt but in 15.21 it's 
> there.  So something (but I don't know what) caused us to start delivering 
> msvcrt-linked python at that time.  There's no binary needed to test this 
> one - you could update/download an updated book_snippets.py, or I could send 
> you one, if you want?

I don't have the versions of 15 between .13 and .21 installed, but I can
confirm that config.c does not contain msvcrt in .13 but config.c in .21 and 
later all do.  So you've definitely found the source of the msvcrt problem -
the python installation on the machine used to run GUB must have changed
between 15.18 and 15.21.  That explains why the symptoms of issue 1933
changed between Sept 11 and Dec 11 too.

It probably isn't worth pursuing this any further as it's no longer an issue,
unless Graham has any recollection of what might have changed between
the .18 and .21 builds.  It would be nice to know, just in case it reappears.

Trevor