Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread Han-Wen Nienhuys 
Adapted description.

On Fri, Jan 31, 2020 at 8:36 AM Han-Wen Nienhuys  wrote:
>
> On Fri, Jan 31, 2020 at 8:30 AM Han-Wen Nienhuys  wrote:
> > Locally, I have
> >
> > Clean up embedded scheme parsing/evaluation.
> >
> > Renames and reorders functions to clarify the mechanism. No
> > consequential functional changes.
> >
> > Separates input and output parameters.
> >
> > but I can't find a button to edit the change description.
>
> found it. It is in the corner ("edit issue"). It would be nice if
> git-cl upload did this automatically.
>
> --
> Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen



-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread Han-Wen Nienhuys 
On Fri, Jan 31, 2020 at 8:30 AM Han-Wen Nienhuys  wrote:
> Locally, I have
>
> Clean up embedded scheme parsing/evaluation.
>
> Renames and reorders functions to clarify the mechanism. No
> consequential functional changes.
>
> Separates input and output parameters.
>
> but I can't find a button to edit the change description.

found it. It is in the corner ("edit issue"). It would be nice if
git-cl upload did this automatically.

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread Han-Wen Nienhuys 
On Fri, Jan 31, 2020 at 1:31 AM  wrote:
>
> On 2020/01/30 23:22:46, hanwenn wrote:
> > I feel this whole discussion has gone out of hand, and in the interest
> of
> > expediency, I have replaced
> >
> >   const Input*
> >
> > with
> >
> >   Input
> >
> > in the class declaration, so somebody can give this an LGTM now.
>
> Please read the following with a friendly tone of voice.

Thanks for keeping a cool head !

> I'm having the same trouble I initially had trying to match the
> description of this change with its content.  "Renames and reorders
> functions to clarify the mechanism. No functional
> changes."  Yet in patch set 4, there is a difference other than renaming
> and reordering.  Parse_start makes a copy of the Input that it didn't
> before, and it also has an additional Input member.  I don't have the
> background knowledge to say whether this discrepancy is consequential,

It is not.

> just that it doesn't match the description; but that keeps me from
> saying "looks good."  Maybe someone else can.  Either way, I don't think
> it's too much to ask to expand the description to reflect the change
> more accurately.

Locally, I have

Clean up embedded scheme parsing/evaluation.

Renames and reorders functions to clarify the mechanism. No
consequential functional changes.

Separates input and output parameters.

but I can't find a button to edit the change description.

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: PropertySet with unbound value

2020-01-30 Thread Dan Eble 
On Jan 30, 2020, at 19:28, David Kastrup  wrote:
> 
> It's conceivable that the code was dead afterwards.  I am not sure.

Seems likely to me.
https://codereview.appspot.com/557260047/
— 
Dan

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread nine . fierce . ballads 
On 2020/01/30 23:22:46, hanwenn wrote:
> I feel this whole discussion has gone out of hand, and in the interest
of
> expediency, I have replaced 
> 
>   const Input*
> 
> with
> 
>   Input
> 
> in the class declaration, so somebody can give this an LGTM now.

Please read the following with a friendly tone of voice.

I'm having the same trouble I initially had trying to match the
description of this change with its content.  "Renames and reorders
functions to clarify the mechanism. No functional
changes."  Yet in patch set 4, there is a difference other than renaming
and reordering.  Parse_start makes a copy of the Input that it didn't
before, and it also has an additional Input member.  I don't have the
background knowledge to say whether this discrepancy is consequential,
just that it doesn't match the description; but that keeps me from
saying "looks good."  Maybe someone else can.  Either way, I don't think
it's too much to ask to expand the description to reflect the change
more accurately.


https://codereview.appspot.com/577410045/

Re: PropertySet with unbound value

2020-01-30 Thread David Kastrup 
Dan Eble  writes:

> Does anyone have an idea of input I could provide to reach the block with the 
> TODO below?  I've tried 
>
> #(make-music 'PropertySet 'symbol 'fingeringOrientations)
>
> (note that no value is defined), but it wasn't enough.  TIA.
>
> void
> Context::set_property_from_event (SCM sev)
> {
>   Stream_event *ev = unsmob (sev);
>
>   SCM sym = ev->get_property ("symbol");
>   if (scm_is_symbol (sym))
> {
>   SCM val = ev->get_property ("value");
>
>   if (SCM_UNBNDP (val)) {
> // TODO: It looks like this ignores \once.
> // Should this be unset_property_from event (sev)?
> unset_property (sym);
> return;
>   }
> ...
> — 
> Dan

The TODO was added by yourself.  I think that unset_property here was
called when \once \set ... was used on a previously unset property and
the implementation used a finalization-hook for unsetting at the point
the event got generated instead of passing the 'once property in the
Stream_event .  This mechanism was incompatible with the stream event
recorder and was ultimately replaced by me with

commit 314743a9769d8094d23cd45eb307b1485b41cb44
Author: David Kastrup 
Date:   Tue Sep 15 20:50:13 2015 +0200

Issue 4609/4: Move \once action from iterators to listeners

This ends the dependency of the events generated for \once\unset and
\once\set on the current context (bad for recording and replaying events
like with the part combiner and quoted music).  It also implements
\once\revert and makes every \once\override and \once\revert impervious
to any other overrides and reverts that may happen at the same time.

It's conceivable that the code was dead afterwards.  I am not sure.

-- 
David Kastrup

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
On 2020/01/30 14:31:07, dan_faithful.be wrote:
> On Jan 30, 2020, at 04:54, mailto:hanw...@gmail.com wrote:
> > 
> > This may predate you, but the decision to not store references was
> > intentional,
> > exactly because storing NULL in them is very useful.  If you have a
> > reference,
> > it has to be initialized in the constructor, and this introduces a
lot
> > of boilerplate
> > because you have to pass the non-null reference across constructors
in
> > the class
> > hierarchy. 
> 
> The discussion has turned from (a) passing a required parameter as a
reference,
> to (b) using a reference as a member variable.  (a) does not imply
(b).  You can
> pass in a T& and store it in a T* to avoid the constraints that (b)
would place
> on the use of your class (though they apparently were not previously a
problem
> in this case).
> 
> > The current code overwhelmingly disavows references. The 2 remaining
> > uses (this being one) stand out like a sore thumb.
> 
> We must be miscommunicating, because I see a lot more than 2.  For
some
> examples,

There are (were) 2 instance of non-const references passed in .hh files.
One dead method in Score_performer, and Lily_lexer::scan_word

In the lily/ directory

 git grep 'vector<[^>]\+> &' *c|grep -v const

returns 20 results, which is pretty small, given the number of methods
in the code base. A cursory inspection suggests that Mike introduced
most of these, and I would have probably suggested to use pointers there
too. I would also change these signatures if would stumble across them
while refactoring something else. Note that const& are OK as an
optimization of call by value; in the caller, the effect of cont& and
call by value is indistinguishable. 

I feel this whole discussion has gone out of hand, and in the interest
of expediency, I have replaced 

  const Input*

with

  Input

in the class declaration, so somebody can give this an LGTM now.

I also have the feeling we spend a lot of time and energy talking past
each other. How about a short voice/video call between the 3 of us to
make sure we are on the same page?

cheers,


> 
> git grep 'vector<[^>]\+> &'
> 
> Please clarify.
> 
> Thanks,
> — 
> Dan
> 



https://codereview.appspot.com/577410045/

PropertySet with unbound value

2020-01-30 Thread Dan Eble 
Does anyone have an idea of input I could provide to reach the block with the 
TODO below?  I've tried 

#(make-music 'PropertySet 'symbol 'fingeringOrientations)

(note that no value is defined), but it wasn't enough.  TIA.

void
Context::set_property_from_event (SCM sev)
{
  Stream_event *ev = unsmob (sev);

  SCM sym = ev->get_property ("symbol");
  if (scm_is_symbol (sym))
{
  SCM val = ev->get_property ("value");

  if (SCM_UNBNDP (val)) {
// TODO: It looks like this ignores \once.
// Should this be unset_property_from event (sev)?
unset_property (sym);
return;
  }
...
— 
Dan

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Dan Eble  writes:

> On Jan 30, 2020, at 14:57, David Kastrup  wrote:
>> 
>> os.symlink ("../../../out-baseline/share",
>> "out-www/offline-root/input/regression/out-test-baseline/share")
>> 
>> What has the test baseline to do with make doc ?
>
> 

Character code properties: customize what to show
  name: ELECTRIC LIGHT BULB
  general-category: So (Symbol, Other)
  decomposition: (128161) ('')

> Try cherry-picking this:
>
> commit dbe42f99406cd415c4b7d1f7349cd3e17ed73e69
> Author: Dan Eble 
> Date:   Wed Oct 23 09:26:24 2019 -0400
>
> Issue 5583: www_post.py: don't mirror regtest baseline
>
> --
> Dan

make[1]: Entering directory '/usr/local/tmp/lilypond'
/usr/local/tmp/lilypond/scripts/build/out/www_post LilyPond 2.19.84 ./out-www 
offline
Mirroring...
Processing HTML pages for offline target...
find ./out-www/offline-root -type l | xargs rm -f
make[1]: Leaving directory '/usr/local/tmp/lilypond'
dak@lola:/usr/local/tmp/lilypond$ 

Bingo.  That was driving me crazy.  Thanks.  I'll put it in the earliest
place in whatever I have not yet pushed to the stable branch.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread Dan Eble 
On Jan 30, 2020, at 14:57, David Kastrup  wrote:
> 
> os.symlink ("../../../out-baseline/share", 
> "out-www/offline-root/input/regression/out-test-baseline/share")
> 
> What has the test baseline to do with make doc ?


Try cherry-picking this:

commit dbe42f99406cd415c4b7d1f7349cd3e17ed73e69
Author: Dan Eble 
Date:   Wed Oct 23 09:26:24 2019 -0400

Issue 5583: www_post.py: don't mirror regtest baseline

--
Dan

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Dan Eble  writes:

> On Jan 30, 2020, at 13:39, David Kastrup  wrote:
>> 
>>> I just had a successful mage -j5 doc as of commit
>>> 8a05312fd8d2a56ec724a793b313949db0cfe729
>> 
>> Current stable/2.20 which failed on my system.
>
> I also am not able to reproduce the problem in a clean workspace; but
> I have verified that I want more cores. :)
>
>> Traceback (most recent call last):
>>  File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 
>> 
>>os.symlink (p, dest)
>> OSError: [Errno 2] No such file or directory
>> # clear __builtin__._
>> # clear sys.path
>> # clear sys.argv
>> # clear sys.ps1
>> # clear sys.ps2
>> 
>> in the central portion.  Which does not appear to help at all.  Any idea
>> what I could/should do to figure out where stuff goes wrong with my
>> system?  One thing I could do is check with current master.
>
> How about editing www_post to print the values of p and dest so we can
> see for which file os.symlink is failing?

Oh wow.

os.symlink ("../../../out-baseline/share", 
"out-www/offline-root/input/regression/out-test-baseline/share")

What has the test baseline to do with make doc ?

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Dan Eble  writes:

> On Jan 30, 2020, at 13:39, David Kastrup  wrote:
>> 
>>> I just had a successful mage -j5 doc as of commit
>>> 8a05312fd8d2a56ec724a793b313949db0cfe729
>> 
>> Current stable/2.20 which failed on my system.
>
> I also am not able to reproduce the problem in a clean workspace; but
> I have verified that I want more cores. :)

Well, LilyPond builds were the reasons I invested in a 4-core processor
my T420 laptop is not designed for thermally.  And the doc build is the
main contributor to LilyPond build time.

>> Traceback (most recent call last):
>>  File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 
>> 
>>os.symlink (p, dest)
>> OSError: [Errno 2] No such file or directory
>> # clear __builtin__._
>> # clear sys.path
>> # clear sys.argv
>> # clear sys.ps1
>> # clear sys.ps2
>> 
>> in the central portion.  Which does not appear to help at all.  Any idea
>> what I could/should do to figure out where stuff goes wrong with my
>> system?  One thing I could do is check with current master.
>
> How about editing www_post to print the values of p and dest so we can
> see for which file os.symlink is failing?

I'll take a look.  Right now I am in another doc build run, but I'll get
back.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread Dan Eble 
On Jan 30, 2020, at 13:39, David Kastrup  wrote:
> 
>> I just had a successful mage -j5 doc as of commit
>> 8a05312fd8d2a56ec724a793b313949db0cfe729
> 
> Current stable/2.20 which failed on my system.

I also am not able to reproduce the problem in a clean workspace; but I have 
verified that I want more cores. :)

> Traceback (most recent call last):
>  File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 
> 
>os.symlink (p, dest)
> OSError: [Errno 2] No such file or directory
> # clear __builtin__._
> # clear sys.path
> # clear sys.argv
> # clear sys.ps1
> # clear sys.ps2
> 
> in the central portion.  Which does not appear to help at all.  Any idea
> what I could/should do to figure out where stuff goes wrong with my
> system?  One thing I could do is check with current master.

How about editing www_post to print the values of p and dest so we can see for 
which file os.symlink is failing?
— 
Dan

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Jean-Charles Malahieude  writes:

> Le 30/01/2020 à 19:46, David Kastrup a écrit :
>> Oops.
>> dak@lola:/usr/local/tmp/lilypond$ git checkout origin
>> error: The following untracked working tree files would be overwritten by 
>> checkout:
>>  Documentation/snippets/non-traditional.snippet-list
>> Please move or remove them before you switch branches.
>> Aborting
>> That sounds suspicious.  Probably created by a makelsr call of mine
>> but
>> did not make it into version control.  I'll check what is up with that:
>> it may be that this is what caused the problem.
>> 
>
> Strange!
> I've a "non-traditional-key-signatures.ly" but not your snippet-list

Sure.  You did not run scripts/auxiliar/makelsr.py did you?  You just
checked out the branch, and this file never made it into version
control.  The stable branch does not have a non-traditional category it
would seem: when getting the snippet I should have removed it from its
list of categories.  But it's well possible that isn't the problem on my
system.  I am checking this as well thoroughly, and then I'll check in
comparison with master.

And if everything else fails, I need to check in a fresh tree.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread Jean-Charles Malahieude 

Le 30/01/2020 à 19:46, David Kastrup a écrit :

Oops.

dak@lola:/usr/local/tmp/lilypond$ git checkout origin
error: The following untracked working tree files would be overwritten by 
checkout:
Documentation/snippets/non-traditional.snippet-list
Please move or remove them before you switch branches.
Aborting

That sounds suspicious.  Probably created by a makelsr call of mine but
did not make it into version control.  I'll check what is up with that:
it may be that this is what caused the problem.



Strange!
I've a "non-traditional-key-signatures.ly" but not your snippet-list

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
David Kastrup  writes:

> David Kastrup  writes:
>
>> Jean-Charles Malahieude  writes:
>>
>>> Le 30/01/2020 à 19:08, David Kastrup a écrit :
 Dan Eble  writes:
 
> On Jan 30, 2020, at 11:39, David Kastrup  wrote:
>> That's not a new development, so there is no point in me to refrain from
>> cherry-picking further material: the last version of the branch I
>> checked was from early December and it failed in the same manner.
>> Compilation of CPU_COUNT=9 make -j9 doc ends with
>
> […]
 Thanks.  It is sort of annoying that it occurs right at the end of
 the
 (comparatively expensive) doc build, but at least then it is reproduced
 with another "make doc" within several seconds.  So there may be a way
 of getting an extensive log just for that final phase of mass-linking
 the offsite-root .
 
>>>
>>> I just had a successful mage -j5 doc as of commit
>>> 8a05312fd8d2a56ec724a793b313949db0cfe729
>>
>> Current stable/2.20 which failed on my system.
>>
>>> It took about half an hour, and I don't know if it matters, but I
>>> build in the root directory.
>>
>> As do I.  I just tried
>>
>> PYTHONVERBOSE=2 
>> PYTHONPATH=/usr/local/tmp/lilypond/python/out:`pwd`/python/auxiliar 
>> /usr/local/tmp/lilypond/scripts/build/out/www_post LilyPond 2.19.84 
>> ./out-www offline 2>&1 | tee /tmp/dump
>>
>> and that leaves me with
>>
>> # trying /usr/local/tmp/lilypond/python/out/postprocess_htmlmodule.so
>> # trying /usr/local/tmp/lilypond/python/out/postprocess_html.py
>> # trying /usr/local/tmp/lilypond/python/out/postprocess_html.pyc
>> # trying 
>> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.x86_64-linux-gnu.so
>> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.so
>> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_htmlmodule.so
>> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
>> # /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc matches 
>> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
>> import postprocess_html # precompiled from 
>> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc
>> import time # builtin
>> Mirroring...
>> Traceback (most recent call last):
>>   File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 
>> 
>> os.symlink (p, dest)
>> OSError: [Errno 2] No such file or directory
>> # clear __builtin__._
>> # clear sys.path
>> # clear sys.argv
>> # clear sys.ps1
>> # clear sys.ps2
>>
>> in the central portion.  Which does not appear to help at all.  Any idea
>> what I could/should do to figure out where stuff goes wrong with my
>> system?  One thing I could do is check with current master.
>
> Oops.
>
> dak@lola:/usr/local/tmp/lilypond$ git checkout origin
> error: The following untracked working tree files would be overwritten by 
> checkout:
>   Documentation/snippets/non-traditional.snippet-list
> Please move or remove them before you switch branches.
> Aborting
>
> That sounds suspicious.  Probably created by a makelsr call of mine but
> did not make it into version control.  I'll check what is up with that:
> it may be that this is what caused the problem.

Nope.  Removing that category from the original snippet and removing the
file still leaves me with a failing doc build at the same stage.  I'll
reconfigure to make sure, but I doubt this is it.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
David Kastrup  writes:

> Jean-Charles Malahieude  writes:
>
>> Le 30/01/2020 à 19:08, David Kastrup a écrit :
>>> Dan Eble  writes:
>>> 
 On Jan 30, 2020, at 11:39, David Kastrup  wrote:
> That's not a new development, so there is no point in me to refrain from
> cherry-picking further material: the last version of the branch I
> checked was from early December and it failed in the same manner.
> Compilation of CPU_COUNT=9 make -j9 doc ends with

 […]
>>> Thanks.  It is sort of annoying that it occurs right at the end of
>>> the
>>> (comparatively expensive) doc build, but at least then it is reproduced
>>> with another "make doc" within several seconds.  So there may be a way
>>> of getting an extensive log just for that final phase of mass-linking
>>> the offsite-root .
>>> 
>>
>> I just had a successful mage -j5 doc as of commit
>> 8a05312fd8d2a56ec724a793b313949db0cfe729
>
> Current stable/2.20 which failed on my system.
>
>> It took about half an hour, and I don't know if it matters, but I
>> build in the root directory.
>
> As do I.  I just tried
>
> PYTHONVERBOSE=2 
> PYTHONPATH=/usr/local/tmp/lilypond/python/out:`pwd`/python/auxiliar 
> /usr/local/tmp/lilypond/scripts/build/out/www_post LilyPond 2.19.84 ./out-www 
> offline 2>&1 | tee /tmp/dump
>
> and that leaves me with
>
> # trying /usr/local/tmp/lilypond/python/out/postprocess_htmlmodule.so
> # trying /usr/local/tmp/lilypond/python/out/postprocess_html.py
> # trying /usr/local/tmp/lilypond/python/out/postprocess_html.pyc
> # trying 
> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.x86_64-linux-gnu.so
> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.so
> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_htmlmodule.so
> # trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
> # /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc matches 
> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
> import postprocess_html # precompiled from 
> /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc
> import time # builtin
> Mirroring...
> Traceback (most recent call last):
>   File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 
> 
> os.symlink (p, dest)
> OSError: [Errno 2] No such file or directory
> # clear __builtin__._
> # clear sys.path
> # clear sys.argv
> # clear sys.ps1
> # clear sys.ps2
>
> in the central portion.  Which does not appear to help at all.  Any idea
> what I could/should do to figure out where stuff goes wrong with my
> system?  One thing I could do is check with current master.

Oops.

dak@lola:/usr/local/tmp/lilypond$ git checkout origin
error: The following untracked working tree files would be overwritten by 
checkout:
Documentation/snippets/non-traditional.snippet-list
Please move or remove them before you switch branches.
Aborting

That sounds suspicious.  Probably created by a makelsr call of mine but
did not make it into version control.  I'll check what is up with that:
it may be that this is what caused the problem.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Jean-Charles Malahieude  writes:

> Le 30/01/2020 à 19:08, David Kastrup a écrit :
>> Dan Eble  writes:
>> 
>>> On Jan 30, 2020, at 11:39, David Kastrup  wrote:
 That's not a new development, so there is no point in me to refrain from
 cherry-picking further material: the last version of the branch I
 checked was from early December and it failed in the same manner.
 Compilation of CPU_COUNT=9 make -j9 doc ends with
>>>
>>> […]
>> Thanks.  It is sort of annoying that it occurs right at the end of
>> the
>> (comparatively expensive) doc build, but at least then it is reproduced
>> with another "make doc" within several seconds.  So there may be a way
>> of getting an extensive log just for that final phase of mass-linking
>> the offsite-root .
>> 
>
> I just had a successful mage -j5 doc as of commit
> 8a05312fd8d2a56ec724a793b313949db0cfe729

Current stable/2.20 which failed on my system.

> It took about half an hour, and I don't know if it matters, but I
> build in the root directory.

As do I.  I just tried

PYTHONVERBOSE=2 
PYTHONPATH=/usr/local/tmp/lilypond/python/out:`pwd`/python/auxiliar 
/usr/local/tmp/lilypond/scripts/build/out/www_post LilyPond 2.19.84 ./out-www 
offline 2>&1 | tee /tmp/dump

and that leaves me with

# trying /usr/local/tmp/lilypond/python/out/postprocess_htmlmodule.so
# trying /usr/local/tmp/lilypond/python/out/postprocess_html.py
# trying /usr/local/tmp/lilypond/python/out/postprocess_html.pyc
# trying 
/usr/local/tmp/lilypond/python/auxiliar/postprocess_html.x86_64-linux-gnu.so
# trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.so
# trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_htmlmodule.so
# trying /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
# /usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc matches 
/usr/local/tmp/lilypond/python/auxiliar/postprocess_html.py
import postprocess_html # precompiled from 
/usr/local/tmp/lilypond/python/auxiliar/postprocess_html.pyc
import time # builtin
Mirroring...
Traceback (most recent call last):
  File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 

os.symlink (p, dest)
OSError: [Errno 2] No such file or directory
# clear __builtin__._
# clear sys.path
# clear sys.argv
# clear sys.ps1
# clear sys.ps2

in the central portion.  Which does not appear to help at all.  Any idea
what I could/should do to figure out where stuff goes wrong with my
system?  One thing I could do is check with current master.

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 
Dan Eble  writes:

> On Jan 30, 2020, at 11:39, David Kastrup  wrote:
>> That's not a new development, so there is no point in me to refrain from
>> cherry-picking further material: the last version of the branch I
>> checked was from early December and it failed in the same manner.
>> Compilation of CPU_COUNT=9 make -j9 doc ends with
>
> I did a bunch of build-related stuff in Oct-Nov, but this doesn't
> trigger any specific memories.  After I eat lunch, I'll try to
> reproduce the problem and see if I can spot anything helpful.

Thanks.  It is sort of annoying that it occurs right at the end of the
(comparatively expensive) doc build, but at least then it is reproduced
with another "make doc" within several seconds.  So there may be a way
of getting an extensive log just for that final phase of mass-linking
the offsite-root .

-- 
David Kastrup

Re: stable/2.20 branch does not complete "make doc"

2020-01-30 Thread Dan Eble 
On Jan 30, 2020, at 11:39, David Kastrup  wrote:
> That's not a new development, so there is no point in me to refrain from
> cherry-picking further material: the last version of the branch I
> checked was from early December and it failed in the same manner.
> Compilation of CPU_COUNT=9 make -j9 doc ends with

I did a bunch of build-related stuff in Oct-Nov, but this doesn't trigger any 
specific memories.  After I eat lunch, I'll try to reproduce the problem and 
see if I can spot anything helpful.
— 
Dan

stable/2.20 branch does not complete "make doc"

2020-01-30 Thread David Kastrup 


That's not a new development, so there is no point in me to refrain from
cherry-picking further material: the last version of the branch I
checked was from early December and it failed in the same manner.
Compilation of CPU_COUNT=9 make -j9 doc ends with

find ./out-www/web -name '*.html' | xargs grep -L 'UNTRANSLATED NODE: IGNORE 
ME' | sed 's!./out-www/!!g' | xargs 
/usr/local/tmp/lilypond/scripts/build/out/mass-link --prepend-suffix .zh hard 
./out-www /usr/local/tmp/lilypond/Documentation/./out-www
(echo | grep -L 'UNTRANSLATED NODE: IGNORE ME' ) | xargs 
/usr/local/tmp/lilypond/scripts/build/out/mass-link --prepend-suffix .zh hard 
./out-www /usr/local/tmp/lilypond/Documentation/./out-www
true
make[3]: Leaving directory '/usr/local/tmp/lilypond/Documentation/zh'
make[2]: Leaving directory '/usr/local/tmp/lilypond/Documentation'
make[1]: Leaving directory '/usr/local/tmp/lilypond'
make --no-builtin-rules out=www WWW-post
make[1]: Entering directory '/usr/local/tmp/lilypond'
/usr/local/tmp/lilypond/scripts/build/out/www_post LilyPond 2.19.84 ./out-www 
offline
Mirroring...
Traceback (most recent call last):
  File "/usr/local/tmp/lilypond/scripts/build/out/www_post", line 101, in 

os.symlink (p, dest)
OSError: [Errno 2] No such file or directory
make[1]: *** [GNUmakefile:162: out-www/offline-root/index.html] Error 1
make[1]: Leaving directory '/usr/local/tmp/lilypond'
make: *** [/usr/local/tmp/lilypond/stepmake/stepmake/generic-targets.make:174: 
doc] Error 2

It's not clear to me what is the problem here and where I would find
logs to figure out more.  The error occurs at a point of time where I
think the main work should already be done and just the offline-root is
getting set up.

I cannot rule out either that VERSION needs to have a particular form
(with regard to VERSION_STABLE and VERSION_DEVEL in relation to the
current version getting compiled) in order to set up things for the web
page, or that a fix already exists in a commit in the master branch that
I just have failed to cherry-pick yet.

If this triggers a bell with anybody, I'd be glad to hear about possible
causes.

Thanks!

-- 
David Kastrup

Re: Introduce ly_scm2utf8_string, and use it for printing text (issue 577440043 by hanw...@gmail.com)

2020-01-30 Thread nine . fierce . ballads 


https://codereview.appspot.com/577440043/diff/553450043/lily/include/lily-guile.hh
File lily/include/lily-guile.hh (right):

https://codereview.appspot.com/577440043/diff/553450043/lily/include/lily-guile.hh#newcode60
lily/include/lily-guile.hh:60: std::string ly_scm2utf8_string (SCM);
The second underscore makes this stand out from the other functions in
this group.

https://codereview.appspot.com/577440043/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread Dan Eble 
On Jan 30, 2020, at 04:54, hanw...@gmail.com wrote:
> 
> This may predate you, but the decision to not store references was
> intentional,
> exactly because storing NULL in them is very useful.  If you have a
> reference,
> it has to be initialized in the constructor, and this introduces a lot
> of boilerplate
> because you have to pass the non-null reference across constructors in
> the class
> hierarchy. 

The discussion has turned from (a) passing a required parameter as a reference, 
to (b) using a reference as a member variable.  (a) does not imply (b).  You 
can pass in a T& and store it in a T* to avoid the constraints that (b) would 
place on the use of your class (though they apparently were not previously a 
problem in this case).

> The current code overwhelmingly disavows references. The 2 remaining
> uses (this being one) stand out like a sore thumb.

We must be miscommunicating, because I see a lot more than 2.  For some 
examples,

git grep 'vector<[^>]\+> &'

Please clarify.

Thanks,
— 
Dan

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread dak 
On 2020/01/30 09:54:48, hanwenn wrote:
> On 2020/01/29 11:44:57, dak wrote:
> > > BTW - I don't want to tell a C++ expert how to use the language
> > > in general.  If
> > > we were in an alternate reality  where we could start from scratch
we could
> > > reconsider the decision to not use non-const references in
> > > structs and function
> > > arguments. As it is however, any that we have are most likely
errors that we
> > > should correct. Check
> > 
> > Errors mean non-intentional things. 
 
> This may predate you, but the decision to not store references was
> intentional, exactly because storing NULL in them is very useful.

That assumes that every variable is created equal, and every purpose
is the same.

> If you have a reference, it has to be initialized in the
> constructor, and this introduces a lot of boilerplate because you
> have to pass the non-null reference across constructors in the class
> hierarchy.

That assumes that references are being used within a class hierarchy.


> Imagine adding something to the Prob class.

We are not talking about the Prob class.  We are not looking for
one-size-fits-all solutions, but for a solution matching the problem.

> If it is a reference, you'll now have to update a dozen classes just
> so it compiles again.

There is no reason whatsoever that the existing constructors in the
Prob class should not cater for initializing the reference.

I agree that references in data structures can be a problem, and very
much so if the data structure is supposed to have a persistent life
time, like basically all Smobs do, because references have a life time
tied to the referenced objects.

But that's not what we are talking about here: here we have a
parameter packet with a life-time exactly bounded by the call.

Basically you state that we should not be using tools appropriate to
the job we need to solve, but restrict ourselves to a common
denominator.  With a language as large as C++, which has grown to its
size exactly because there are a lot of things not solved well by
existing tools, that is not going to be a good fit.

We recently decided to bump our C++ standard up to C++11 in order to
enable a more idiomatic programming style for newcomers, and yet you
argue for restricting use of the most basic C++ programming techniques
regardless of the context they are used in.

Prescribing a rigid mixture of archaic and modern is not likely to
increase the attractiveness for LilyPond contributors in general.

> Maybe LilyPond has passed the phase that we need to refactorings,
> but I remember refactoring constructors being a major PITA. Hence,
> no references.

This sounds like "my way or the highway".  That is a valid and in some
manners efficient way of structuring a project top-down, but at the
current point of time LilyPond's leadership model is more based on
consensus-finding.  If changing that is desired in order to get rid of
cumbersome discussions, I think it should at least be put up to
debate.

> > My own take here is that we would not want
> > to use references on things that may be used as SCM values, so
things like
> > 
> > Grob  = *unsmob (sbla);
> > 
> > would be quite undesirable.  However, for code that has no Schemish
> > implications, I find it perfectly fine to use C++ references in
> > the manner they
> > are intended to be used.  There has been a recent push to settle on
C++11 as a
> > programming standard to adhere to in order to be more friendly to
> > newcomers, and
> > it seems sort of pointless to promote C89 standards to go
> > alongside.  That makes
> > people less, not more confident in what they may rely on using.
> 
> People usually make changes by copy & pasting existing code, and then
adapting
> it to their needs. If there are two ways of structuring the code, this
makes
> things more confusing.

C++ is not Lua.  It just doesn't have one-size-fits-all constructs, so
there will always either be a considerable number of tools in play, or
the code becomes non-idiomatic and cumbersome to work with.

> what I am trying to say: 
> 
> For reading the code, it is important for the source code to be
consistent with
> itself.

C++ is what it is, and it is what we have to work with.  Consistency
for me does not mean that I refrain from using references where
appropriate but that I refrain from littering code all over the place,
like with copy  For example, consistency for me means that all
accesses to Scheme data structures now without exception run through
smobs.hh and associated header and C files.  That is why disabling
Scheme marking passes in all of LilyPond can be done by outcommenting
a single line in all of the source code.  It is also why changing the
overall GC schemes is feasible.

> This is also why we have automated formatting.

Nope.  We have automated formatting so that people reading the code do
not get irritated by _gratuitous_ differences, asking themselves "why
is this different?".  That is different from having differences with

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread fedelogy 


https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (right):

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi#newcode208
Documentation/contributor/quick-start.itexi:208: sufficient to choose
number@tie{}71 (Generic, 105 keys).  After that,
It seems it's not necessarily number 71. I would remove the number.
keyboards are sorted alphabetically so "Generic, 105 keys" is enough.

Here's what I see:

65. Generic 101-key PC
66. Generic 102-key (Intl) PC
67. Generic 104-key PC
68. Generic 105-key (Intl) PC

I've tried 68 but didn't seem to work. I had to go to the GUI settings
for the layout, disable system setting and remove the english from the
list, then adding new layouts worked fine. (Probably a bug in Debian or
XFCE? Honestly, I don't have time nor will to investigate.)

Anyway, to me the point is: doing trial and error in the GUI is way
better than having to follow the same procedure with dpkg-reconfigure.
So I'm not sure adding keyboard-configuration to LilyDev was a good
idea. I thought that it would have saved the user from guessing the
right layout but that's not the case, so there's no improvement over
previous situation AFAICS.

https://codereview.appspot.com/561360043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread lemzwerg--- via Discussions on LilyPond development 
And hopefully some final nits.  Thanks for your patience!


https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (right):

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi#newcode147
Documentation/contributor/quick-start.itexi:147: Verify the summary
details and click @q{Create}, when you are
I think it's better to avoid the comma before 'when'.  Maybe it is even
better to replace 'when' with 'as soon as' (and still without comma).

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi#newcode175
Documentation/contributor/quick-start.itexi:175: @warning{Since the
default keyboard layout is US (american).  You
s/american/American/
s/.  You/, you/

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi#newcode194
Documentation/contributor/quick-start.itexi:194: You might need to
change the keyboard layout from default US (american)
s/american/American/

https://codereview.appspot.com/561360043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread lemzwerg--- via Discussions on LilyPond development 
> > Documentation/contributor/quick-start.itexi:46:
> > create it from the sources located in the
> > @code{/mkosi} subdirectory
> > s/@code/@file/.
> 
> Slightly puzzled about this one. You suggested
> '@code' in your first review, didn't you?

Yep.  A mistake, sorry.  '@file' is the right one (or rather, the better
one).

> > 30@tie{}GB
> 
> Is there a particular reason why you suggest to use
> '@tie{}' here and @dmn{GB} for another occurrence of
> this dimension?
> Just curious...

You are completely right.  It should be @dmn{GB} here, too.

https://codereview.appspot.com/561360043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread michael . kaeppler--- via Discussions on LilyPond development 
On 2020/01/29 15:36:59, fedelogy wrote:
> LGTM
> 
> Just two thoughts:
> 
> 1. I'm not sure why you suggest how to set the root password, as the
dev user
> can get admin privileges via sudo.

Well, I just thought that it would be bad idea for >any< Linux system 
to leave the root password unsetted. Maybe the amount of damage that
can occur to the host system through the virtualization barrier is
small,
but at least it minimizes the risk of scrambling your guest system by
accident. If you use 'sudo', you have to think about what you type, at
least. 
AFAIK, Ubuntu disables the root account by default, for such reasons.
But you could argue that the docs suggest to log in as 'dev', anyway,
and if I do want to log in as root, it does not matter if I have to use
a password or none. 
What do you think about disabling the root account?

> 
> 2. While keyboard configuration is a step needed for any non-US
contributor,
> reconfiguring the locales should be useful only for translators,
unless I miss
> something.
> 
> Perhaps you could say that these two configurations are optional.

One benefit is to have the window manager and the output of localized 
tools in your own language. But that's not essential, I admit.
But I noticed that umlauts are not displayed correctly without
configuring the locales. Or am I missing some simpler solution here?





https://codereview.appspot.com/561360043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread michael . kaeppler--- via Discussions on LilyPond development 
On 2020/01/30 10:12:14, lemzwerg wrote:
> Next round of nits :-)
> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi
> File Documentation/contributor/quick-start.itexi (left):
> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#oldcode103
> Documentation/contributor/quick-start.itexi:103: specific option
choose @q{Linux
> 2.6/3.x/4.x (64-bit)}.
> Is it really '64 bit' in one case and '64-bit' in the other case?

No, you're right. It's always '64-bit'. Fixed.

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi
> File Documentation/contributor/quick-start.itexi (right):
> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode46
> Documentation/contributor/quick-start.itexi:46: create it from the
sources
> located in the @code{/mkosi} subdirectory
> s/@code/@file/.

Slightly puzzled about this one. You suggested '@code' in your first
review, didn't you?

>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode97
> Documentation/contributor/quick-start.itexi:97: On Linux, run the
following
> command in the directory where you've
> s/you've/you have/
> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode108
> Documentation/contributor/quick-start.itexi:108: As VirtualBox does
not support
> the raw format, you'll have to
> The GNU documentation guidelines recommend to not use the future tense
if
> possible.
> 
>   s/you'll have/you have/
> 
> Please change similar occurrences of future tense, too.

Some future constructs were already there. 
Done within the section I rewrote. (Hopefully I did not miss any) 
Adjusting this for the whole chapter is another thing...

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode118
> Documentation/contributor/quick-start.itexi:118: @warning{You will
need a fair
> amount of disk space (around 30 GB) to
> 30@tie{}GB

Is there a particular reason why you suggest to use '@tie{}' here
and @dmn{GB} for another occurrence of this dimension?
Just curious...

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode176
> Documentation/contributor/quick-start.itexi:176: layout, like e.g.
@q{lilzpond}
> on a German keyboard.}
> There is an eternal battle whether there is a comma or not after
'e.g.' (in the
> LilyPond documentation, we *do* use a comma).  However, I would
reformulate this
> to
> 
>   ... like @q{lilzpond} on a German keyboard, for example.

Done.

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode202
> Documentation/contributor/quick-start.itexi:202: rights temporarily. 
It will
> show you a warning message on it's first
> s/it's/its/

Urgh. Done. 

>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode217
> Documentation/contributor/quick-start.itexi:217: To set up your system
> language(charset, localized messages etc.),
> s/To set up your system language(charset, localized messages etc.),/
>   To set up your system language (charset, localized messages, etc.),/

Done.

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode229
> Documentation/contributor/quick-start.itexi:229: Finally, you should
run a setup
> script.  If you're on the command line
> The GNU documentation guidelines recommend to not use contractions, so
> 
>   s/you're/you are/

Done.

> 
>
https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode231
> Documentation/contributor/quick-start.itexi:231: which will set up git
and
> download all the repositories needed to build
> s/which/that/

Done.

https://codereview.appspot.com/561360043/

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

2020-01-30 Thread trueroad 
How about this patch?
Sorry, not tested.

If I understand correctly,
this patch solves not only Issue 4943 but also Issue 4975.
These issues do not only occur on Windows, but on all x86 platforms
except Linux.
`defined (__MINGW32__)` is not necessary.

For Linux-x86, it uses the original precision setting. i.e. _FPU_SETCW
().
For other x86 platforms, it uses Arnold's inline asm for setting
precision.

```
diff --git a/lily/main.cc b/lily/main.cc
index 9145345fff..cba6856159 100644
--- a/lily/main.cc
+++ b/lily/main.cc
@@ -209,19 +209,40 @@ char const *LILYPOND_DATADIR = PACKAGE_DATADIR "/"
TOPLEVEL_VERSION;
unpredictable places. To get around this, we tell the x87 FPU to use
only
double precision. Note that this is not needed for x86_64 because
that uses
the SSE unit by default instead of the x87 FPU. */
-#if ((defined (__x86__) || defined (__i386__)) \
-  && defined (HAVE_FPU_CONTROL_H) && (HAVE_FPU_CONTROL_H == 1))
+#if defined (__x86__) || defined (__i386__)
+// This environment is x86.
+// It is necessary that setting precision by setting x87 FPU control
word.
 
+#if defined (HAVE_FPU_CONTROL_H) && (HAVE_FPU_CONTROL_H == 1)
 #include 
+#endif
+
 static void
 configure_fpu ()
 {
+#if defined (HAVE_FPU_CONTROL_H) && (HAVE_FPU_CONTROL_H == 1)
+  // This environment has fpu_control.h. (e.g. Linux glibc)
+  // We use _FPU_SETCW () to set x87 FPU control word.
   fpu_control_t fpu_control = 0x027f;
   _FPU_SETCW (fpu_control);
+#else
+  // This environment doesn't have fpu_control.h. (e.g. MinGW)
+  // We use inline asm to set x87 FPU control word.
+  asm(
+  " push %ebp"
+"\n mov  $0x027f, %eax"
+"\n push %eax"
+"\n mov  %esp, %ebp"
+"\n fldcw (%ebp)"
+"\n pop %eax"
+"\n pop %ebp"
+  );
+#endif
 }
 
 #else
-
+// This environment isn't x86. (e.g. x86_64)
+// It is unnecessary that setting precision.
 static void
 configure_fpu ()
 {
-- 
```



https://codereview.appspot.com/577450043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread lemzwerg--- via Discussions on LilyPond development 
Next round of nits :-)


https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (left):

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#oldcode103
Documentation/contributor/quick-start.itexi:103: specific option choose
@q{Linux 2.6/3.x/4.x (64-bit)}.
Is it really '64 bit' in one case and '64-bit' in the other case?

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (right):

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode46
Documentation/contributor/quick-start.itexi:46: create it from the
sources located in the @code{/mkosi} subdirectory
s/@code/@file/.

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode97
Documentation/contributor/quick-start.itexi:97: On Linux, run the
following command in the directory where you've
s/you've/you have/

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode108
Documentation/contributor/quick-start.itexi:108: As VirtualBox does not
support the raw format, you'll have to
The GNU documentation guidelines recommend to not use the future tense
if possible.

  s/you'll have/you have/

Please change similar occurrences of future tense, too.

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode118
Documentation/contributor/quick-start.itexi:118: @warning{You will need
a fair amount of disk space (around 30 GB) to
30@tie{}GB

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode176
Documentation/contributor/quick-start.itexi:176: layout, like e.g.
@q{lilzpond} on a German keyboard.}
There is an eternal battle whether there is a comma or not after 'e.g.'
(in the LilyPond documentation, we *do* use a comma).  However, I would
reformulate this to

  ... like @q{lilzpond} on a German keyboard, for example.

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode202
Documentation/contributor/quick-start.itexi:202: rights temporarily.  It
will show you a warning message on it's first
s/it's/its/

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode217
Documentation/contributor/quick-start.itexi:217: To set up your system
language(charset, localized messages etc.),
s/To set up your system language(charset, localized messages etc.),/
  To set up your system language (charset, localized messages, etc.),/

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode229
Documentation/contributor/quick-start.itexi:229: Finally, you should run
a setup script.  If you're on the command line
The GNU documentation guidelines recommend to not use contractions, so

  s/you're/you are/

https://codereview.appspot.com/561360043/diff/575590043/Documentation/contributor/quick-start.itexi#newcode231
Documentation/contributor/quick-start.itexi:231: which will set up git
and download all the repositories needed to build
s/which/that/

https://codereview.appspot.com/561360043/

Re: GUILE 2: Increase initial heap (issue 547540043 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
For deployments (say, our builds that always run on the same dedicated
machine), it makes sense to set this to a max size.

Servers at Google usually set the equivalent parameter for Java upfront
as well, because they get a fixed allocation of RAM.  

https://codereview.appspot.com/547540043/

Re: GUILE 2: Increase initial heap (issue 547540043 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
On 2020/01/29 10:24:12, lemzwerg wrote:
> LGTM
> 
> https://codereview.appspot.com/547540043/diff/561380043/lily/main.cc
> File lily/main.cc (right):
> 
>
https://codereview.appspot.com/547540043/diff/561380043/lily/main.cc#newcode772
> lily/main.cc:772: sane_putenv("GC_INITIAL_HEAP_SIZE", "40M", false);
> Do you think it makes sense to mention this environment variable in
the lilypond
> documentation?  Is there ever a reason for the user to change the
value?

It is too early to tell. Ideally, we would tune GC so it does the right
thing. 

https://codereview.appspot.com/547540043/

Introduce ly_scm2utf8_string, and use it for printing text (issue 577440043 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
Reviewers: dak,

Message:
I think this may have lacked sf.net issue. Please review.

Description:
Introduce ly_scm2utf8_string, and use it for printing text

Before, Text_interface::interpret_string used ly_scm2string, which
runs scm_to_locale_stringn to get at the string. The default locale is
"C", so all Unicode chars are then converted to "?".

Please review this at https://codereview.appspot.com/577440043/

Affected files (+20, -1 lines):
  M lily/include/lily-guile.hh
  M lily/lily-guile.cc
  M lily/text-interface.cc


Index: lily/include/lily-guile.hh
diff --git a/lily/include/lily-guile.hh b/lily/include/lily-guile.hh
index 
ab14e8541e4f6a39bd9d87a0bc262c0b86805af1..ccbebcadf976bd2da40a14e2e0d88edc715ac0a4
 100644
--- a/lily/include/lily-guile.hh
+++ b/lily/include/lily-guile.hh
@@ -57,6 +57,7 @@ std::string gulp_file_to_string (const std::string , bool 
must_exist, int siz
 
 SCM ly_string2scm (std::string const );
 std::string ly_scm2string (SCM s);
+std::string ly_scm2utf8_string (SCM);
 std::string ly_symbol2string (SCM);
 std::string robust_symbol2string (SCM, const std::string&);
 Rational ly_scm2rational (SCM);
Index: lily/lily-guile.cc
diff --git a/lily/lily-guile.cc b/lily/lily-guile.cc
index 
7f084806e928aebf1a1add8e0dbe9f41a95fcf82..84e2e69672f2590acdbf3938fa739edcacecda7b
 100644
--- a/lily/lily-guile.cc
+++ b/lily/lily-guile.cc
@@ -119,6 +119,24 @@ extern "C" {
 /*
   STRINGS
  */
+string
+ly_scm2utf8_string (SCM str)
+{
+#if GUILEV2
+  assert (scm_is_string (str));
+  size_t len = 0;
+  char *bytes = scm_to_utf8_stringn (str, );
+
+  // It would be nice to stick 'bytes' directly into string.
+  string result(bytes, len);
+
+  free(bytes);
+  return result;
+#else
+  return ly_scm2string(str);
+#endif
+}
+
 string
 ly_scm2string (SCM str)
 {
Index: lily/text-interface.cc
diff --git a/lily/text-interface.cc b/lily/text-interface.cc
index 
1f62e9b287290439d45375e717b03196656ac763..9d8de13778966b169771c7762a186adc6cb61e00
 100644
--- a/lily/text-interface.cc
+++ b/lily/text-interface.cc
@@ -78,7 +78,7 @@ Text_interface::interpret_string (SCM layout_smob,
   LY_ASSERT_SMOB (Output_def, layout_smob, 1);
   LY_ASSERT_TYPE (scm_is_string, markup, 3);
 
-  string str = ly_scm2string (markup);
+  string str = ly_scm2utf8_string (markup);
   Output_def *layout = unsmob (layout_smob);
   Font_metric *fm = select_encoded_font (layout, props);

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 


https://codereview.appspot.com/577410045/diff/551410043/lily/parse-scm.cc
File lily/parse-scm.cc (right):

https://codereview.appspot.com/577410045/diff/551410043/lily/parse-scm.cc#newcode59
lily/parse-scm.cc:59: 
On 2020/01/29 14:42:22, dak wrote:
> Pouring oil on the fire...  Rietveld highlighting indicates non-empty
lines
> consisting only of spaces.  Incidentally, those would already get
highlighted
> with
> 
> git log --check

Done.

added a hook to my .emacs to delete them.

https://codereview.appspot.com/577410045/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
On 2020/01/29 11:44:57, dak wrote:
> > BTW - I don't want to tell a C++ expert how to use the language in
general. If
> > we were in an alternate reality  where we could start from scratch
we could
> > reconsider the decision to not use non-const references in structs
and
> function
> > arguments. As it is however, any that we have are most likely errors
that we
> > should correct. Check
> 
> Errors mean non-intentional things. 

This may predate you, but the decision to not store references was
intentional,
exactly because storing NULL in them is very useful.  If you have a
reference,
it has to be initialized in the constructor, and this introduces a lot
of boilerplate
because you have to pass the non-null reference across constructors in
the class
hierarchy. 

Imagine adding something to the Prob class. If it is a reference, you'll
now have 
to update a dozen classes just so it compiles again.

Maybe LilyPond has passed the phase that we need to refactorings, but I
remember 
refactoring constructors being a major PITA. Hence, no references. 

> My own take here is that we would not want
> to use references on things that may be used as SCM values, so things
like
> 
> Grob  = *unsmob (sbla);
> 
> would be quite undesirable.  However, for code that has no Schemish
> implications, I find it perfectly fine to use C++ references in the
manner they
> are intended to be used.  There has been a recent push to settle on
C++11 as a
> programming standard to adhere to in order to be more friendly to
newcomers, and
> it seems sort of pointless to promote C89 standards to go alongside. 
That makes
> people less, not more confident in what they may rely on using.

People usually make changes by copy & pasting existing code, and then
adapting it to their needs. If there are two ways of structuring the
code, this makes things more confusing.

> > grep --color -nH  -e '&' lily/include/*h|grep -v const
> > 
> > Also, it is rare to check incoming parameters for nullness in
implementation.
> 
> I am not exactly sure I'd call that a feature: we had crashes because
of it. 
> Some trampolining code now has the requisite assertions inside since
one cannot
> perform those checks too late: GCC optimises checks like "if (!this)"
away these
> days.

It is correct that using references makes it harder to do null-pointer
derefs, and 
I am not saying the lack of null checks are a desirable feature.

what I am trying to say: 

For reading the code, it is important for the source code to be
consistent with itself. This is also why we have automated formatting.  

The current code overwhelmingly disavows references. The 2 remaining
uses (this being one) stand out like a sore thumb.

If anyone wants to modernize the source code to introduce references, I
think this should be done with an overall plan of how this will become a
consistent idiom. I personally think doing so does not solve pressing
problems, but as I won't be doing the work that doesn't bother me so
much.

My proposal is to go ahead with this change, so I can go then go on with

https://github.com/hanwen/lilypond/commit/599878a08c18323810aaa1dee3bf4d9b208e223c

which is based on this one. 

If anyone has a plan for changing pointers to references globally, I am
happy to comment on it in a different review thread.

Meta question: if I would keep shut for 7 days, is this a change that
would go in based on "countdown" ?


https://codereview.appspot.com/577410045/

Re: automated formatting

2020-01-30 Thread Werner LEMBERG 


> * statically linked binaries of clang-format are available here:
> https://github.com/angular/clang-format/tree/master/bin ;

Aaah!  This is very nice, thanks.

> 1. Recognize clang-format as a "good enough", so I can use it
> locally on my diffs and not get reviews on style.

I did a quick view, and I think the output looks promising; I would
definitely call it 'good enough', and formatting only new code as you
suggested should be easily doable.

The only thing that I dislike is that a situation like

   //   . foo bar blabla blubb.
   //   Woodle woodle crunch ...

possibly gets reformatted as

   //   . foo bar blabla blubb. Woodle woodle crunch ...
  
this is, a full stop at a line end gets converted to a mid-line full
stop followed by a single space only.

Is there an 'emacs' option for clang-format to get two spaces for such
cases?


Werner

Re: Data structure for (package) options

2020-01-30 Thread Urs Liska 
Am Donnerstag, den 30.01.2020, 09:50 +0100 schrieb Han-Wen Nienhuys:
> On Tue, Jan 28, 2020 at 2:48 PM David Kastrup  wrote:
> 
> > > I think we should aim to avoid textual inclusion as a mechanism
> > > that
> > > powers packaging.
> > 
> > At the implementation side, "textual inclusion" will be what has to
> > power systems where one can "plug in" packages and have them work
> > by
> > being present.  Even Guile's use-modules mechanism works at that
> > level.
> > But of course that doesn't mean that we want \include as a user-
> > level
> > access method in the long haul.
> 
> By textual inclusion, I mean a mechanism that must insert all its
> data
> in the global namespace. Library systems usually must have means to
> be
> explicit about external APIs and internal implementation details. I
> am
> advocating that there is an easy to use mechanism such that OLL
> packages can shield some of the implementation from other packages.
> 

My idea for packages is:

 * packages *always* have .ly files as entry points (even if their
   "real" work is exclusively Scheme). That makes it easier to handle
   (no need to load packages in two different forms) and gives regular
   users a lower threshold of moving library code to packages.
 * they explicitly define their public interface. See below for my
   ideas about that.
 * A new command \usepackage will either load the whole package or only
   a subset of its interface (like the "from X load Y" syntax in
   Python).

Maybe a clean way of making an interface explicit would be not to do

  myFunction = #(define-music-function ...

but having a new (set of?) command(s) that allow

  \export myFunction #(define-music-function

When parsing the package (i.e. loading it for the first time
`myFunction` is stored as the package's interface (in an internal
structure storing the interfaces of all loaded packages, along with
their defined options). [This is something *I* can do with the current
Scheme possibilities]

When using a package that has already been loaded (\usepackage checks
that first) the stored Scheme values (which may equally be variables or
procedures) are made visible in the caller's scope.

The question I have is:
If \usepackage makes the names available to everything else later (like
a regular \include of a LilyPond file) I can do this with what we
already have in Scheme.
If \usepackage should make the names available only within the current
input file (like (use-modules) does) I think \usepackage would have to
be  implemented at the parser level.

Urs

Re: Is the CG out of date?

2020-01-30 Thread Peter Toye 
Monday, January 20, 2020, 10:17:42 AM, Michael Käppler wrote:

> Am 20.01.2020 um 11:04 schrieb Michael Käppler:
>>
>> Attached is an extract of the CG pages we're discussing, with my recent
>> (yet only local) changes.
>>
> Urgh, sorry. Really(tm) attached now.

Just noticed a silly typo here - in 'Installing LilyDev in VirtualBox" step 11 
/keybord/keyboard/

Hope it's not too late, but doesn't really matter.

I was hoping to get my amendments to NR and LM organised, but haven't been able 
to as I've been having problems with VirualBox and am very busy now.

Best regards,

Peter

Re: lily: fix some type conversion warnings (issue 557190043 by hanw...@gmail.com)

2020-01-30 Thread lemzwerg--- via Discussions on LilyPond development 
> I don't like this methodology, 

+1

> I prefer what Dan does right now (btw thanks for
> working on this!): Pick a warning, investigate
> the issue and fix it correctly.

I currently add `CXXFLAGS="-Wno-sign-conversion"` while calling make;
this reduces the amount of warnings with clang to a level which looks
acceptable to me.


https://codereview.appspot.com/557190043/

Re: automated formatting

2020-01-30 Thread Han-Wen Nienhuys 
On Tue, Jan 28, 2020 at 8:08 PM David Kastrup  wrote:
> >>Can you provide me with a presubmit hook that applies fixcc? Can you
> >>guarantee that, if fixcc has run on the code, there will be no further
> >>remarks on code formatting during review?
> >>
> >> I think that it's a really good idea to have presubmit hooks.  I
> >> believe, but can't guarantee, that if all code were automatically
> >> reformatted on submission (by either fixcc or clang-format) there
> >> would be virtually no comments on formatting.  And you could
> >> completely ignore any that were made.
> >
> > I'm all for making it possible for a contributor to set up something
> > like this if he pleases, but I think it's a bad idea to rely on this
> > level of automation and integration with a specific source-control
> > tool.

Sorry, I was unclear. I mean: a local precommit hook (which wouldn't
be mandatory to install)

> To clarify: before you joined the project, there were recurrent
> reformatting passes preferably at "quiescent" times to bring consistent
> style into the code base.  There were large discussions about it but it
> was sort-of agreed that having a uniform style presented to people
> reading the code was a reasonably good idea and that the tool chosen and
> maintained did not cause noticeable damage (formatting of scm was a more
> tricky proposition and the respective script available by now just
> defers to Emacs for doing it).


I ran some of the discussion points by the authors of clang-format
(they work in the Google Munich office.)

Their suggestions:

* formatting can and will change subtly across versions. Difference
will be relatively small, because large users (eg. google) have an
auto-formatted code base, and don't want needless churn on their code
base.

* If there are doubts, it's best to recommend the "diff" option
instead 
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting),
which reformats diffs, instead of whole files.

* we could introduce an automated check that verifies the diff against
formatting problems. Either as part of the build, in git-cl, or as
suggested pre-commit hook in the docs.

* statically linked binaries of clang-format are available here:
https://github.com/angular/clang-format/tree/master/bin ; we could
standardize on whatever the Angular project recommends (which is v11,
at the moment.)

> Basically, no real great solution offered itself then (and I don't see
> that we can do this significantly better now, even while the particular
> tool to use may be subject to debate) and the adapted solution was to
> encourage using the fixcc scripts on one's own contribution and
> regularly apply it to the repository at reasonable points of time.
>
> I am not sure how far this policy dates back, but while Graham was
> project leader, it was done regularly.  There was no intent by me not
> following this practice: I simply dropped the ball.
>
> I'd be willing to pick up on it since the adopted compromise was sort of
> agreed upon by the active developers.
>
> The set of active developers has changed by now, and of course we also
> try angling for the mystic developer just missing one particular
> tool/workflow in order to become productive or productive again.  There
> is little enough sense in turning this into a showdown rather than an
> attempt of finding a consensus everyone can find themselves able to live
> with.  I'll do (respectively already have done in a few issues) what I
> feel brings the comparatively simple fixcc tool back up to scratch
> (fittingly for my non-use of it, its --sloppy option was broken) and
> then hopefully we'll be in a better position of deciding how to
> continue.

So here is my list of desires, priority from most to least important.
I'd be happy with 1), but I think going to 2) or even 3) would be good
for the LilyPond project as a whole.

1. Recognize clang-format as a "good enough", so I can use it locally
on my diffs and not get reviews on style.

2. Recognize clang-format as recommended, and document how to set it
up for contributors

3. Recognize clang-format as standard, and setup a formatting test on
diffs in the Makefile.

4. Recognize a certain version of clang-format as standard, and setup
a formatting test on the source tree in the Makefile. Users can setup
integration with emacs, vim, CLion, Eclipse etc.

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: lily: fix some type conversion warnings (issue 557190043 by hanw...@gmail.com)

2020-01-30 Thread jonas . hahnfeld 
On 2020/01/30 08:37:44, hanwenn wrote:
> There are two problems that are tangentially related, and I think the
discussion
> comes from us wanting to solve different problems. 
> 
> My problem is that it is very distracting to change the C++ code base
if there
> are compiler warnings about existing code. So, I want a code base that
compiles
> with zero warnings as a starting point.
> 
> There is a different problem, which that there are some dubious casts
in our
> source code. I think that you, ie. Dan and David, want to address that
problem.
> 
> Here is a proposal that can make both of us happy:
> 
> 1) fix all warning provisorially, by adding the casts that the
compiler does
> today explicitly. We mark them with "TODO: investigate cast". Result:
the
> compile becomes warning-free
> 
> 2) go over all the "investigate cast" warnings, changing return
types/signatures
> where necessary. Result: our casts are now all kosher.
> 
> I'm happy to contribute the work for 1).
> 
> How does that sound?

I don't like this methodology, what's the difference over disabling
-Wconversion? (which I don't think would be a good idea)
I prefer what Dan does right now (btw thanks for working on this!): Pick
a warning, investigate the issue and fix it correctly. Otherwise I see
the risk that the TODOs will stick for longer than they should. So I
think it's a good idea to keep the "distracting" warnings in place until
they are correctly addressed.

https://codereview.appspot.com/557190043/

Re: Move Guile-style modules from scm to scm-modules (issue 567140045 by m...@ursliska.de)

2020-01-30 Thread Han-Wen Nienhuys 
On Wed, Jan 29, 2020 at 4:02 PM  wrote:
>
> On 2020/01/29 12:20:10, mail5 wrote:
> > Unfortunately I haven't set up a build system on my new computer yet,
> so this
> > patch is not tested locally at all, so I'm humbly waiting for the
> automated
> > tests to succeed or fail ...
>
> I don't like the "use-modules clauses adjusted accordingly".  I don't
> think it makes sense readjusting use-modules clauses all the time while
> we are deciding on the final module organisation, so I'd strongly
> suggest first biting the bullet and deciding on a syntax for a

I agree with David here. Let's solve the hard problem first before we
care about cosmetics.


-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Data structure for (package) options

2020-01-30 Thread Han-Wen Nienhuys 
On Tue, Jan 28, 2020 at 2:48 PM David Kastrup  wrote:

> > I think we should aim to avoid textual inclusion as a mechanism that
> > powers packaging.
>
> At the implementation side, "textual inclusion" will be what has to
> power systems where one can "plug in" packages and have them work by
> being present.  Even Guile's use-modules mechanism works at that level.
> But of course that doesn't mean that we want \include as a user-level
> access method in the long haul.

By textual inclusion, I mean a mechanism that must insert all its data
in the global namespace. Library systems usually must have means to be
explicit about external APIs and internal implementation details. I am
advocating that there is an easy to use mechanism such that OLL
packages can shield some of the implementation from other packages.

> It's likely that a typical more complex package may consist of both .ly
> and/or .scm components and that is an implementation detail that a
> package user should not need to bother about, and that hopefully should
> not significantly complicate things for people wanting to provide
> packages with either or both .ly and/or .scm components.
>
> --
> David Kastrup



-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: lily: fix some type conversion warnings (issue 557190043 by hanw...@gmail.com)

2020-01-30 Thread hanwenn 
There are two problems that are tangentially related, and I think the
discussion comes from us wanting to solve different problems. 

My problem is that it is very distracting to change the C++ code base if
there are compiler warnings about existing code. So, I want a code base
that compiles with zero warnings as a starting point.

There is a different problem, which that there are some dubious casts in
our source code. I think that you, ie. Dan and David, want to address
that problem.

Here is a proposal that can make both of us happy:

1) fix all warning provisorially, by adding the casts that the compiler
does today explicitly. We mark them with "TODO: investigate cast".
Result: the compile becomes warning-free

2) go over all the "investigate cast" warnings, changing return
types/signatures where necessary. Result: our casts are now all kosher.

I'm happy to contribute the work for 1).

How does that sound?

https://codereview.appspot.com/557190043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread michael . kaeppler--- via Discussions on LilyPond development 
On 2020/01/29 06:48:43, lemzwerg wrote:
> Some more nits.  Thanks for working on this!
> 
>
https://codereview.appspot.com/561360043/diff/547520043/Documentation/contributor/quick-start.itexi
> File Documentation/contributor/quick-start.itexi (right):
> 
All done.
Thank you, Werner!

https://codereview.appspot.com/561360043/

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-30 Thread michael . kaeppler--- via Discussions on LilyPond development 
On 2020/01/28 23:58:01, Dan Eble wrote:
> Good work.  Needs some revision.
> 
>
https://codereview.appspot.com/561360043/diff/547520043/Documentation/contributor/quick-start.itexi
> File Documentation/contributor/quick-start.itexi (right):

All done.

Dan, many thanks for working through this. 
I should get some lessons in English punctuation. :)

https://codereview.appspot.com/561360043/