Spec files and new src.rpms now uploaded http://www.all-the-johnsons.co.uk/emulation/downloads/lib765-0.3.3-1.spec http://www.all-the-johnsons.co.uk/emulation/downloads/libspectrum-0.2.2-2.spec http://www.all-the-johnsons.co.uk/emulation/downloads/libdsk-1.1.4-1.spec src rpm names libdsk-1.1.4-1.2.src.rpm lib765-0.3.3-1.2.src.rpm libspectrum-0.2.2-2.3.src.rpm Description: These libraries are required in order to run the fuse emulator (#166960).
I did actually mean a separate review for each library (i.e. each SRPM), then each one can be reviewed on its own merits and approved when ready. Otherwise each SRPM has to be individually reviewed and any blocker blocks all packages in this review.
Surely though as fuse needs all three then to review each on their merit does seem a bit daft. All three have to go through at the same time! I can see your point, lib765 may hold the other two up (say). Shall I keep as is, or farm three out and just do this one for libspectrum?
(In reply to comment #2) > Surely though as fuse needs all three then to review each on their merit does > seem a bit daft. All three have to go through at the same time! > > I can see your point, lib765 may hold the other two up (say). Shall I keep as > is, or farm three out and just do this one for libspectrum? My personal preference would be a separate review for each SRPM. I think that is also the intention of the review process. It'll leave behind a separate "audit trail" for each package where some design decisions may be discussed, without being cluttered by discussion about other packages.
Review (libdsk package ONLY): - rpmlint clean, apart from debuginfo package - package naming OK - license is GPL - spec file written in English and is legible - could not check sources because upstream doesn't seem to archive old versions - package builds OK in mock for rawhide (i386) and also on RHEL3 (x86_64) - no locales to worry about - ldconfig called correctly in scriptlets - not relocatable - no directory ownership or permissions issues - no duplicate files - %clean section present and correct - macro usage is fairly consistent - code, not content - no large docs - docs don't affect runtime - no pkgconfig file to worry about - devel files properly included in -devel subpackage - libtool archive excluded properly - no desktop entry needed Needswork: - spec file name should be libdsk.spec, not libdsk-1.1.4-1.spec - there should be no packager: or vendor: tags - use full URL for Source:, e.g. http://www.seasip.demon.co.uk/Unix/LibDsk/libdsk-%{version}.tar.gz - please used standard Fedora Extras buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) - buildreqs sed and perl aren't required as they part of the minimal build environment - see the "Exceptions" list in the Packaging Guidelines page on the wiki - license is "GPL", not "distributable" - "%{__rm} -rf %{buildroot}" should be in %install, not %prep - don't include INSTALL in the docs because it's not relevant to users of the packaged RPM - don't include static libraries without a good reason Notes: - version 1.1.6 is available from upstream - "Applications/System" (same as mtools) would probably be a better choise for the group of the -tools subpackage - if the edit of libtool (hardcode_libdir_flag_spec etc.) is actually necessary (is it?), please add a comment to the spec explaining it - "%{__make} DESTDIR=%{buildroot} install" is preferred to "%makeinstall" when possible - use %{?_smp_mflags} with make when possible - consider moving doc/libdsk* to the -devel subpackage - consider making the dependency of the tools package on the main library package as tight as the -devel package (Requires: %{name} = %{version}-%{release}) or just leave out the explicit dep altogether and let RPM's automatic library dependencies handle it
Created attachment 120506 [details] Patch addressing most libdsk review issues
Review (lib765 package ONLY): - rpmlint nearly clean (see below) - package naming OK - package meets guidelines - license is LGPL, matches spec, text included - spec file written in English and is legible - sources match upstream - package builds OK in mock for FC4 (i386) - BR's OK - no locales or pkgconfigs to worry about - %post and %postun correctly call /sbin/ldconfig - not relocatable - no directory ownership or permissions issues - %clean section present and correct - macro usage is consistent - code, not content - no large docs - docs don't affect runtime - header and .so file properly delegated to -devel subpackage - -devel package has proper base package dependency - libtool archive properly excluded - no desktop entry needed - scriptlets are OK Needswork: * spec file should be called lib765.spec, not lib765-0.3.3-1.spec * Vendor: and Packager: tags should be removed * remove buildroot in %install rather than %prep * add libdsk-devel dep for -devel subpackage; this is because the library provides additional functionality if libdsk-devel is installed, and so libdsk-devel should be added as a dep to maintain consistency of build results Suggestions: * use Fedora Extras standard buildroot * unpack tarball quietly * use DESTDIR with make instead of %makeinstall * don't build or include the static library Notes: * rpmlint output: W: lib765-devel no-provides lib-devel This is due to rpmlint's assumption that "765" is a library version number rather than just a name, and can be ignored.
Created attachment 120945 [details] Patch addressing lib765 review issues
Review (libspectrum package ONLY): - rpmlint clean - package naming OK - package meets guidelines - license is GPL, matches spec, text included - spec file written in English and is legible - source matches upstream - package builds OK in mock on FC4 (i386) - no redundant buildreqs - no locales or pkgconfigs to worry about - %post and %postun scriptlets properly call /sbin/ldconfig - not relocatable - no directory ownership or permissions issues - no duplicate files - %clean section present and correct - macro usage is consistent - code, not content - no large docs - docs don't affect runtime - header and .so files properly delegated to -devel subpackage - -devel package has proper base package dependency - libtool archive properly excluded - no desktop entry needed - scriptlets are OK Needswork: * spec file should be called libspectrum.spec, not libspectrum-0.2.2-2.spec * missing buildreqs bzip2-devel, libgcrypt-devel, glib2-devel * use full URL for source: http://dl.sf.net/fuse-emulator/libspectrum-%{version}.tar.gz * -devel subpackage should have a dependency on libgcrypt-devel because it includes a header file that includes gcrypt.h Suggestions: * use Fedora Extras standard buildroot * unpack tarball quietly * use DESTDIR with make instead of %makeinstall * don't build or include the static library * use %{?_smp_mflags} with make * put the manpage libspectrum.3 in the -devel subpackage
Created attachment 120950 [details] Patch addressing libspectrum review issues
lib765 is the name of the library, not a version number (it's a chipset used as the disc controller on the Speccy +3)
http://www.all-the-johnsons.co.uk/emulation/downloads/lib765.spec http://www.all-the-johnsons.co.uk/emulation/downloads/libspectrum.spec http://www.all-the-johnsons.co.uk/emulation/downloads/libdsk.spec src rpm names libdsk-1.1.6-1.src.rpm lib765-0.3.3-2.src.rpm libspectrum-0.2.2-4.src.rpm All of the changes suggested have now been incorporated and libdsk has been bumped to 1.1.6
(In reply to comment #10) > lib765 is the name of the library, not a version number (it's a chipset used as > the disc controller on the Speccy +3) You and I know that; it's rpmlint that's ignorant ;-) As I said, that one can be ignored. (In reply to comment #11) > http://www.all-the-johnsons.co.uk/emulation/downloads/lib765.spec > http://www.all-the-johnsons.co.uk/emulation/downloads/libspectrum.spec > http://www.all-the-johnsons.co.uk/emulation/downloads/libdsk.spec > > src rpm names > > libdsk-1.1.6-1.src.rpm > lib765-0.3.3-2.src.rpm > libspectrum-0.2.2-4.src.rpm > > All of the changes suggested have now been incorporated and libdsk has been > bumped to 1.1.6 Last couple of points: * lib765 could use %{?_smp_mflags} with make in %build * you misspelled my name in the changelog for libdsk; I'd just remove the changelog entry added by my patch because I forgot to get round to actually listing what the changes were (covered by your new changelog entry). Approved.
I'll make the last couple of changes. According to the thread running the F.E. list, I do have my email address registered and awaiting approval. Any chance of looking into it so I can upload these packages?
http://www.all-the-johnsons.co.uk/emulation/downloads/lib765.spec http://www.all-the-johnsons.co.uk/emulation/downloads/libdsk.spec src rpm names libdsk-1.1.6-1.src.rpm lib765-0.3.3-3.src.rpm Only real change is in lib765 in that it now uses %{?_smp_flags}. libdsk is a rebuild with Paul H's comment removed from the changelog.
(In reply to comment #13) > I'll make the last couple of changes. > > According to the thread running the F.E. list, I do have my email address > registered and awaiting approval. Any chance of looking into it so I can upload > these packages? I still don't know what your account name is in the system (it's only accessible to me by account name and not by email address); I can't see either of the ones you mentioned on-list as being in the cla_done group, nor waiting for approval for the cvsextras group. I think you need to chase that up with Elliot, who has better access to the accounts system.
Ping PFJ; your account seems to be sorted so is there some reason why there's been no activity on this, with the approval being nearly 2 months ago?
D'oh! I'll get it sorted tonight - I've just been snowed under with work!
Package Change Request ====================== Package Name: libspectrum Updated Fedora Owners: packages,paul.uk Package Change Request ====================== Package Name: libdsk Updated Fedora Owners: packages,paul.uk Package Change Request ====================== Package Name: lib765 Updated Fedora Owners: packages,paul.uk Note, the review requests appear to have taken place for all three packages under the same Review bug, so I'm placing all 3 CVS requests here. The current maintainer (paul.uk) appears to be AWOL at the moment due to personal reasons. I was given explicit permission to take over "fuse-emulator" and "fuse-emulator-libs" but lib765, libdsk and libspectrum also need updating in order to maintain the former. The original maintainer should be left as co-maintainer in case he wishes to resume in the future.