Spec URL: http://www.codemonkey.org.uk/junk/midisport-firmware.spec SRPM URL: http://www.codemonkey.org.uk/junk/midisport-firmware-1.2-1.src.rpm Description: Firmware for M-Audio/Midiman USB MIDI and Audio devices I based this specfile on the ivtv-firmware spec. TODO: add udev rules to autoload firmware.
Updated to add udev rules.
3 Release: 1 It is fine that this doesn't contain dist tag as this package doesn't ship any executable and is release independent. (Just a note to me and other reviewers who may eventually look at this.) 6 License: GPL This is no longer valid, it needs the version specification, e.g. GPL+, GPLv2 or GPLv3+ According to LICENSE file this is GPLv2+ 18 %setup -q -c No need for -c, tarball actually contains midisport-firmware-1.2 directory 26 install -pm 0644 *.{fw,mpg} $RPM_BUILD_ROOT/lib/firmware This is not correct, this package has not *.fw or *.mpg files. Did you mean *.ihx? It doesn't build due to this. I'll continue the review once it builds. 28 # Install the license with the firmware 29 mv LICENSE license-midisport.txt What was the reason for installing license in /lib/firmware? It should be included in %doc instead. Forthermore, please add README to %doc (and probably also ChangeLog). 31 install -pm 0644 81-midisport.rules $RPM_BUILD_ROOT/etc/udev/rules.d/ ... 46 /etc/udev/rules.d/81-midisport.rules Please replace occurences of /etc with %{_sysconfdir} 49 * Thu Feb 28 2008 Dave Jones <davej> Please include - version-release (sans disttag, " - 1.2-1" in this case) at the end of changelog header
Something from rpmlint: midisport-firmware.noarch: E: description-line-too-long This package contains the firmware for M-Audio/Midiman USB MIDI and Audio devices Your description lines must not exceed 79 characters. If a line is exceeding this number, cut it to fit in two lines. midisport-firmware.noarch: W: non-conffile-in-etc /etc/udev/rules.d/81-midisport.rules A non-executable file in your package is being installed in /etc, but is not a configuration file. All non-executable files in /etc should be configuration files. Mark the file as %config in the spec file. Please mark the udev rules file as %config. rpmlint will probably complain that it's not %config(noreplace). That's fine, ignore it. And some more issues: 32 install -pm 0644 81-midisport.rules $RPM_BUILD_ROOT/etc/udev/rules.d/ Replace 81-midisport.rules with %{SOURCE1} Create the directory $RPM_BUILD_ROOT%{_sysconfdir}/udev/rules.d/ prior to copying to it. That should be all.
One more thing; you rules seem to be incorrect. SUBSYSTEM=="usb", ENV{PRODUCT}=="763/1010/*", RUN+="/sbin/fxload -D $env{DEVNAME} -s /etc/firmware/MidiSportLoader.ihx -I /etc/firmware/MidiSport1x1.ihx" SUBSYSTEM=="usb", ENV{PRODUCT}=="763/1020/*", RUN+="/sbin/fxload -D $env{DEVNAME} -s /etc/firmware/MidiSportLoader.ihx -I /etc/firmware/MidiSport4x4.ihx" You install the files into /lib/firmware, not /etc/firmware
And yet one more cosmetical one: Change the Source0 tag not to point to a specific mirror: http://download.sourceforge.net/usb-midi-fw/midisport-firmware-%{version}.tar.gz
Both files updated with comments from above. Thanks for the review.
install -pm 0644 *.{*.ihx} $RPM_BUILD_ROOT/lib/firmware ^^^^^^^^^ You didn't mean this, did you? %doc license-midisport.txt README Changelog license file is called LICENSE and not license-midisport.txt -* Thu Feb 28 2008 Dave Jones <davej> - 1.2 +* Thu Feb 28 2008 Dave Jones <davej> - 1.2-1
fixed.
Thanks for the package, Dave! APPROVED
New Package CVS Request ======================= Package Name: midisport-firmware Short Description: firmware for M-Audio/Midiman USB MIDI & Audio device Owners: davej Branches: InitialCC: Cvsextras Commits: yes
cvs done.