Bug 189656 - Review Request: lilypond - A typesetting system for music notation
Review Request: lilypond - A typesetting system for music notation
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On: 189655
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-04-22 01:15 EDT by Quentin Spencer
Modified: 2014-02-10 09:44 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-24 09:59:01 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+


Attachments (Terms of Use)
Log from last failed build attempt (728.12 KB, text/plain)
2006-05-02 14:14 EDT, Jason Tibbitts
no flags Details

  None (edit)
Description Quentin Spencer 2006-04-22 01:15:17 EDT
Spec URL: http://webpages.charter.net/qspencer/rpm/lilypond.spec
SRPM URL: http://webpages.charter.net/qspencer/rpm/lilypond-2.8.1-1.src.rpm
Description:
LilyPond is an automated music engraving system. It formats music beautifully and automatically, and has a friendly syntax for its input files.

Note: All of the build dependencies are in Fedora Core or Extras, except for mftrace, which I have also submitted for review (see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189655), so I recommend that a prospective reviewer review both of these packages together.
Comment 1 Jason Tibbitts 2006-04-23 17:27:53 EDT
A few quick comments:

Referencing %{_datadir}/locale/ in %files is forbidden; you should
BuildRequires: gettext and use %find_lang.

I'm not sure why RPM would fail to strip your shared libraries; brp-strip-shared
looks like it will plow through everything in the buildroot for anything that
file calls a "shared object".  In addition, I'm concerned that stripping
manually might break the -debuginfo package.

Is it worth adding a desktop file for .ly files?

Be aware that Fedora development has vim 7 which stores its data in
/usr/share/vim/vim70e.
Comment 2 Quentin Spencer 2006-04-25 17:05:31 EDT
OK, I added gettext and %find_lang. It seems to be working properly, though I'm
not really familiar with how to use them.

I have changed the vim directory to match that of development.

I agree that manually stripping object files is undesirable, but it doesn't seem
to get it any other way. Running rpmlint on the resulting rpm gives:

W: lilypond unstripped-binary-or-object /usr/lib/lilypond/2.8.1/python/midi.so
W: lilypond file-not-utf8 /usr/share/info/music-glossary.info.gz

(Note that ignoring the not-utf8 error for now because I think the best way to
fix this is in upstream). Is there something I need to do to make sure it looks
for the .so file? It's only one file and not that big, so I suppose maybe it's
not a big deal to have debuginfo in it.

New spec and srpm:
http://webpages.charter.net/qspencer/rpm/lilypond.spec
http://webpages.charter.net/qspencer/rpm/lilypond-2.8.1-2.src.rpm
Comment 3 Ville Skyttä 2006-04-26 04:28:54 EDT
$ rpmls.sh lilypond-2.8.1-2.x86_64.rpm  | grep midi.so
-rw-r--r--  /usr/lib64/lilypond/2.8.1/python/midi.so

Executable bits missing, that's probably why it's not automatically stripped.
Comment 4 Quentin Spencer 2006-05-02 11:46:02 EDT
You were right. With the correct permissions, the file is now automatically
stripped. I've uploaded updated files:

http://webpages.charter.net/qspencer/rpm/lilypond-2.8.1-3.src.rpm
http://webpages.charter.net/qspencer/rpm/lilypond.spec

As I noted earlier, I'm ignoring the "file-not-utf8" warning from rpmlint, and
I'm also not going to deal with adding a desktop file for the time being. If you
consider either of these important for approval, I'm willing to reconsider.
Comment 5 Jason Tibbitts 2006-05-02 12:49:20 EDT
I just tried to build in mock and received this:

----
ERROR: Please install required programs:  Python.h (python-devel, python-dev or
libpython-dev package)

See INSTALL.txt for more information on how to build LilyPond
----

I added BuildRequires: python-devel and got a bunch of errors, which I'll try to
condense:

----
kpathsea: Running mktexfmt mf.base
/usr/bin/mktexfmt: line 331: /usr/share/texmf/texconfig/tcfmgr: No such file or
directory
fmtutil: config file `fmtutil.cnf' not found.
I can't find the base file `mf.base'!
make[1]: *** [out/feta11.tfm] Error 1
----

tcfgmgr is in tetex, so I added BR: tetex, but then I get to one I can't figure out:

cat out/feta23.lisp \
out/parmesan23.lisp \
out/feta-alphabet23.lisp > out/feta23.otf-table
cat out/feta26.lisp \
out/parmesan26.lisp \
out/feta-alphabet26.lisp > out/feta26.otf-table
cat: out/parmesan23.lisp: No such file or directory
make[1]: *** [out/feta23.otf-table] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: *** wait: No child processes.  Stop.
make: *** [all] Error 2

Full build log available on request.
Comment 6 Jason Tibbitts 2006-05-02 12:56:55 EDT
More oddness.  On a hunch I disabled parallel make (which turns up lots of
errors on an 8-CPU machine) and to:

cat out/feta-braces-h.lisp \
 \
 > out/feta-braces-h.otf-table
cat out/feta-braces-i.lisp \
 \
 > out/feta-braces-i.otf-table
make[1]: *** No rule to make target `unknown/c059013l.pfb', needed by
`out/CenturySchL-Ital.ttf'.  Stop.
make[1]: Leaving directory `/builddir/build/BUILD/lilypond-2.8.1/mf'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.5626 (%build)
Comment 7 Jason Tibbitts 2006-05-02 14:14:27 EDT
Created attachment 128504 [details]
Log from last failed build attempt
Comment 8 Quentin Spencer 2006-05-02 15:05:02 EDT
Thanks for the logs. It appears that the configure script looks for the location
of some fonts using locate, and the default location is "unknown" if locate is
not present. Fortunately, you can work around this with a configure option that
specifies the directory. I have also disabled parallel builds in response to
your report, although it may be worth testing them one more time. I've uploaded
new files:

http://webpages.charter.net/qspencer/rpm/lilypond.spec
http://webpages.charter.net/qspencer/rpm/lilypond-2.8.1-4.src.rpm
Comment 9 Jason Tibbitts 2006-05-02 16:02:21 EDT
This is tough; now I'm getting a segfault:

Initializing FontConfig...
Rebuilding FontConfig cache /builddir/.rh-fontconfig/.fonts.cache-2. this may
take a while...
adding font directory:
/builddir/build/BUILD/lilypond-2.8.1/out/share/lilypond/current/fonts/otf/
adding font directory:
/builddir/build/BUILD/lilypond-2.8.1/out/share/lilypond/current/fonts/type1/Building
font database.
/bin/sh: line 1: 23617 Segmentation fault     
/builddir/build/BUILD/lilypond-2.8.1/lily/out/lilypond --verbose
/builddir/build/BUILD/lilypond-2.8.1/ly/generate-documentation
make[2]: *** [out/lilypond-internals.texi] Error 139
make[2]: Leaving directory `/builddir/build/BUILD/lilypond-2.8.1/Documentation/user'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/builddir/build/BUILD/lilypond-2.8.1/Documentation'
make: *** [all] Error 2

I'll try an i386 build.
Comment 10 Jason Tibbitts 2006-05-02 16:25:05 EDT
An i386 build segfaults as well, in the same place.  This is all running on an
x86_64 system; I'll try to get mock running on an i386 system just to make sure.

Note that I can't try FC5 builds due to lack of mftrace in that repo.
Comment 11 Jason Tibbitts 2006-05-09 12:33:32 EDT
I have not been able to get through a build on any combination FC5 or
development, x86_64 or i386, without lilypond segfaulting as above.
Comment 12 Quentin Spencer 2006-05-14 02:12:47 EDT
I finally tracked down the problem. The segfaults happen when trying to create a
font cache in a file in ~/.rh-fontconfig/ . If the directory is present, it
doesn't fail, but if it is not present, it tries to write to the file in the
directory, resulting in a crash. There is a new release of lilypond which
appears to avoid this problem. I have built it in FC-5 having removed the font
directory, but haven't tried mock yet.

New spec and SRPM:
http://webpages.charter.net/qspencer/rpm/lilypond-2.8.2-1.src.rpm
http://webpages.charter.net/qspencer/rpm/lilypond.spec
Comment 13 Jason Tibbitts 2006-05-14 23:14:06 EDT
I'm afraid it still segfaults at the same place on FC5.  (I think guile is
broken in development, which keeps the build from making past it %configure there.)

Initializing FontConfig...
Rebuilding FontConfig cache /builddir/.rh-fontconfig/.fonts.cache-2. this may
take a while...
adding font directory:
/builddir/build/BUILD/lilypond-2.8.2/out/share/lilypond/current/fonts/otf/
adding font directory:
/builddir/build/BUILD/lilypond-2.8.2/out/share/lilypond/current/fonts/type1/Building
font database.
/bin/sh: line 1: 11282 Segmentation fault     
/builddir/build/BUILD/lilypond-2.8.2/lily/out/lilypond --verbose
/builddir/build/BUILD/lilypond-2.8.2/ly/generate-documentation
Comment 14 Quentin Spencer 2006-05-15 13:02:59 EDT
You're right, it wasn't fixed. I thought I had duplicated the conditions to make
the build fail, but I guess I hadn't. Anyway, the upstream author responded to
my bug report with what turned out to be a fairly trivial patch, and this time I
think it's actually fixed.

Updated files:
http://webpages.charter.net/qspencer/rpm/lilypond-2.8.2-2.src.rpm
http://webpages.charter.net/qspencer/rpm/lilypond.spec
Comment 15 Jason Tibbitts 2006-05-15 16:40:54 EDT
You're right, that was a trivial patch; sorry I couldn't have been more help in
debugging it.

In any case, guile in development is still busted, but a mock build on FC5
x86_64 ran through to completion.  (!)  Now, down to the review.

Issues:
Please consider removing BuildRequires: gcc-c++.  This is no longer a blocker,
but it tends to annoy folks.

rpmlint complains:
W: lilypond file-not-utf8 /usr/share/info/music-glossary.info.gz

This is odd because music-glossary.tely explicitly says: @documentencoding utf-8
I'm not completely sure what should be done here; there are a few international
characters in that file; you could try running

iconv -f iso-8859-1 -t utf-8 music-glossary.info > music-glossary.info.utf8
mv music-glossary.info.utf8 music-glossary.info

in $RPM_BUILD_ROOT%{_infodir}.

You should consider ghosting the .pyo files instead of shipping them in the package.

Nothing in the input directory seems to be shipped.  Might it be worth it to
package these up in an -examples subpackage or something like that?

I noticed this in the build.log; I wonder if this indicates a problem:
Compiling fdl.texi...
fdl.texi is up to date.
lilypond-book.py: warning: option --psfonts not used
lilypond-book.py: warning: processing with dvips will have no fonts

DVIPS usage:
    dvips -h ./out/lilypond.psfonts ./out/lilypond.dvi
lilypond-book.py (GNU LilyPond) @TOPLEVEL_VERSION@

Perhaps I'm just missing something, but I'm not seeing the installed menual.  I
see a few relatively small info files but for some reason I thought lilypond had
an impressively large manual.  Is it all contained in a few info files or is
something being left out?


Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* license field matches the actual license.
* license is open source-compatible; license text is included in the package.
* source files match upstream:
   7053cad744c6c62d5895983b67220e26  lilypond-2.8.2.tar.gz
   7053cad744c6c62d5895983b67220e26  lilypond-2.8.2.tar.gz-srpm
* latest version is being packaged.
O BuildRequires are proper (well, perhaps gcc-c++ could go)
* package builds in mock (FC5, x86_64).
X rpmlint is silent.
* final provides and requires are sane.
O shared libraries are present, but they are internal to the Python interface
and so ldconfig does not need to be run.
* package is not relocatable.
* owns the directories it creates.
O doesn't own any directories it shouldn't (owns /usr/share/emacs/site-lisp and
/usr/share/vim/vim70, but this is acceptable).
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
O %check is not present.  There seems to be some sort of regression test, but
I'm not sure how you'd run it.
* scriptlets are present and sane.
* code, not content.
? not sure about the documentation at this point.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
X Python .pyo files are present and not ghosted.
* not a GUI app.
Comment 16 Quentin Spencer 2006-05-15 23:58:08 EDT
I'll take care of the c++ dependency and the ghosted pyo files, and I'll see if
your suggestion fixes the encoding on the info file. The remaining issue is the
documentation. lilypond does indeed have a very large set of documentation,
which is created by running "make web" in the build directory. The problem is
that it will fail about half way through unless you have ghostscript 8.5. Both
FC5 and devel have 8.15.x. I had hoped that FC6 would update to have 8.5, but it
still hasn't changed in devel. Other than building the docs, I don't see this as
a huge problem as it's really just a couple of cases, and lilypond is otherwise
99% functional with ghostscript 8.15.

Anyway, my thinking was that I would just leave out the docs until ghostscript
could build them and then add them as a subpackage later on. However, the docs
are available as a separate tarball from the lilypond web site, so I suppose I
could just include it in the SRPM for the time being, but it is 24 MB.
Comment 17 Jason Tibbitts 2006-05-16 12:39:25 EDT
I think it would be a good idea to package the manual, given that we don't know
if GS 8.50 will even be in FC6 and that it will probably never make it into FC4
or FC5.  This package has good documentation so it would be a shame to leave it
out.  It will need to be in a -doc subpackage given its size.
Comment 18 Quentin Spencer 2006-05-17 20:26:21 EDT
OK, I have fixed the encoding and buildrequires issues and have bundled the
tarball containing the documentation, from which I'm creating a -doc package.
The SRPM is now 28 MB, which bigger than the space I had available, so I'm
putting this on my own web server, which is unfortunately behind a 128 KB
connection, so it is a long download. You may have better luck just getting the
spec and downloading the other source tarball from upstream. Anyway, the new
spec and SRPM can be found at:
http://qspencer.homeip.net/rpm/

The large documentation package does raise one issue: it seems a subpackage
can't be noarch if the main package isn't, so we'll have these large -doc
packages for each arch, which seems wasteful. Looking around core and extras, it
seems some projects actually create a separate SRPM so the docs can be noarch.
I'm not sure whether that makes sense here or not.
Comment 19 Jason Tibbitts 2006-05-19 12:46:54 EDT
First off, there are three empty PNG files in the documentation:

E: lilypond-doc zero-length
/usr/share/doc/lilypond-doc-2.8.2/input/mutopia/J.S.Bach/wtk1-fugue2.png
E: lilypond-doc zero-length
/usr/share/doc/lilypond-doc-2.8.2/input/mutopia/W.A.Mozart/mozart-hrn-3.png
E: lilypond-doc zero-length
/usr/share/doc/lilypond-doc-2.8.2/input/mutopia/E.Satie/petite-ouverture-a-danser.png

Any idea about these?  They come that way from upstream.  I guess there's not
much point in including them if they're empty.

Other than that I have no issues.  I do understand your point about the size of
the documentation.  Technically you could build it as a completely separate
package because you're using a different upstream tarball, but once GS 8.50 is
in you'll be able to build it directly and you'd be back to the same problem. 
Or you could just stick with using the upstream tarball.  The packages with
separate -doc packages not built from the main RPM all seem to be shipping
preformatted documentation from upstream.

I'm going to approve this package and leave it to you to fix the three empty
files when you check in.  Or if you like, you can strip the documentation from
the base package and submit a separate package, which I promise to review quickly.

APPROVED
Comment 20 Quentin Spencer 2006-05-20 02:28:56 EDT
I've decided to go ahead and create a lilypond-doc package, since upstream
maintains provides a tarball for it. Review request coming soon...
Comment 21 Jason Tibbitts 2006-05-24 09:43:53 EDT
This seems to be built and out to the mirrors for devel and FC5; any reason this
ticket is still open?
Comment 22 Quentin Spencer 2006-05-24 09:59:01 EDT
No, just forgot to close it.
Comment 23 Gwyn Ciesla 2014-02-10 09:43:01 EST
Package Change Request
======================
Package Name: lilypond
New Branches: el6 epel7
Owners: cicku limb
Comment 24 Gwyn Ciesla 2014-02-10 09:44:11 EST
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.