Bug 428435 (shezhu) - Review Request: shezhu - Shezhu Resource Sharing System
Summary: Review Request: shezhu - Shezhu Resource Sharing System
Keywords:
Status: CLOSED NOTABUG
Alias: shezhu
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2008-01-11 15:01 UTC by Tim Colles
Modified: 2008-10-27 16:53 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-27 16:53:10 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Tim Colles 2008-01-11 15:01:02 UTC
Spec URL: http://homepages.inf.ed.ac.uk/timc/repos/fedora8/SPECS/shezhu.spec
SRPM URL: http://homepages.inf.ed.ac.uk/timc/repos/fedora8/SRPMS/shezhu-1.19-1.fc8.src.rpm
Description: The Shezhu Resource Sharing System (pronounced Says-Who) is an application for scheduling and booking shared resources such as rooms and equipment. Note that this is my second package (for review) and I am seeking a sponsor.

Comment 1 Alec Leamas 2008-01-11 18:45:13 UTC
This is not a review, just a rookie skimming  through the spec file...

Style: CAPITAL letters in Summary: and Description looks odd.
%files: You should probably use %{_var} instead of /var
%files: /usr/lib should probably be %{_libdir}, taking possible 64-bit 
issues into account.
but then also the Makefile must do the Right Thing.
%files: Use %{_sbindir} instead of /usr/sbin IMHO.
Is it important that the file(s) installed under /usr/sbin is owned by
apache/apache? Don't know if/where this is regulated, but normally executables
are owned by root. Since you seem to install with default 755 permission, I
don't get the point... OTOH, I'm used to that feeling ;-)

BTW. this also introduces a dependency on the 'apache' user... How is that handled?

Comment 2 Tim Colles 2008-01-15 15:26:31 UTC
> Style: CAPITAL letters in Summary: and Description looks odd.
Can change this easily enough.

Regards using %{_var|_libdir|_sbindir}. This is trickier since they would then
need to be embedded/referenced in the makefile and other configuration data of
the source itself - I could however hardcode a branch of the specfile and source
configuration just for FC8. Will look into doing this.

You are correct about ownership of /usr/sbin files - there is no need for them
to be owned by apache:apache - it was just that way for simplicity. Will change
this.

I believe that "apache" is a standard user in /etc/passwd (owned by the "setup"
rpm) however that is not listed as one of the exceptions for dependencies so I
guess "setup" ought to be explicitly listed as a requirement?

Comment 3 Alec Leamas 2008-01-19 14:55:13 UTC
There is some boilerplate in wiki/Packaging/UsersAndGroups which resolve the
dependency on apache user/group if needed.

I don't really think you have to reference the various rpm macros in the
Makefile(s), this is not how other packages handles it. The only connection
normally is in 'make DESTDIR=%someshere'. Besides this, the Makefile just
installs, and if anything goes wrong it's detected by the check for unpacked files.

Comment 4 Tim Colles 2008-01-31 17:24:20 UTC
Thanks. Got a new version that fixes the users and groups (using the
boilerplate), maps paths properly to %{_libdir}, %{_var} etc., also fixes the
ownership of files in /usr/sbin and capital letters. Will make SRPM and SPEC
available here in the next few days - will post comment with new link.

Comment 5 Tim Colles 2008-02-01 12:27:11 UTC
Updated review versions now available at:

Spec URL: http://homepages.inf.ed.ac.uk/timc/repos/fedora8/SPECS/shezhu.spec
SRPM URL:
http://homepages.inf.ed.ac.uk/timc/repos/fedora8/SRPMS/shezhu-1.20-1.src.rpm

Comment 6 manuel wolfshant 2008-07-06 14:25:23 UTC
Tim, are you still interested in this bug? 'Cause if you do, there are a couple
of fixes needed. Most important one being the fact that
- you claim the package is noarch but
- in %files you use %{_libdir}/shezhu which leads to the following error:
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/var/tmp/shezhu-1.20-1.fc10-OFXMjR
error: Installed (but unpackaged) file(s) found:
   /usr/lib/shezhu/Makefile
   /usr/lib/shezhu/README.loc
   /usr/lib/shezhu/access.loc
   /usr/lib/shezhu/action.js
[...] (very long list of unpackaged stuff here)

On a different line, I am not sure how important is this fragment from the end
of the build process:
TXT="MIDI" BOX=48x14 STR=23x8 ASC=8 DES=0 LFT=12 TOP=3 OUT=base-images/MIDI.gif
TXT="MINI" BOX=48x14 STR=23x8 ASC=8 DES=0 LFT=12 TOP=3 OUT=base-images/MINI.gif
TXT="LCDP" BOX=48x14 STR=29x8 ASC=8 DES=0 LFT=9 TOP=3 OUT=base-images/LCDP.gif
Error: string width bigger than bounding box, 84 > -3
(I usually tend to avoid "error"s)

In terms of the spec file
- you should also use add "-q" to setup, in order to silence it 
- remove one of the two occurences of "perl" in the BR line.
- what's the significance of the following lines under %description:
Generic (Build 2008.02.01.12.24)
Supports dates from 01/02/2008 to 31/01/2009 and expires on 01/02/2009
- last but not least, the web page of the project claims that Image::Magick
would be needed, but the spec does not require / use it.


Comment 7 Tim Colles 2008-07-25 13:30:42 UTC
Hi. Thanks very much for your suggestions. Haven't had time recently to work on
this as what needs to be done is going to require some changes to the upstream
source. However, will be getting back to this shortly.

Comment 8 Jason Tibbitts 2008-10-21 04:20:27 UTC
Any updates?  It's been almost another three months now.

Comment 9 Jason Tibbitts 2008-10-27 16:53:10 UTC
And its been another week; I guess I'll close this.


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