Directs makeinfo and texi2html output to logfiles

New patch set to address JR and GP's helpful comments.  I've used $(buildscript-dir)/run-and-check rather than setting another env variable - this is equally as short and so I think OK.  Name changed.  I'm thinking this is good to go now - please confirm.

===============================================

This uses a shell script that I've written to direct the output from a command to a logfile, and to issue a line identifying the logfile if the command fails.  I've updated the calls (all of them, I believe) to makeinfo and texi2html to use this script.  The result is about 3400 fewer lines output from make doc, and lots of lovely logfiles to check for errors - it makes debugging docs a lot easier.  It's slightly non-trivial to test, since neither makeinfo or texi2html stops on errored input by default.  If you force it to do so (see below) sample output is:

/home/phil/lilypond-git/scripts/build/CheckAndRun.sh "DEPTH=../../.. AJAX_SEARCH= TOP_SRC_DIR=/home/phil/lilypond-git PERL_UNICODE=SD texi2html --error-limit=0 --I=/home/phil/lilypond-git/input/regression/lilypond-book --I=./out-www -I /home/phil/lilypond-git/Documentation --I=/media/IntelSSD/lilypond/lilypond-git/build/./out-www/xref-maps --init-file=/home/phil/lilypond-git/Documentation/lilypond-texi2html.init  --output=out-www/collated-files.html out-www/collated-files.texi"  "collated-files.texilog.log"

Please check the logfile collated-files.texilog.log for errors

make[4]: *** [out-www/collated-files.html] Error 2
make[4]: Leaving directory `/media/IntelSSD/lilypond/lilypond-git/build/input/regression/lilypond-book'
make[3]: *** [WWW-2] Error 2
make[3]: Leaving directory `/media/IntelSSD/lilypond/lilypond-git/build/input/regression'
make[2]: *** [WWW-2] Error 2
make[2]: Leaving directory `/media/IntelSSD/lilypond/lilypond-git/build/input'
make[1]: *** [WWW-2] Error 2
make[1]: Leaving directory `/media/IntelSSD/lilypond/lilypond-git/build'
make: *** [doc] Error 2
Command exited with non-zero status 2

The logfile that's fingered contains this:

*** Duplicate node found: Top (in out-www/papersize-docs.texi l. 8)
Max error number exceeded

To show this in action, you would need to edit texinfo-vars.make as below.
-TEXI2HTML_FLAGS += $(TEXI2HTML_INCLUDES) $(TEXI2HTML_INIT) $(TEXI2HTML_LANG)
+TEXI2HTML_FLAGS += --error-limit=0 $(TEXI2HTML_INCLUDES) $(TEXI2HTML_INIT) $(TEXI2HTML_LANG)

[PhilEHolmes, 2012-02-07 15:37:40]

Slightly complicated (for me, anyway) but please review and ask questions if
it's not clear what I'm doing here.

[Graham Percival, 2012-02-07 19:49:46]

Looks basically good, I just have some trivial quibbles.  This should vastly
simplify debugging doc problems, though!

http://codereview.appspot.com/5645046/diff/1/Documentation/GNUmakefile
File Documentation/GNUmakefile (right):

http://codereview.appspot.com/5645046/diff/1/Documentation/GNUmakefile#newcod...
Documentation/GNUmakefile:198: $(top-src-dir)/scripts/build/CheckAndRun.sh
"$(MAKEINFO) -I$(src-dir) -I$(outdir) --output=$@ $<"  "$*.makeinfo.log"
Could this be condensed into
$(RUN_WITH_LOG)
or something like that?  i.e. define
RUN_WITH_LOG = $(top-src-dir)/scripts/build/CheckAndRun.sh
somewhere in a "main" build file (maybe stepmake.make?) and then just call that
definition everywhere else?

http://codereview.appspot.com/5645046/diff/1/scripts/build/CheckAndRun.sh
File scripts/build/CheckAndRun.sh (right):

http://codereview.appspot.com/5645046/diff/1/scripts/build/CheckAndRun.sh#new...
scripts/build/CheckAndRun.sh:3: # CheckAndRun.sh.  Takes 2 parameters - the
command to be run and
sorry, totally silly complaint, but I wince every time I see this: unix people
are generally allergic to Capital letters.  Maybe because it requires pressing
shift, and the whole point of the command-line is to be lazy?

Could this be check_and_run.sh or check-and-run.sh ?  I think that would fit in
with other scripts/build/* items as well.

Also, technically I'd call it run-and-check.sh, since it runs the command first,
and then checks the results afterwards.

[Julien Rioux, 2012-02-07 20:22:11]

Graham basically covered it...

http://codereview.appspot.com/5645046/diff/1/Documentation/GNUmakefile
File Documentation/GNUmakefile (right):

http://codereview.appspot.com/5645046/diff/1/Documentation/GNUmakefile#newcod...
Documentation/GNUmakefile:198: $(top-src-dir)/scripts/build/CheckAndRun.sh
"$(MAKEINFO) -I$(src-dir) -I$(outdir) --output=$@ $<"  "$*.makeinfo.log"
Can you use $(buildscript-dir) here? That could make these lines a bit shorter.

[Julien Rioux, 2012-02-07 20:23:39]

On 2012/02/07 19:49:46, Graham Percival wrote:
>   i.e. define
> RUN_WITH_LOG = $(top-src-dir)/scripts/build/CheckAndRun.sh
> somewhere in a "main" build file (maybe stepmake.make?) and then just call
that
> definition everywhere else?

in make/genericgeneric-vars.make, I would think

[PhilEHolmes, 2012-02-08 12:10:22]

Updates pretty much as suggested.  Please review and agree to push.

[Graham Percival, 2012-02-08 12:44:30]

LGTM, but I think it should go through a normal countdown.

[Julien Rioux, 2012-02-08 12:56:34]

I'm not sure why the run-and-check.sh script needs to be so complicated.

http://codereview.appspot.com/5645046/diff/1005/scripts/build/run-and-check.sh
File scripts/build/run-and-check.sh (right):

http://codereview.appspot.com/5645046/diff/1005/scripts/build/run-and-check.s...
scripts/build/run-and-check.sh:3: # CheckAndRun.sh.  Takes 2 parameters - the
command to be run and
run-and-check.sh

http://codereview.appspot.com/5645046/diff/1005/scripts/build/run-and-check.s...
scripts/build/run-and-check.sh:14: exec 1>&6 6>&- # Restore stdout and close
file descriptor #6
Wouldn't this be sufficient:
$1 >& $2
RetVal=$?

why is all the extra redirection needed?
why is exec and eval needed?

[mail_philholmes.net, 2012-02-08 13:20:57]

----- Original Message ----- 
From: <julien.rioux@gmail.com>
To: <PhilEHolmes@googlemail.com>; <dak@gnu.org>; <graham@percival-music.ca>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Wednesday, February 08, 2012 12:56 PM
Subject: Re: Directs makeinfo and texi2html output to logfiles (issue 
5645046)


> I'm not sure why the run-and-check.sh script needs to be so complicated.

Nor me.  But I did a lot of experimentation to get it to work.  I think all 
the extra redirection and the exec stuff is required because you're running 
a command inside a shell script, and so simply trying:

command 2>&1

doesn't work.  There's lots of stuff on the net about needing to use exec.

If you don't use eval, the shell tries to run the first "command" it sees, 
which is "DEPTH=$(depth)/../" and this fails as a command.  So the way round 
that was to use eval.

As you know, I'm no unix script expert.  But I'd be surprised if there was a 
simpler way to actually make this work.


--
Phil Holmes

[mail_philholmes.net, 2012-02-08 13:26:02]

----- Original Message ----- 
From: <graham@percival-music.ca>
To: <PhilEHolmes@googlemail.com>; <dak@gnu.org>; <julien.rioux@gmail.com>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Wednesday, February 08, 2012 12:44 PM
Subject: Re: Directs makeinfo and texi2html output to logfiles (issue 
5645046)


> LGTM, but I think it should go through a normal countdown.


The reason I was suggesting an early push was that cleaning up make doc is a 
cumulative affair, and I find it easier to manage changes/patches if I don't 
have multiple differences from master in my build system.  I'd like to get 
one done and into master, then work on proving the next works.

--
Phil Holmes

[Graham Percival, 2012-02-08 13:26:49]

On Wed, Feb 08, 2012 at 01:23:06PM -0000, Phil Holmes wrote:
> The reason I was suggesting an early push was that cleaning up make
> doc is a cumulative affair, and I find it easier to manage
> changes/patches if I don't have multiple differences from master in
> my build system.  I'd like to get one done and into master, then
> work on proving the next works.

well... ok, I'm fine with that as long as Julien says it's ok.

- Graham

[Julien Rioux, 2012-02-08 13:35:42]

On Wed, Feb 8, 2012 at 2:20 PM, Phil Holmes <mail@philholmes.net> wrote:
> ----- Original Message ----- From: <julien.rioux@gmail.com>
> To: <PhilEHolmes@googlemail.com>; <dak@gnu.org>; <graham@percival-music.ca>
> Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
> Sent: Wednesday, February 08, 2012 12:56 PM
> Subject: Re: Directs makeinfo and texi2html output to logfiles (issue
> 5645046)
>
>
>
>> I'm not sure why the run-and-check.sh script needs to be so complicated.
>
>
> Nor me.  But I did a lot of experimentation to get it to work.  I think all
> the extra redirection and the exec stuff is required because you're running
> a command inside a shell script, and so simply trying:
>
> command 2>&1
>
> doesn't work.  There's lots of stuff on the net about needing to use exec.
>
> If you don't use eval, the shell tries to run the first "command" it sees,
> which is "DEPTH=$(depth)/../" and this fails as a command.  So the way round
> that was to use eval.
>
> As you know, I'm no unix script expert.  But I'd be surprised if there was a
> simpler way to actually make this work.
>
>
> --
> Phil Holmes
>
>

Yes, the DEPTH=... confuses bash, so eval is needed. Please try
something like the attached though. It removes all the exec stuff and
will be much easier to understand for future contributors. The exec
stuff is needed only for complex redirection, for example if you
wanted stderr to appear both on the terminal and in the log file. But
currently we just redirect both stdout and stderr to the log file, and
this scenario does not need complex exec stuff.

Regards,
Julien

[Julien Rioux, 2012-02-08 13:39:23]

On Wed, Feb 8, 2012 at 2:26 PM, Graham Percival
<graham@percival-music.ca> wrote:
> On Wed, Feb 08, 2012 at 01:23:06PM -0000, Phil Holmes wrote:
>> The reason I was suggesting an early push was that cleaning up make
>> doc is a cumulative affair, and I find it easier to manage
>> changes/patches if I don't have multiple differences from master in
>> my build system.  I'd like to get one done and into master, then
>> work on proving the next works.
>
> well... ok, I'm fine with that as long as Julien says it's ok.
>
> - Graham

It seems that simple patches have been allowed to be pushed without
review from time to time, but an inability to manage multiple patches
locally should be no reason to do so. That being said, if Phil has
tested his work with full make doc, then I would feel pretty confident
about this patch.

Regards,
Julien

[dak, 2012-02-08 13:40:13]

"Phil Holmes" <mail@philholmes.net> writes:

> ----- Original Message ----- 
> From: <julien.rioux@gmail.com>
> To: <PhilEHolmes@googlemail.com>; <dak@gnu.org>; <graham@percival-music.ca>
> Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
> Sent: Wednesday, February 08, 2012 12:56 PM
> Subject: Re: Directs makeinfo and texi2html output to logfiles (issue
> 5645046)
>
>
>> I'm not sure why the run-and-check.sh script needs to be so complicated.
>
> Nor me.  But I did a lot of experimentation to get it to work.  I
> think all the extra redirection and the exec stuff is required because
> you're running a command inside a shell script, and so simply trying:
>
> command 2>&1
>
> doesn't work.

Why wouldn't it?

> There's lots of stuff on the net about needing to use exec.

There is a whole lot of nonsense "on the net".  That does not mean
anything.

> If you don't use eval, the shell tries to run the first "command" it
> sees, which is "DEPTH=$(depth)/../" and this fails as a command.  So
> the way round that was to use eval.

This seriously sounds rather fishy.

> As you know, I'm no unix script expert.  But I'd be surprised if there
> was a simpler way to actually make this work.

I'm a unix script expert so to say, but I can't keep up with running the
whole show.  I am already juggling too many tasks related to LilyPond,
and all of the juggling costs more time than the individual tasks do
since it breaks concentration, and I still have to write up a
publication for the LilyPond Report explaning to people why they should
bother paying me for tasks I am doing "voluntarily" because noone
*fscking* else does and they are the right thing to do.  I am currently
quitting my choir (just need to get the outfit through the dry cleaner)
because I have been told that it is indecent of me to mention the
efforts I take for doing decidedly more than my share of the work
because "you would not need to do this if you did not want to".  What a
convenient excuse to grab all one can get.

Sorry, you (singular) don't deserve this.  I am just in a bad mood.

From the sounds of it, it would appear to be that you are doing
something too complex here.  Most certainly
command 2>&1
should work, and you can group commands like
{
  command
  command
  command
} 2>&1

as well if you don't want to patch up single commands.

-- 
David Kastrup

[mail_philholmes.net, 2012-02-08 14:05:31]

----- Original Message ----- 
From: "Julien Rioux" <julien.rioux@gmail.com>
To: "Phil Holmes" <mail@philholmes.net>
> Yes, the DEPTH=... confuses bash, so eval is needed. Please try
> something like the attached though. It removes all the exec stuff and
> will be much easier to understand for future contributors. The exec
> stuff is needed only for complex redirection, for example if you
> wanted stderr to appear both on the terminal and in the log file. But
> currently we just redirect both stdout and stderr to the log file, and
> this scenario does not need complex exec stuff.
>
> Regards,
> Julien

eval $1 >& $2

gives:

./new-run-check.sh: 2: Syntax error: Bad fd number - finger trouble on the 
redirect.  eval $1 &> $2 simply doesn't redirect the output - it appears on 
the terminal and no logfile is created.  I spent quite a long time a few 
weeks ago getting this to work and was surprised it didn't, but convinced 
myself that the way I arrived at is pretty much the only solution.  I 
understand what you say about maintainability, but I've commented it quite a 
bit and the script could just be removed if it was too confusing, so I'm not 
convinced it's worth too much time repeating what I did earlier to try to 
get this to work.

--
Phil Holmes

[dak, 2012-02-08 14:28:25]

"Phil Holmes" <mail@philholmes.net> writes:

> ----- Original Message ----- 
> From: "Julien Rioux" <julien.rioux@gmail.com>
> To: "Phil Holmes" <mail@philholmes.net>
>> Yes, the DEPTH=... confuses bash, so eval is needed. Please try
>> something like the attached though. It removes all the exec stuff and
>> will be much easier to understand for future contributors. The exec
>> stuff is needed only for complex redirection, for example if you
>> wanted stderr to appear both on the terminal and in the log file. But
>> currently we just redirect both stdout and stderr to the log file, and
>> this scenario does not need complex exec stuff.
>>
>> Regards,
>> Julien
>
> eval $1 >& $2

What is >& $2 supposed to be?

-- 
David Kastrup

[Julien Rioux, 2012-02-08 14:28:42]

On Wed, Feb 8, 2012 at 3:05 PM, Phil Holmes <mail@philholmes.net> wrote:
> ----- Original Message ----- From: "Julien Rioux" <julien.rioux@gmail.com>
> To: "Phil Holmes" <mail@philholmes.net>
>
>> Yes, the DEPTH=... confuses bash, so eval is needed. Please try
>> something like the attached though. It removes all the exec stuff and
>> will be much easier to understand for future contributors. The exec
>> stuff is needed only for complex redirection, for example if you
>> wanted stderr to appear both on the terminal and in the log file. But
>> currently we just redirect both stdout and stderr to the log file, and
>> this scenario does not need complex exec stuff.
>>
>> Regards,
>> Julien
>
>
> eval $1 >& $2
>
> gives:
>
> ./new-run-check.sh: 2: Syntax error: Bad fd number - finger trouble on the
> redirect.  eval $1 &> $2 simply doesn't redirect the output - it appears on
> the terminal and no logfile is created.  I spent quite a long time a few
> weeks ago getting this to work and was surprised it didn't, but convinced
> myself that the way I arrived at is pretty much the only solution.  I
> understand what you say about maintainability, but I've commented it quite a
> bit and the script could just be removed if it was too confusing, so I'm not
> convinced it's worth too much time repeating what I did earlier to try to
> get this to work.
>
> --
> Phil Holmes
>

I guess I made a typo and it should be
eval $1 &> $2                        # Redirect both stdout and stderr
to log file.

but apparently my version of bash understands either &> or >&, and it
works perfectly:

$ ./run.sh "DEPTH=../.. ls run.sh x" mylogfile
An error occured. Please check the logfile mylogfile
$ cat mylogfile
ls: cannot access x: No such file or directory
run.sh

Are you sure you run the script with bash? What do you get from bash
--version? I have bash --version
GNU bash, version 4.1.10(1)-release (x86_64-suse-linux-gnu)
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Cheers,
Julien

[dak, 2012-02-08 14:31:13]

Julien Rioux <julien.rioux@gmail.com> writes:

> I guess I made a typo and it should be
> eval $1 &> $2                        # Redirect both stdout and stderr
> to log file.
>
> but apparently my version of bash understands either &> or >&, and it
> works perfectly:

Please refrain from entirely unneeded bashisms as they make it harder to
understand the code for others.

eval $1 > $2 2>&1

works on every Bourne shell.

-- 
David Kastrup

[Julien Rioux, 2012-02-08 14:37:30]

On Wed, Feb 8, 2012 at 3:31 PM, David Kastrup <dak@gnu.org> wrote:
> Julien Rioux <julien.rioux@gmail.com> writes:
>
>> I guess I made a typo and it should be
>> eval $1 &> $2                        # Redirect both stdout and stderr
>> to log file.
>>
>> but apparently my version of bash understands either &> or >&, and it
>> works perfectly:
>
> Please refrain from entirely unneeded bashisms as they make it harder to
> understand the code for others.
>
> eval $1 > $2 2>&1
>
> works on every Bourne shell.
>
> --
> David Kastrup

Let's go with that then.
Regards,
Julien

[mail_philholmes.net, 2012-02-08 14:42:41]

----- Original Message ----- 
From: "David Kastrup" <dak@gnu.org>
To: "Julien Rioux" <julien.rioux@gmail.com>
Cc: "Phil Holmes" <mail@philholmes.net>; <PhilEHolmes@googlemail.com>; 
<graham@percival-music.ca>; <lilypond-devel@gnu.org>; 
<reply@codereview-hr.appspotmail.com>
Sent: Wednesday, February 08, 2012 2:31 PM
Subject: Re: Directs makeinfo and texi2html output to logfiles (issue 
5645046)


> Julien Rioux <julien.rioux@gmail.com> writes:
>
>> I guess I made a typo and it should be
>> eval $1 &> $2                        # Redirect both stdout and stderr
>> to log file.
>>
>> but apparently my version of bash understands either &> or >&, and it
>> works perfectly:
>
> Please refrain from entirely unneeded bashisms as they make it harder to
> understand the code for others.
>
> eval $1 > $2 2>&1
>
> works on every Bourne shell.


LGTM on my local VM.  I'll test it on my build machine and put up a new 
patch if all is good.  Thanks.

--
Phil Holmes

[Julien Rioux, 2012-02-08 14:56:11]

And one last question:

http://codereview.appspot.com/5645046/diff/1005/stepmake/stepmake/texinfo-rul...
File stepmake/stepmake/texinfo-rules.make (right):

http://codereview.appspot.com/5645046/diff/1005/stepmake/stepmake/texinfo-rul...
stepmake/stepmake/texinfo-rules.make:37: $(buildscript-dir)/run-and-check
"$(MAKEINFO) -I$(src-dir) -I$(outdir) -D web_version --output=$@ $<" 
"$*.makeinfo.log"
Do the log files end up in the $(outdir) ?

I guess not unless we specify "$(outdir)/$*.makeinfo.log", right?

[mail_philholmes.net, 2012-02-08 15:47:32]

----- Original Message ----- 
From: <julien.rioux@gmail.com>
To: <PhilEHolmes@googlemail.com>; <dak@gnu.org>; <graham@percival-music.ca>; 
<mail@philholmes.net>
Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com>
Sent: Wednesday, February 08, 2012 2:56 PM
Subject: Re: Directs makeinfo and texi2html output to logfiles (issue 
5645046)


> And one last question:
>
>
>
http://codereview.appspot.com/5645046/diff/1005/stepmake/stepmake/texinfo-rul...
> File stepmake/stepmake/texinfo-rules.make (right):
>
>
http://codereview.appspot.com/5645046/diff/1005/stepmake/stepmake/texinfo-rul...
> stepmake/stepmake/texinfo-rules.make:37:
> $(buildscript-dir)/run-and-check "$(MAKEINFO) -I$(src-dir) -I$(outdir)
> -D web_version --output=$@ $<"  "$*.makeinfo.log"
> Do the log files end up in the $(outdir) ?
>
> I guess not unless we specify "$(outdir)/$*.makeinfo.log", right?
>
> http://codereview.appspot.com/5645046/
>

Correct - they go into build/Documentation.  The benefit of that is that 
it's a less cluttered directory.  If you think it's important, I could 
easily update it to put them in outdir.  logdir??

--
Phil Holmes