Bug 171801 - Review Request: libspectrum, lib765 and libdsk - libraries required for the fuse-emulator
Review Request: libspectrum, lib765 and libdsk - libraries required for the f...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
David Lawrence
http://www.all-the-johnsons.co.uk/emu...
:
Depends On:
Blocks: FE-ACCEPT 166960
  Show dependency treegraph
 
Reported: 2005-10-26 13:23 EDT by Paul F. Johnson
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-01-12 21:22:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑cvs+


Attachments (Terms of Use)
Patch addressing most libdsk review issues (3.10 KB, patch)
2005-10-28 11:14 EDT, Paul Howarth
no flags Details | Diff
Patch addressing lib765 review issues (2.06 KB, patch)
2005-11-11 10:19 EST, Paul Howarth
no flags Details | Diff
Patch addressing libspectrum review issues (2.44 KB, patch)
2005-11-11 11:50 EST, Paul Howarth
no flags Details | Diff

  None (edit)
Description Paul F. Johnson 2005-10-26 13:23:28 EDT
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).
Comment 1 Paul Howarth 2005-10-26 13:27:01 EDT
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.
Comment 2 Paul F. Johnson 2005-10-26 13:41:07 EDT
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?
Comment 3 Paul Howarth 2005-10-27 04:49:52 EDT
(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.
Comment 4 Paul Howarth 2005-10-28 11:13:21 EDT
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

Comment 5 Paul Howarth 2005-10-28 11:14:33 EDT
Created attachment 120506 [details]
Patch addressing most libdsk review issues
Comment 6 Paul Howarth 2005-11-11 10:15:30 EST
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.
Comment 7 Paul Howarth 2005-11-11 10:19:20 EST
Created attachment 120945 [details]
Patch addressing lib765 review issues
Comment 8 Paul Howarth 2005-11-11 11:49:36 EST
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
Comment 9 Paul Howarth 2005-11-11 11:50:36 EST
Created attachment 120950 [details]
Patch addressing libspectrum review issues
Comment 10 Paul F. Johnson 2005-11-11 18:28:50 EST
lib765 is the name of the library, not a version number (it's a chipset used as
the disc controller on the Speccy +3)
Comment 11 Paul F. Johnson 2005-11-11 19:03:52 EST
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
Comment 12 Paul Howarth 2005-11-14 07:31:36 EST
(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.
Comment 13 Paul F. Johnson 2005-11-14 09:41:26 EST
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?
Comment 14 Paul F. Johnson 2005-11-14 09:49:38 EST
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.
Comment 15 Paul Howarth 2005-11-14 11:40:22 EST
(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.
Comment 16 Paul Howarth 2006-01-04 11:54:52 EST
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?
Comment 17 Paul F. Johnson 2006-01-04 13:02:38 EST
D'oh!

I'll get it sorted tonight - I've just been snowed under with work!
Comment 18 Ian Chapman 2007-07-02 12:58:25 EDT
Package Change Request
======================
Package Name: libspectrum
Updated Fedora Owners: packages@amiga-hardware.com,paul@all-the-johnsons.co.uk


Package Change Request
======================
Package Name: libdsk
Updated Fedora Owners: packages@amiga-hardware.com,paul@all-the-johnsons.co.uk


Package Change Request
======================
Package Name: lib765
Updated Fedora Owners: packages@amiga-hardware.com,paul@all-the-johnsons.co.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@all-the-johnsons.co.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.

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