Re: GOP-PROP 5: build system output (final)

[Reinhold Kainhofer]

Am Dienstag, 9. August 2011, 12:07:12 schrieb Phil Holmes:
> I know you have many 10s of times more experience with lilypond than I do,
> and I agree with 99% of what you say...  But...
> 
> The truth is, no-one pays any attention to warnings during the build
> process.  If I grep the output of make for the word "warning" I get 380
> lines in the middle of 37,000 other lines of output.  Most of these
> warnings come from other parts of the build system, but no-one's looking
> at them. There are nine warnings from the code compiler:

And that number is really amazing and absolutely proves my point: Coders PAY 
attention to warnings and usually fix them in the code they write.
I would have expected way more compiler warnings in the C++ code!

To give you numbers: When I wrote the patch for loglevels, it took me several 
compiler errors and at least 5 compiler warnings (that I explicitly remember). 
So having only 9 warnings in our codebase (four of which are in the 
lexer/parser, which hardly anyone of us really understands!) is amazing.


> /flower/file-name.cc:100: warning: ignoring return value of 'char*
> getcwd(char*, size_t)', declared with attribute warn_unused_result
> /lily/glissando-engraver.cc:124: warning: comparison between signed and
> unsigned integer expressions
> /lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int
> chdir(const char*)', declared with attribute warn_unused_result
> /lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 'sev'
> /lily/skyline.cc:395: warning: unused parameter 'a'


-) The "unused parameter" warnings are harmless and easy to fix. 
-) The "ignoring return value" ar a bit trickier, but should also be easy to
   fix. They are bad coding style, in particular:
    o) the file-name.cc one returns a string with undefined contents if getcwd
       fails, which might either crash guile or lead to other bugs. 
    o) the lily-parser-scheme.cc warning was also a programming error (if you
       used --output=dir/file, and dir/ had the wrong permissions, then
       lilypond would not print an error, but simply put the output file into
       the current directory!).

I'll post a patch for all of them.


> out/parser.cc:2392: warning: conversion to 'short int' from 'int'
> may alter its value
> /lily/lexer.ll:634: warning, rule cannot be matched
> /lily/lexer.ll:637: warning, rule cannot be matched
> /lily/lexer.ll:706: warning, -s option given but default rule can be
> matched 

Anyone here who knows more about the lexer and the parser?

> If warnings are there to prevent code errors, why have these not been
> fixed?

When coders work on some code, they usually only look at the warnings caused 
by their own code.


> In practice, displaying warnings on the console is a waste.  It's really
> far, far better to put them in a file, where concerned individuals can grep
> the file, open it in an editor and view it, etc.

so, you really expect us developers to do the following when compiling:

1) change some code
2) run make
3) WAIT until the build is finished (which takes a LONG time)
4) find the logfile
5) open the logfile
6) search the part of the logfile that is about the code you changed
7) Check the warnings
8) fix them  -> start at 1)

Currently, my workflow is:

1) change some code
2) run make
3) look at the console output until the file you changed is compiled (i.e.
   scrolls by)
4) check whether any warning is displayed 
   (terminal windows have toolbars, so they won't scroll away!)
5) fix them (meanwhile the build can continue!) -> start at 1)

Typically takes some 15 seconds total.

> I would have got nowhere
> understanding the build system unless I routinely redirected the build
> output to file.

For analyzing that's absolutely true.

> They're not being ignored.  They're not even being seen.  Please address my
> point of how you would see them in 37,000 lines of console output.

On the other hand: we have 37.000 lines of make output and 9 compiler warnings 
from the C-code. If each takes 2 lines, that increases the make output by 0.05 
percent (by 0.0005). Is this really a significant reduction to remove the 
warnings???


Cheers,
Reinhold

PS: I should also mention that many of the other 500 warnings from a clean 
"make" build are messages printed by lilypond in --verbose mode:
warning: Relocation: is absolute: 
argv0=/home/reinhold/lilypond/lilypond/out/bin/lilypond
PATH=/home/reinhold/lilypond/lilypond/out/bin (voranstellen)
PATH wird auf 
/home/reinhold/lilypond/lilypond/out/bin:/home/reinhold/lilypond/lilypond/lily/out:/home/reinhold/lilypond/lilypond/scripts/build/out:/home/reinhold/lilypond/lilypond/scripts/out:/home/reinhold/lilypond/lilypond/lily/out:/home/reinhold/lilypond/lilypond/scripts/build/out:/home/reinhold/lilypond/lilypond/scripts/out:/home/reinhold/.build/bin:/home/reinhold/.bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games::
 
gesetzt
Warnung: Verlagerung: Kompilier-Datenverzeichnis=, neues 
Datenverzeichnis=/home/reinhold/lilypond/lilypond/out/share/lilypond//current
Warnung: Verlagerung: 
framework_prefix=/home/reinhold/lilypond/lilypond/out/bin/..

All three of them should actually not be warnings, but are simple debug output 
when lilypond is run with --relocate. My loglevels patch already changes those 
message to debug output.


And finally, most of the warnings are from makeinfo:

LANG= makeinfo --enable-encoding -I 
/home/reinhold/lilypond/lilypond/Documentation -I. -I./out --
output=out/lilypond-essay.info out/essay.texi
/home/reinhold/lilypond/lilypond/Documentation/out//essay/engraving.texi:62: 
Warnung: @image-Datei „lilypond/pictures/baer-suite1-fullpage.txt“ (für Text) 
nicht lesbar: Datei oder Verzeichnis nicht gefunden.

In other words, makeinfo tries to insert a *.txt file whenever we add a *.png 
graphics into the documentation. How shall we proceed with them?
-- 
------------------------------------------------------------------
Reinhold Kainhofer, address@hidden, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org