Spec URL: http://math.ifi.unizh.ch/fedora/spec/tclabc.spec SRPM URL: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/tclabc-1.0.7-1.src.rpm Description: Tclabc is designed to help on writing music in ABC notation.
{Not Official Reviewer} packaging looks ok. + Mockbuild is successfull for i386 FC6 - rpmlint on source rpm is not silent W: tclabc mixed-use-of-spaces-and-tabs The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. + rpmlint on binary rpm is silent - dist tag is NOT present + Buildroot is correct + source URL is correct + BR is correct + License used is GPL + License file LICENSE is included + MD5 sum on tarball is matching upstream tarball 34dbcb0177e11888d23ca7fa2304fb17 tclabc-1.0.7.tar.gz + No duplicate files
You may like to use sed -i -e 's|\t| |g' tclabc.spec to remove that rpmlint warning
Quick look: (In reply to comment #1) > + Mockbuild is successfull for i386 FC6 Really? I cannot rebuild this in mock. Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.91772 + umask 022 + cd /builddir/build/BUILD + LANG=C + export LANG + unset DISPLAY + cd /builddir/build/BUILD + rm -rf tclabc-1.0.7.fc6 + /bin/gzip -dc /builddir/build/SOURCES/tclabc-1.0.7.tar.gz + tar -xf - + STATUS=0 + '[' 0 -ne 0 ']' + cd tclabc-1.0.7.fc6 /var/tmp/rpm-tmp.91772: line 33: cd: tclabc-1.0.7.fc6: No such file or directory error: Bad exit status from /var/tmp/rpm-tmp.91772 (%prep) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.91772 (%prep) %{?dist} should not be added in version but in release. * Well, tk requires tcl, tk-devel requires tcl-devel and tk, so BuildRequires tcl tk tcl-devel Requires tcl are all unnecessary.
mtasaka, Gérard Milmeister has changed package and SPEC after i did revivew. I have that SPEC file which did not contained any dist tag. If you see my review i have clearly mentioned that - dist tag is NOT present Here is diff between those 2 SPECs --- tclabc_old.spec 2006-09-27 12:41:26.000000000 +0530 +++ tclabc.spec 2006-09-27 12:41:49.000000000 +0530 @@ -1,20 +1,20 @@ Name: tclabc -Version: 1.0.7 +Version: 1.0.7%{?dist} Release: 1 Summary: A Tcl interface and a Tk GUI to the ABC notation Group: Applications/Multimedia -License: GPL +License: GPL URL: http://moinejf.free.fr Source0: http://moinejf.free.fr/tclabc-1.0.7.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: tcl -BuildRequires: tk -BuildRequires: tcl-devel -BuildRequires: tk-devel +BuildRequires: tk +BuildRequires: tcl-devel +BuildRequires: tk-devel Requires: tcl -Requires: tk -Requires: abcm2ps +Requires: tk +Requires: abcm2ps %description
mtasaka, I also checked timestamps and filesizes of both SRPMS, one i have and the one that is online both are different. do u want MD5 checksum of that?
Hello, Parag: Well, anyway Gérard should upload new spec and srpm (with release tag incremented) to avoid confusion. You and me can wait for it.
This is the correct new srpm: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/tclabc-1.0.7-2.src.rpm
I will review this later.
Well, Reviewing this. 1. From http://fedoraproject.org/wiki/Packaging/Guidelines : * Requires and BuildRequires - The following Requires/BuildRequires are not needed. * BuildRequires: tcl <- required by tk * BuildRequires: tk <- required by tk-devel * BuildRequires: tcl-devel <- required by tk-devel * Requires: tcl <- required by tk - The results of rebuilding this between by normal rpmbuild and by mockbuild differ * The comparison of what tclabc-1.0.7-1.fc6 requires between the one rebuild by rpmbuild and by mockbuild: --- 2.txt 2006-09-28 23:34:40.000000000 +0900 +++ 1.txt 2006-09-28 23:34:15.000000000 +0900 @@ -1,7 +1,5 @@ /bin/sh abcm2ps -libasound.so.2 -libasound.so.2(ALSA_0.9) libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3) Build log also differ: ............. @@ -65,7 +341,7 @@ checking for dlopen in -ldl... yes checking for sound card... yes checking for SB AWE 32... yes -checking for the ALSA library... yes +checking for the ALSA library... no updating cache ./config.cache creating ./config.status creating Makefile ............. @@ -147,67 +424,69 @@ midi.c:2720: warning: ignoring return value of 'write', declared with attribute warn_unused_result gcc -shared tclabc.o abcparse.o change.o midi.o \ -o tclabc.so \ - -lasound + + exit 0 Check the BuildRequires dependency for alsa library. * Encoding - Some text files are encoded in ISO-8859-1 /usr/lib/tclabc/change.tcl /usr/lib/tclabc/detail.tcl /usr/lib/tclabc/help.tcl /usr/lib/tclabc/lang-de.tcl /usr/lib/tclabc/lang-fr.tcl /usr/lib/tclabc/lang-nl.tcl /usr/lib/tclabc/lang-ptBR.tcl /usr/lib/tclabc/lang-se.tcl /usr/lib/tclabc/play.tcl /usr/lib/tclabc/pref.tcl /usr/lib/tclabc/select.tcl /usr/lib/tclabc/tkabc.tcl /usr/lib/tclabc/tkorgan.tcl /usr/share/doc/tclabc-1.0.7/README * At least the encoding of /usr/share/doc/tclabc-1.0.7/README should be changed to UTF-8. * Also check if other tcl files can also have their encodings changed to UTF-8. * Timestamps - This package includes some html files in source files, so keeping timestamps for these html files are recommended. Use: make install prefix=....... INSTALL_DATA="install -c -p" to keep timestamps. 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : * The sources used to build the package... Use: http://moinejf.free.fr/tclabc-%{version}.tar.gz for Source0 3. Other things I have noticed. * Well, I don't understand how to use this, however, when I try "tkabc bachto.mid" (bachto.mid is MIDI file), I get: [tasaka1@localhost TEMP]$ tkabc bachto.mid no output MIDI synth open: No such file or directory cannot open MIDI in '/dev/midi00' .. is this okay? It seems I cannot hear any sound.
(In reply to comment #9) > Well, Reviewing this. > > 1. From http://fedoraproject.org/wiki/Packaging/Guidelines : > > * Requires and BuildRequires > - The following Requires/BuildRequires are not needed. > * BuildRequires: tcl <- required by tk > * BuildRequires: tk <- required by tk-devel > * BuildRequires: tcl-devel <- required by tk-devel > * Requires: tcl <- required by tk Ok. > Check the BuildRequires dependency for alsa library. Ok. > * At least the encoding of /usr/share/doc/tclabc-1.0.7/README > should be changed to UTF-8. > * Also check if other tcl files can also have their encodings changed > to UTF-8. Ok. > * Timestamps > - This package includes some html files in source files, so keeping > timestamps for these html files are recommended. This I don't understand. Why is it necessary? > 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : > > * The sources used to build the package... > Use: http://moinejf.free.fr/tclabc-%{version}.tar.gz > for Source0 I see nothing in the guidelines about this, but ok. > 3. Other things I have noticed. > * Well, I don't understand how to use this, however, when I try > "tkabc bachto.mid" (bachto.mid is MIDI file), I get: > > [tasaka1@localhost TEMP]$ tkabc bachto.mid > no output MIDI synth > open: No such file or directory > cannot open MIDI in '/dev/midi00' The MIDI devices have to be configured in the preferences dialog. When alsa is used, something like "17:0" has to be used for an alsa midi devices. However this depends on the soundcard.
(In reply to comment #10) > > * Timestamps > > - This package includes some html files in source files, so keeping > > timestamps for these html files are recommended. > This I don't understand. Why is it necessary? Keeping timestamps is preferable because it make it clearer * if a packager like you added some modification for the original sources or text files * when these text files or source files are originally written So this is preferable especially when the package created contains large number of documentations (e.g. kernel-doc).
Here is the update with fixes: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/tclabc-1.0.7-3.src.rpm
Well, sometimes iconv fails so I usually use: iconv -f iso-8859-1 -t utf-8 $f -o $f.iconv && mv $f.iconv $f Perhaps this is preferable (this is not a blocker). Other things are okay. ---------------------------------------------------------------------- This package (tclabc) is APPROVED by me.
(In reply to comment #13) > Well, sometimes iconv fails so I usually use: > > iconv -f iso-8859-1 -t utf-8 $f -o $f.iconv && mv $f.iconv $f > > Perhaps this is preferable (this is not a blocker). Ok. Package imported. Build on FC-5 and devel. added entries to owners.list, comps-fe5.xml.in and comps-fe6.xml.in. Thanks for the review.