Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 236642 - Review Request: Revisor - Revisor GUI
Summary: Review Request: Revisor - Revisor GUI
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
Depends On:
TreeView+ depends on / blocked
Reported: 2007-04-16 22:05 UTC by Jonathan Steffan
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2007-06-02 05:13:45 UTC
tibbs: fedora-review+
tibbs: fedora-cvs+

Attachments (Terms of Use)
Updated Spec with review inspired corrections (deleted)
2007-04-18 08:27 UTC, Jef Spaleta
no flags Details

Description Jonathan Steffan 2007-04-16 22:05:30 UTC
Spec URL:
Description: Fedora "Spin" Graphical User Interface

Comment 1 Jeroen van Meeuwen 2007-04-16 22:10:56 UTC
Adding myself to CC

Comment 2 Jef Spaleta 2007-04-18 08:25:30 UTC
Okay initial review comments.  Attached you will find an updated specfile which
fixes some simple problems in the specfile and the important rpmlint error
message concerning consolehelper symlink.
Fixes in updated specfile:

- remove makeinstall macro
- add usermode and pam requires
- make symlink to consolehelper relative path instead of absolute path
- removed desktop-update-database scriptlets and associated deps
  Refer to

One thing I can't easily fix is the SOURCE0 tag.
Please refer to
and follow the directions for Using Revision Control to add the appropriate
comment blog which gives the necessary instructions to rebuild the source
tarball locally. The goal is to provide the steps necessary for me to rebuild
the tarball so that I can do the appropriate md5sum check for the tarball you
have included.

Please regenerate a new spec and srpm with these corrections and I should be
able to run through the full formal review.


Comment 3 Jef Spaleta 2007-04-18 08:27:37 UTC
Created attachment 152880 [details]
Updated Spec with review inspired corrections

This is an updated spec which includes corrections found during submission

Comment 4 Jeroen van Meeuwen 2007-04-18 10:11:29 UTC
Thank you for taking the time to review our package.

Updated Source RPM:

Updated Spec file:

Comment 5 Jef Spaleta 2007-04-19 06:06:05 UTC
You really really really need to do something about the SOURCE tag as mentioned
in comment #2. 

Since the source tarball is being generated from a source control system (git in
this case if im reading the revisor development webpages correctly) and its not
available at an established static URL address as an identifiable 'release' then
the spec file must contain a comment blog which details the actual steps that
need to be taken to pull the appropriated tagged git entity and then repackage
it as a tarball. has an example of how to do a
comment block for a public svn server. You should do something similar for your
git tree.

I cannot initiate a formal review if I can't confirm that the md5sum of the
tarball is as it should be.


Comment 6 Jeroen van Meeuwen 2007-04-19 07:59:50 UTC
I'm sorry, obviously you are right. I didn't regenerate the .spec after editting

I've updated the .spec file on the location mentioned above.

Comment 7 Jeroen van Meeuwen 2007-04-19 08:11:50 UTC
No I've not updated, for some reason it won't update.

New location is

Thank you Jef, for your patience ;-)

Comment 8 Jef Spaleta 2007-04-20 06:34:58 UTC
hmm im getting a name resolution failure for at the
moment.  Is the result of a problem on my end or is the machine down currently?


Comment 9 Jonathan Steffan 2007-04-20 06:47:32 UTC
Yeah, the VM has been moved to another host due to Bob needing to use the
physical machine for another purpose (to replace a server having hardware issues
for $work) and as a side effect the DNS servers are down at the moment.
Minimally we are going to add my primary DNS server as a slave to help prevent
this in the future but we are also evaluating just moving the plone instance to
my dedicated zope server. Sorry Jef, it'll be back tomorrow I assume.

Comment 10 Jef Spaleta 2007-04-22 19:52:57 UTC
still isn't resolving correctly. that URL gets redirected to index.html for the
fedoraunity site.

Please provide an updated url for both the current srpm and spec.


Comment 11 Jeroen van Meeuwen 2007-04-23 08:41:24 UTC
We're sorry for the inconvenience, backups were trashed.

Files are up now;

Comment 12 Jef Spaleta 2007-04-26 08:27:38 UTC
Okay the SOURCE0 tag still isn't good enough.
I need a valid url at which I can find the revisor-2.0.1.tar.gz tarball
I need a comment block in the specfile which tells me how to generate the
revisor-2.0.1.tar.gz using git commands.


Comment 13 Jeroen van Meeuwen 2007-04-26 09:05:57 UTC
Source0 tag is now:

I'm not sure I understand what is wrong here, because that URL gives you the
proper tarball, located at

Comment 14 Jef Spaleta 2007-04-26 18:41:16 UTC
oops sorry, I had a stale srpm here that i was looking at.
The new SOURCE0 looks fine I should be able to start the formal review.

One thing, you should consider changing how the consolehelper symlink is created
in the release tarball for the next release, so we can avoid the hack to fix it
in the specfile.


Comment 15 Jef Spaleta 2007-04-26 18:47:53 UTC
Problem:  md5sum from SOURCE0 url and included sources do not match.  Please
doublecheck that you are including the tarball as listed from the SOURCE0 location.

Here's my local actions for md5sum test

>rpm -ivh revisor-2.0.1-3.fc7.src.rpm

>md5sum rpmbuild/SOURCES/revisor-2.0.1.tar.gz
1ae8d73266079f93e3d543631ce59704  rpmbuild/SOURCES/revisor-2.0.1.tar.gz

>spectool rpmbuild/SPECS/revisor.spec

>md5sum revisor-2.0.1.tar.gz
6acf0e69808e014683607ab9628b5586  revisor-2.0.1.tar.gz


Comment 17 Jef Spaleta 2007-05-04 07:50:14 UTC
+ naming is good
+ specfile name matches base package name 
+ specfile written in english-ese and is legible
+ included source md5sum checks with upstream source as listed in SOURCE0 url
9b1bb4207f9c8a64609d1007420955ef  revisor-2.0.1.tar.gz
+ builds on x86 fedora-development in mock
+ no locales
+ not relocatable
+ clean section is okay
+ consistent use of macros
+ permissible code and content
+ items in doc are not runtime necessary
+ does not obviously own files from another package 
+ directory ownership of parent directories is accounted for in package deps.
+ No .la files
+ No devel subpackage
+ Need need to for shared libraries sciptlets
+ no need for scriptlets.

- Licensed as GPL but COPYING file NOT included in docs!!

- install section has a problem
needs to include a desktop-file-install stanza to install the desktop file
correctly, and require desktop-file-utils Refer to

? permissions on
Is meant to be run stand-alone as an executable. You should either
strip the 
shell invocation at the top of the file or make it executable.

The Suggestions:
You can remove pam from the requires list, usermode requires pam. I know I know
i suggested it originally, based on just the directory ownership crap. But
taking a closer look the pam requires is redundant.

I haven't actually used this yet. Is there a simple example walkthrough on
usage? Like how to make a stupidly simple livedvd image or something, so I dont
have to think about the package selection but I can test the wizard interface.

rpmlint revisor-2.0.1-4.fc7.noarch.rpm
E: revisor non-executable-script
/usr/lib/python2.5/site-packages/revisor/ 0644
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-ppc.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-f7-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-ppc.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/pungi-fc6-x86_64.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-x64_86.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-fc6-i386.conf
W: revisor non-conffile-in-etc /etc/revisor/conf.d/sample-ks.cfg
W: revisor non-conffile-in-etc /etc/revisor/revisor.conf
W: revisor non-conffile-in-etc /etc/security/console.apps/revisor
W: revisor non-conffile-in-etc /etc/revisor/conf.d/revisor-f7-i386.conf
W: revisor non-conffile-in-etc /etc/pam.d/revisor

-- warnings are bogus, might want to patch the to either be
executable or to strip
the intepreter from the first line.

rpmlint revisor-2.0.1-4.fc7.src.rpm
W: revisor strange-permission revisor.spec 0600
-- not important, but you might consider making it world and group readable by

Comment 18 Jeroen van Meeuwen 2007-05-10 20:43:20 UTC

The only error I get with rpmlint is on the SRPM which still reports
strange-permission (I don't know how to solve that)

Comment 19 Max Spevack 2007-05-22 20:06:43 UTC
Can we PLEASE do whatever it takes to get this thing through review and into the
Fedora repo BEFORE the F7 release? (meaning, ASAP)

People are going to be looking for it.  Let's make sure they can have it.  We've
spent lots of time and energy building it and promoting the hell out of it.  It
would be a shame if people can't find and use it.

We're going to have a feature article in RH Magazine that talks about Revisor,
and we made a big deal about it in the press interviews that are ongoing and all
F7 release material.

What's the biggest hold up?  I assume between Jef, Steffan, and Jeroen, we can
get this thing done.  You three have enough combined brainpower to make Zod
himself nervous.

Comment 20 Jason Tibbitts 2007-05-22 20:20:52 UTC
All that's necessary is for Jef to verify that the issues he saw have been fixed
and give his approval.  Then the process of getting it branched, checked in and
built can happen.

I'm happy to tale a look if Jef is on vacation or something.

Comment 21 Jeroen van Meeuwen 2007-05-22 21:24:50 UTC
As I understand it Mr. Tibbitts is going to take Revisor to a nice Indian
restaurant, only 6 blocks from here, so I'm gonna stand-by online and wait for
the ping ;-)

Thank you Mr. Tibbitts!

Comment 22 Jason Tibbitts 2007-05-23 00:41:53 UTC
OK, assigning to me with visions of real, hot vindaloo (as opposed to "painful
for a Yankee, boring for a Texan" Boston-style vindaloo).

I'm going to build on top of Jef's review here, check the issues he raised and
trust him for the rest.  Just let me run this through mock....

Comment 23 Jason Tibbitts 2007-05-23 01:36:06 UTC
OK, that didn't take long.

You're right, rpmlint is now silent except for:

W: revisor strange-permission revisor.spec 0600

which I don't think is a blocker, but should be easily fixed by just "chmod 644
revisor.spec".  It shouldn't make much difference once it gets checked into CVS

There is a copy of the license in the tarball; that needs to be included in the
final package as %doc.

As Jef pointed out above, I see no mention of desktop-file-install; you'll need
to add BR: desktop-file-utils and then something like this in %install:

desktop-file-install --vendor="fedora"              \
  --delete-original                                 \
  --dir=%{buildroot}%{_datadir}/applications        \

The package installs fine on a handy rawhide system, shows up in the KDE menu
and starts without problems.  I note, though, that the menu entries do nothing
for me, and if I select "Rescue Image" as the type of media to compose, I get an
error dialog telling me "No media selected".  I'd wager that these are not
issues related to packaging and so aren't blockers, but you may know better than I.

Comment 24 Max Spevack 2007-05-23 03:18:37 UTC
Thanks Jason.  Looks like we should be able to get this thing into CVS pretty
easily at this point, which is going to be great.

All you, Jeroen!

Comment 25 Jonathan Steffan 2007-05-23 18:23:17 UTC
For a note about "W: revisor strange-permission revisor.spec 0600":

revisor.spec is generated by autmake. If there is anyone that knows how to fix
this, we are more then willing to fix it. I just have not figured out how.

Comment 26 Jonathan Steffan 2007-05-23 18:24:12 UTC

Comment 27 Jason Tibbitts 2007-05-23 18:51:56 UTC
rpmlint is complaining about the spec that's in the SRPM, not anything generated
by the build process, but I guess you could be building the SRPM with rpmbuild
-ts on the tarball.  To add to oddness, though, the spec in the tarball has mode

In any case, it's really not a big deal.  The other issues are much more
interesting, but should also be much easier to fix.

Comment 28 Jason Tibbitts 2007-05-25 03:02:29 UTC
I received an updates SRPM via IRC.  Here's a quick recap of the IRC discussion
for the record:

The desktop file gets installed properly, although desktop-file-install does
emit a warning:

warning: The 'Application' category is not defined by the desktop entry
specification.  Please use one of "AudioVideo", "Audio", "Video", "Development",
"Education", "Game", "Graphics", "Network", "Office", "Settings", "System",
"Utility" instead

I don't think this is a big deal, although it would be good to fix at some
point.  Either "Development" or "System" seem OK to me.

The COPYING file still doesn't get included in the package.  All you need to do
is change:
in your %files section.

That should be it.

Comment 29 Jason Tibbitts 2007-05-25 04:10:46 UTC
OK, I've received an updated package via IRC.

rpmlint is silent except for the aforementioned strange-permission warning on
the spec which isn't a blocker.

The desktop-file-install issue is fixed.

COPYING is included in the package.

So we're done.


Comment 30 Jonathan Steffan 2007-05-25 06:00:31 UTC
New Package CVS Request
Package Name: revisor
Short Description: Fedora "Spin" Graphical User Interface
Branches: F-7

Comment 31 Tom "spot" Callaway 2007-05-25 20:24:36 UTC
cvs done.

Comment 32 Jason Tibbitts 2007-06-01 05:39:12 UTC
Please remember to close this bug once the package has been built, thanks.  (My
open ticket list is getting long.)

Comment 33 Jeroen van Meeuwen 2007-06-16 16:48:23 UTC
Package Change Request
Package Name: revisor
Updated Fedora Owners:,
Updated Fedora CC:,

Comment 34 Jonathan Steffan 2007-06-16 21:14:35 UTC
Yes, this should be done.

Comment 35 Jason Tibbitts 2007-06-18 16:40:40 UTC
Co-maintainer added.

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