lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: make check currently fails (beam-skip.ly, commit7bcdd37be15ece09cd97


From: Phil Holmes
Subject: Re: make check currently fails (beam-skip.ly, commit7bcdd37be15ece09cd97841137b075a576bbe696)
Date: Sun, 26 Jun 2011 17:55:58 +0100

----- Original Message ----- From: "Graham Percival" <address@hidden>
To: "Phil Holmes" <address@hidden>
Cc: "Reinhold Kainhofer" <address@hidden>; <address@hidden>
Sent: Sunday, June 26, 2011 1:47 PM
Subject: Re: make check currently fails (beam-skip.ly, commit7bcdd37be15ece09cd97841137b075a576bbe696)


On Sun, Jun 26, 2011 at 10:46:15AM +0100, Phil Holmes wrote:
----- Original Message ----- From: "Graham Percival"
<address@hidden>
>I have reverted that commit; the build should behave as usual now.
>I'll take another look at this tomorrow, and we'll go through
>another "patch countdown" for whatever new patch we have.

I reckon this problem would go away if we adopted the "hide output
if QUIET_BUILD is defined" - but I would like to discuss this, as
stated earlier.  I'll hold off any further work pending this.

I'd like to treat
 lilypond-book --redirect-lilypond-output
as a separate feature request from how (if at all) we use it in
the lilypond build system.  I mean, some people use lilypond
inside their own build systems (see today's message on -user about
makefiles), and it would be useful to do more debugging+testing of
--redirect-lilypond-output before re-proposing using it in our
build system.

For example, here's an excerpt of the pre-reverted lilylib.py:

   if redirect_output:
       stdout_filename = ''.join([log_file, '.log'])
       stderr_filename = ''.join([log_file, '.err.log'])
       stdout_setting = open(stdout_filename, 'w')
       stderr_setting = open(stderr_filename, 'w')

   proc = subprocess.Popen (cmd,
                            shell=True,
                            universal_newlines=True,
                            stdout=stdout_setting,
                            stderr=stdout_setting)

... yeah.  No wonder Reinhold saw a 0-byte .err.log file!

Yep - think that would explain it. I'd guess we'd get a long .err.log file and a zero byte .log file if this were corrected.

You missed it, I missed it, and the entire development community
missed it in the 48-hour patch countdown process.  Stuff happens;
there's no value in making a big deal out of it.  We just think
about how it happened, and consider adjusting our safeguards.
(in this case, I think it falls within the bounds of "occasional
error" and there's no need to adjust our patch policies)

While looking at this, I was a bit confused about stdout_setting:
139     stdout_setting = None
141     if not show_progress:
142         stdout_setting = subprocess.PIPE
144     if redirect_output:
147         stdout_setting = open(stdout_filename, 'w')

... what's the final value of stdout_setting for any given set of
function arguments?  ok, when I've isolated those lines, it's not
too hard to track -- but I'm wondering if a more "functional
programming" approach might be easier to read.  Namely, "binding
variables", meaning "you can only assign a value to each variable
once".  I think a nested if:else: construct might make it easier
to track what the final value of stdout_setting is going to be.

Part of the rationale of this code was to make as small a change as possible in the original - so I did not change any aspect of the show_progress flag in case this was being used for something else. As you say, having both flags set would lead to over-writing the first, but you would not be likely to do that, since they're neither common flags and somewhat mutually exclusive.

Could I interest you (or anybody else) in re-submitting a patch
for the
 lilypond-book --redirect-lilypond-output
feature?  Given the past few days, I'm optimistic that we'll see
more interest in reviewing that patch.  Then once we finally *do*
settle on a general policy for build system output, we've got all
(or at least some more?) of the "building blocks" that would be
needed to implement that policy.

This would just be as a standalone change, not linked to make at present - is that your idea?

--
Phil Holmes





reply via email to

[Prev in Thread] Current Thread [Next in Thread]