Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)
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)
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)
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
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)
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
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)
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
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"
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"
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"
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"
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"
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"
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"
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"
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"
Le 30/01/2020 à 19:39, David Kastrup a écrit : Jean-Charles Malahieude writes: 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 If it may help, my dump is enclosed. # installing zipimport hook import zipimport # builtin # installed zipimport hook # trying /home/jcharles/GIT/Stable/python/out/site.so # trying /home/jcharles/GIT/Stable/python/out/sitemodule.so # trying /home/jcharles/GIT/Stable/python/out/site.py # trying /home/jcharles/GIT/Stable/python/out/site.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/site.so # trying /home/jcharles/GIT/Stable/python/auxiliar/sitemodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/site.py # trying /home/jcharles/GIT/Stable/python/auxiliar/site.pyc # trying /usr/lib64/python2.7/site.so # trying /usr/lib64/python2.7/sitemodule.so # trying /usr/lib64/python2.7/site.py # /usr/lib64/python2.7/site.pyc matches /usr/lib64/python2.7/site.py import site # precompiled from /usr/lib64/python2.7/site.pyc # trying /home/jcharles/GIT/Stable/python/out/os.so # trying /home/jcharles/GIT/Stable/python/out/osmodule.so # trying /home/jcharles/GIT/Stable/python/out/os.py # trying /home/jcharles/GIT/Stable/python/out/os.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/os.so # trying /home/jcharles/GIT/Stable/python/auxiliar/osmodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/os.py # trying /home/jcharles/GIT/Stable/python/auxiliar/os.pyc # trying /usr/lib64/python2.7/os.so # trying /usr/lib64/python2.7/osmodule.so # trying /usr/lib64/python2.7/os.py # /usr/lib64/python2.7/os.pyc matches /usr/lib64/python2.7/os.py import os # precompiled from /usr/lib64/python2.7/os.pyc import errno # builtin import posix # builtin # trying /home/jcharles/GIT/Stable/python/out/posixpath.so # trying /home/jcharles/GIT/Stable/python/out/posixpathmodule.so # trying /home/jcharles/GIT/Stable/python/out/posixpath.py # trying /home/jcharles/GIT/Stable/python/out/posixpath.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/posixpath.so # trying /home/jcharles/GIT/Stable/python/auxiliar/posixpathmodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/posixpath.py # trying /home/jcharles/GIT/Stable/python/auxiliar/posixpath.pyc # trying /usr/lib64/python2.7/posixpath.so # trying /usr/lib64/python2.7/posixpathmodule.so # trying /usr/lib64/python2.7/posixpath.py # /usr/lib64/python2.7/posixpath.pyc matches /usr/lib64/python2.7/posixpath.py import posixpath # precompiled from /usr/lib64/python2.7/posixpath.pyc # trying /home/jcharles/GIT/Stable/python/out/stat.so # trying /home/jcharles/GIT/Stable/python/out/statmodule.so # trying /home/jcharles/GIT/Stable/python/out/stat.py # trying /home/jcharles/GIT/Stable/python/out/stat.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/stat.so # trying /home/jcharles/GIT/Stable/python/auxiliar/statmodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/stat.py # trying /home/jcharles/GIT/Stable/python/auxiliar/stat.pyc # trying /usr/lib64/python2.7/stat.so # trying /usr/lib64/python2.7/statmodule.so # trying /usr/lib64/python2.7/stat.py # /usr/lib64/python2.7/stat.pyc matches /usr/lib64/python2.7/stat.py import stat # precompiled from /usr/lib64/python2.7/stat.pyc # trying /home/jcharles/GIT/Stable/python/out/genericpath.so # trying /home/jcharles/GIT/Stable/python/out/genericpathmodule.so # trying /home/jcharles/GIT/Stable/python/out/genericpath.py # trying /home/jcharles/GIT/Stable/python/out/genericpath.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/genericpath.so # trying /home/jcharles/GIT/Stable/python/auxiliar/genericpathmodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/genericpath.py # trying /home/jcharles/GIT/Stable/python/auxiliar/genericpath.pyc # trying /usr/lib64/python2.7/genericpath.so # trying /usr/lib64/python2.7/genericpathmodule.so # trying /usr/lib64/python2.7/genericpath.py # /usr/lib64/python2.7/genericpath.pyc matches /usr/lib64/python2.7/genericpath.py import genericpath # precompiled from /usr/lib64/python2.7/genericpath.pyc # trying /home/jcharles/GIT/Stable/python/out/warnings.so # trying /home/jcharles/GIT/Stable/python/out/warningsmodule.so # trying /home/jcharles/GIT/Stable/python/out/warnings.py # trying /home/jcharles/GIT/Stable/python/out/warnings.pyc # trying /home/jcharles/GIT/Stable/python/auxiliar/warnings.so # trying /home/jcharles/GIT/Stable/python/auxiliar/warningsmodule.so # trying /home/jcharles/GIT/Stable/python/auxiliar/warnings.py # trying /home/jcharles/GIT/Stable/python/auxiliar/warnings.pyc # trying /usr/lib64/python2.7/warnings.so # trying /usr/lib64/python2.7/warningsmodule.so # trying /usr/lib64/python2.7/warnings.py # /usr/lib64/python2.7/warnings.pyc matches /usr/lib64/python2.7/warnings
Re: stable/2.20 branch does not complete "make doc"
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"
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"
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 It took about half an hour, and I don't know if it matters, but I build in the root directory. Cheers, -- Jean-Charles
Re: stable/2.20 branch does not complete "make doc"
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"
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"
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)
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: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)
On Jan 30, 2020, at 05:17, truer...@gmail.com wrote: > > How about this patch? > Sorry, not tested. Bringing the _FPU_SETCW() branch and the asm() branch closer together is an improvement. (I don't have the resources to test this patch.)
Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)
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: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)
Il giorno gio 30 gen 2020 alle ore 12:18 ha scritto: > 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? > > I see that it's possible to log in as root user without any password _even in the virtual machine_. Not good. I used the --password="" in the Makefile to avoid the step to set the password when starting the container with systemd-nspawn. In mkosi manual I read: --password= > > : Set the password of the root user. By default the root account is > locked. If this option is not used but a file mkosi.rootpw exists in the > local directory the root password is automatically read from it. > So we may remove the --password option to keep the root account disabled and use the mkosi.rootpw to set the password. I will test this and hopefully include it in LilyDev v3.
Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)
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 &bla = *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&paste. 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 differ
Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)
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)
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)
> > 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)
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)
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)
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)
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)
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)
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)
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 &fn, bool must_exist, int siz SCM ly_string2scm (std::string const &s); 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, &len); + + // 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)
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)
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 &bla = *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
> * 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
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?
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)
> 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
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)
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)
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
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)
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)
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)
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/