Fix issue #1852: manuals needs more explicit dependencies.

Fix issue #1852: manuals needs more explicit dependencies.

Dependencies are already being tracker by lilypond-book.py,
which produces .dep files containing make rules. The file
stepmake/stepmake/generic-targets.make includes them into
the build.

The problem is that the .dep files contain errors.

We fix the .dep files produced by lilypond-book.py by
  1) using the relative path to the target file,
  2) using the correct file extension for the target file.

[janek, 2011-09-09 19:02:05]

By Julien Rioux.

[janek, 2011-09-09 19:19:47]

Hi,

as usual my review will contain only questions...
Please pardon my ignorance, i don't know what this area of the code does. 
However, what your patch does seems reasonable to me.

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (left):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
scripts/lilypond-book.py:691: + '.%s' % global_options.format)
Pardon my ignorance, but what the
'.%s' %
part was doing before?

[Graham Percival (old account), 2011-09-09 19:28:29]

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (right):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode677
scripts/lilypond-book.py:677: relative_output_dir = global_options.output_dir
did you want an
  os.path.relpath()
here or something?  I don't see the point of defining relative_output_dir
instead of just keeping global_options.output_dir down on line 690.

[Reinhold, 2011-09-09 19:35:45]

LGTM, although I would also move that code changing global_options.output_dir
from do_file to main, to make things much clearer!

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (left):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
scripts/lilypond-book.py:691: + '.%s' % global_options.format)
On 2011/09/09 19:19:47, janek wrote:
> Pardon my ignorance, but what the
> '.%s' %
> part was doing before?

It simply prepended the "." and the correct file extension to the base name
(which is the file name without extension).

Granted, writing it as 
  base_file_name + "." + global_options.format
would have given the same result...

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (right):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode539
scripts/lilypond-book.py:539: global_options.output_dir =
os.path.abspath(global_options.output_dir)
Ouch, that's the real culprit here: Why do we change the GLOBAL options in the
function to process one file? That code should be moved out of do_file and into
main, so the option handling is easier to understand!

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode677
scripts/lilypond-book.py:677: relative_output_dir = global_options.output_dir
On 2011/09/09 19:28:29, Graham Percival wrote:
> did you want an
>   os.path.relpath()
> here or something?  I don't see the point of defining relative_output_dir
> instead of just keeping global_options.output_dir down on line 690.

The Problem is that do_file above CHANGES the global_options.output_dir to an
absolute path... 
It took me a while to figure that out, too.

[Julien Rioux, 2011-09-09 19:38:07]

yes, you guys got it right with the 'hello %s' % 'world' syntax
and yes, it's kind of bad form to change global options within that
loop, but I didn't feel like telling you what not to do :)

Julien

On Fri, Sep 9, 2011 at 9:35 PM,  <reinhold.kainhofer@gmail.com> wrote:
> LGTM, although I would also move that code changing
> global_options.output_dir from do_file to main, to make things much
> clearer!
>
>
> http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
> File scripts/lilypond-book.py (left):
>
>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
> scripts/lilypond-book.py:691: + '.%s' % global_options.format)
> On 2011/09/09 19:19:47, janek wrote:
>>
>> Pardon my ignorance, but what the
>> '.%s' %
>> part was doing before?
>
> It simply prepended the "." and the correct file extension to the base
> name (which is the file name without extension).
>
> Granted, writing it as
>  base_file_name + "." + global_options.format
> would have given the same result...
>
> http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
> File scripts/lilypond-book.py (right):
>
>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode539
> scripts/lilypond-book.py:539: global_options.output_dir =
> os.path.abspath(global_options.output_dir)
> Ouch, that's the real culprit here: Why do we change the GLOBAL options
> in the function to process one file? That code should be moved out of
> do_file and into main, so the option handling is easier to understand!
>
>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode677
> scripts/lilypond-book.py:677: relative_output_dir =
> global_options.output_dir
> On 2011/09/09 19:28:29, Graham Percival wrote:
>>
>> did you want an
>>   os.path.relpath()
>> here or something?  I don't see the point of defining
>
> relative_output_dir
>>
>> instead of just keeping global_options.output_dir down on line 690.
>
> The Problem is that do_file above CHANGES the global_options.output_dir
> to an absolute path...
> It took me a while to figure that out, too.
>
> http://codereview.appspot.com/4996044/
>

[Julien Rioux, 2011-09-09 19:39:45]

And global_options.format contains "texinfo" while
global_options.formatter.default_extension contains ".texi"

On Fri, Sep 9, 2011 at 9:38 PM, Julien Rioux <julien.rioux@gmail.com> wrote:
> yes, you guys got it right with the 'hello %s' % 'world' syntax
> and yes, it's kind of bad form to change global options within that
> loop, but I didn't feel like telling you what not to do :)
>
> Julien
>
> On Fri, Sep 9, 2011 at 9:35 PM,  <reinhold.kainhofer@gmail.com> wrote:
>> LGTM, although I would also move that code changing
>> global_options.output_dir from do_file to main, to make things much
>> clearer!
>>
>>
>> http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
>> File scripts/lilypond-book.py (left):
>>
>>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
>> scripts/lilypond-book.py:691: + '.%s' % global_options.format)
>> On 2011/09/09 19:19:47, janek wrote:
>>>
>>> Pardon my ignorance, but what the
>>> '.%s' %
>>> part was doing before?
>>
>> It simply prepended the "." and the correct file extension to the base
>> name (which is the file name without extension).
>>
>> Granted, writing it as
>>  base_file_name + "." + global_options.format
>> would have given the same result...
>>
>> http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
>> File scripts/lilypond-book.py (right):
>>
>>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode539
>> scripts/lilypond-book.py:539: global_options.output_dir =
>> os.path.abspath(global_options.output_dir)
>> Ouch, that's the real culprit here: Why do we change the GLOBAL options
>> in the function to process one file? That code should be moved out of
>> do_file and into main, so the option handling is easier to understand!
>>
>>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode677
>> scripts/lilypond-book.py:677: relative_output_dir =
>> global_options.output_dir
>> On 2011/09/09 19:28:29, Graham Percival wrote:
>>>
>>> did you want an
>>>   os.path.relpath()
>>> here or something?  I don't see the point of defining
>>
>> relative_output_dir
>>>
>>> instead of just keeping global_options.output_dir down on line 690.
>>
>> The Problem is that do_file above CHANGES the global_options.output_dir
>> to an absolute path...
>> It took me a while to figure that out, too.
>>
>> http://codereview.appspot.com/4996044/
>>
>

[Graham Percival (old account), 2011-09-09 19:40:28]

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (right):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#newcode681
scripts/lilypond-book.py:681: chunks = do_file (files[0])
you mean "that do_file BELOW", right?

woah, you're right (line 539).  That's an obscene side effect of the function!

[janek, 2011-09-09 19:41:06]

2011/9/9  <reinhold.kainhofer@gmail.com>:
>
http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
> scripts/lilypond-book.py:691: + '.%s' % global_options.format)
> On 2011/09/09 19:19:47, janek wrote:
>>
>> Pardon my ignorance, but what the
>> '.%s' %
>> part was doing before?
>
> It simply prepended the "." and the correct file extension to the base
> name (which is the file name without extension).
>
> Granted, writing it as
>  base_file_name + "." + global_options.format
> would have given the same result...

ok, thanks!
Janek

[Julien Rioux, 2011-09-09 19:42:23]

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py
File scripts/lilypond-book.py (left):

http://codereview.appspot.com/4996044/diff/1/scripts/lilypond-book.py#oldcode691
scripts/lilypond-book.py:691: + '.%s' % global_options.format)
On 2011/09/09 19:19:47, janek wrote:
> Pardon my ignorance, but what the
> '.%s' %
> part was doing before?

'hello %s' % 'world'
This is a syntax for string templates in python. The %s inside the first string
is a spaceholder. The % followed by the second string means: substitute this
second string into the placeholder of the first string. After python processes
it you end up with the string 'hello world'.

In the case here
'.%s' % global_options.format
it basically takes the string global_options.format and creates a new string
with a dot in front. This was necessary because global_options.format is e.g.
'texinfo' and did not include the dot for a filename extension.

In the new code we use global_options.formatter.default_extension
which is a string with the dot already included, and of course it contains the
correct extension, e.g. '.texi'.

[pkx166h, 2011-09-10 13:01:07]

passes make and reg tests

[janek, 2011-09-17 21:33:33]

pushed by Graham as 1b99f1907fb77b0f3a0e65725776782c3eeaa025 (thanks - i didn't
have access to mi Lilydev machine lately).

I close Rietveld issue.

Thanks Julien!
Janek