Hide Forgot
Spec URL: http://oget.fedorapeople.org/review/tex-musixtex.spec SRPM URL: http://oget.fedorapeople.org/review/tex-musixtex-0.114-1.src.rpm Description: MusicTeX provides TeX extensions for music typesetting. It allows you to use TeX to write polyphonic, orchestral or instrumental music. MusixTeX is growing up from MusicTeX and has advantages both in set of macros and quality of output. The package contains source files (macros, styles), fonts (mf, tfm) and the MusiXTeX+plain format. Rpmlint: tex-musixtex.noarch: W: dangling-relative-symlink /usr/share/texmf/fonts/type1/public/musixtex ../../../../fonts/ctan-musixtex/ This one is resolved through dependencies tex-musixtex-doc.noarch: W: no-documentation Well, this package is TeX documentation, which goes to its own TeX folder. Notes: The upstream version of this package is T114. The project is pretty mature and I don't think they will change the release version scheme. I set the Fedora version as 0.114, which is also the way this is versioned in Debian.
Taking this review, since I own a couple of TeX font packages: - Could you use the ctan archive for all the sources? - Could you download the sources from the said archive with "wget -N" or similar mechanism to preserve the timestamp on the files? (md5sums match) - _texmf_main is declared in /etc/rpm/macros.texlive from texlive-texmf, so the declaration -f _texmf is unnecessary. (btw if it is not a system macro, it better not start with an "_"). I believe the "BuildRequires: kpsewhich" can go as well after this. - Could you please specify a little more explicitly the directories in the files section? The spec is correct as is, but the directory ownerships are not obvious to review, especially in the future. (You can reuse the variables MUSIXTEXDIR etc). - I think the package should update the map files (running updmap-sys). Please see tetex-font-kerkis in F-10 (or ctan-kerkis-fonts, subpackage tex-kerkis) in rawhide for an example. No major problems found, fix the above and I will approve it.
Thanks. I have a few questions: * SOURCE0 and SOURCE1 are already hosted in ctan. Can you give me a specific page (if it is at a different place at ctan) where I should download them? * I'm not very familiar with the map file updating. The package already contains two map files in %{_texmf_main}/fonts/map/dvipdfm/%{texname}/ %{_texmf_main}/fonts/map/dvips/%{texname}/ Do these map files need updated? If yes, why? (I successfully ran musixtex on some musixtex documents to produce dvi, ps and pdf files without updating any map file) Or is there a system map file that needs to be updated using these map files? Do I have to call updmap-sys once for each map file?
(In reply to comment #2) > Thanks. I have a few questions: > > * SOURCE0 and SOURCE1 are already hosted in ctan. Can you give me a specific > page (if it is at a different place at ctan) where I should download them? Oops, sorry, I misread the URL for the Source0 link. My bad. > * I'm not very familiar with the map file updating. The package already > contains two map files in > %{_texmf_main}/fonts/map/dvipdfm/%{texname}/ > %{_texmf_main}/fonts/map/dvips/%{texname}/ > Do these map files need updated? If yes, why? (I successfully ran musixtex on > some musixtex documents to produce dvi, ps and pdf files without updating any > map file) > Or is there a system map file that needs to be updated using these map files? The latter. updmap-sys updates updmap.cfg which lists the active map files. In my system, musix.map is currently commented out in updmap.cfg, so it will not be read. I suspect that in your system the line musix.map is present in your updmap.cfg either through custom editing or from a manual musixtex install. We must also care of removing (or commenting out) that line when uninstalling. > Do I have to call updmap-sys once for each map file? No, we don't actually call updmap-sys to update the map files, but to point out that a map file called musix.map exists in the respective directories (be they dvips or dvipdfm is irrelevant). I suspect that in your system the line musix.map is present in your updmap.cfg either through custom editing or from a manual musixtex install. We must also care of removing (or commenting out) that line when uninstalling.
Files updated: Spec URL: http://oget.fedorapeople.org/review/tex-musixtex.spec SRPM URL: http://oget.fedorapeople.org/review/tex-musixtex-0.114-2.src.rpm %changelog - Use standard macros from /etc/rpm/macros.texlive - Drop BR: kpsewhich - %%files section is made more explicit - Update updmap.cfg via updmap-sys in post&postun > (You can reuse the variables MUSIXTEXDIR etc). I couldn't use them because rpmbuild fails when a %files entry doesn't start with "/".
(In reply to comment #4) > > (You can reuse the variables MUSIXTEXDIR etc). > I couldn't use them because rpmbuild fails when a %files entry doesn't start > with "/". News to me. I guess this is because they are variables and not rpm macros and in another section of the rpm spec. You could walk around this by declaring these variables as macros and using them as such, e.g. %define musixtexdir %{_texmf_main}/tex/generic/%{texname} and then using %{musixtexdir} instead of $MUSIXTEXDIR The only reason I would insist on that is for consistency reasons, so you could either: a) drop the variables alltogether, or b) change them to macros, so that you can re-use them at the %files section I am ok with all the other issues, and this is a style issue, so this package is APPROVED.
(Please keep the assignee to the reviewer)
Thanks for the review. I will switch those to macros. New Package CVS Request ======================= Package Name: tex-musixtex Short Description: Sophisticated music typesetting Owners: oget Branches: F-9 F-10 InitialCC:
cvs done.
tex-musixtex-0.114-3.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
tex-musixtex-0.114-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines > SHOULD: The reviewer should test that the package functions as described. > A package should not segfault instead of running, for example. tex-musixtex-doc cannot even be installed!
True, I didn't catch this. Please place spaces in the doc subpackage in the Requires: line as in: --- tex-musixtex.spec 3 Feb 2009 16:47:29 -0000 1.2 +++ tex-musixtex.spec 5 Feb 2009 13:08:40 -0000 @@ -26,7 +26,7 @@ %package doc Summary: Documentation for %{name} Group: Applications/Publishing -Requires: %{name}=%{version}-%{release} +Requires: %{name} = %{version}-%{release} %description doc MusicTeX provides TeX extensions for music typesetting. It allows you to use (and bump the release number to reflect this)
Weird that none of us caught this before. Also weird that rpm can't handle this situation. Anyways, I fixed and built this for F-9, F-10 and rawhide. I hope it didn't cause much trouble to people.
Not only that, but rpmlint doesn't catch it, either. Perhaps time to look into filing an enhancement request with rpmlint. Or even a patch; heck, I'm learning Python anyway.
tex-musixtex-0.114-4.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
tex-musixtex-0.114-4.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.