Bug 481071 - Review Request: tex-musixtex - Sophisticated music typesetting
Summary: Review Request: tex-musixtex - Sophisticated music typesetting
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Sarantis Paskalis
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: ctan-musixtex-fonts
Blocks: 480887
TreeView+ depends on / blocked
 
Reported: 2009-01-21 23:52 UTC by Orcan Ogetbil
Modified: 2009-02-06 05:25 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-06 05:19:18 UTC
Type: ---
Embargoed:
paskalis: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Orcan Ogetbil 2009-01-21 23:52:32 UTC
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.

Comment 1 Sarantis Paskalis 2009-01-30 10:10:52 UTC
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.

Comment 2 Orcan Ogetbil 2009-01-30 11:18:35 UTC
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?

Comment 3 Sarantis Paskalis 2009-01-30 11:58:20 UTC
(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.

Comment 4 Orcan Ogetbil 2009-01-30 18:41:38 UTC
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 "/".

Comment 5 Sarantis Paskalis 2009-02-02 09:01:12 UTC
(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.

Comment 6 Mamoru TASAKA 2009-02-02 13:03:00 UTC
(Please keep the assignee to the reviewer)

Comment 7 Orcan Ogetbil 2009-02-02 16:11:11 UTC
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:

Comment 8 Kevin Fenzi 2009-02-03 04:21:28 UTC
cvs done.

Comment 9 Fedora Update System 2009-02-05 02:11:16 UTC
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.

Comment 10 Fedora Update System 2009-02-05 02:13:26 UTC
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.

Comment 11 Michael Schwendt 2009-02-05 09:16:40 UTC
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!

Comment 12 Sarantis Paskalis 2009-02-05 13:13:43 UTC
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)

Comment 13 Orcan Ogetbil 2009-02-05 16:52:23 UTC
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.

Comment 14 J. Randall Owens 2009-02-05 20:29:47 UTC
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.

Comment 15 Fedora Update System 2009-02-06 05:19:14 UTC
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.

Comment 16 Fedora Update System 2009-02-06 05:25:01 UTC
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.


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