Red Hat Bugzilla – Bug 194481
Review Request: eggdrop
Last modified: 2007-11-30 17:11:34 EST
Spec URL: http://labs.linuxnetz.de/bugzilla/eggdrop.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/eggdrop-1.6.17-3.src.rpm
Description: Eggdrop is the world's most popular Open Source IRC bot,
designed for flexibility and ease of use. It is extendable with Tcl
scripts and/or C modules, has support for the big five IRC networks
and is able to form botnets, share partylines and userfiles between
Looks like this came in right before the last backup before the disk crash. I'm
re-adding the CCs I saw added to this bug; I'm quite interested in seeing this
package get into Extras (I maintain a copy on the side).
To summarize what happened (to any roving sponsors), Christopher Stone suggested
a few changes, and Robert applied them.
I suspect you'd be well-off removing the Requires: line, as it's technically
redundant. Rpmbuild should (and, ultimately, is) able to resolve what the end
package's dependencies are. I spun a test built without it and the resultant
package depended on (among others):
libdns.so.21 (which is provided by bind-lib)
Anything else I see...nothing non-trivial. I'd personally change line 37 to "#
Move modules into /usr/lib*" for honesty's sake. ;-)
Not a full review, but then, I'm not a sponsor (sorry).
Thanks, your suggestions were also applied as they are reasonable.
Any non-x86_32 rpmlint outputs (especially of x86_64) would be very interesting
to me, to see whether there are any rpath issues. On x86_32, I've got no rpmlint
outputs any longer...
The binary packages on ppc and sparc have no rpmlint output. I was going to tell
you about the rpmlint output from the debuginfo RPMs, but it looks like you
added src/mod/transfer.mod/*.c to your chmod -x line, which should resolve that.
One step ahead of me on that. :-)
Throwing the current incarnation into my Plague system... *taps foot for a
while* ...done. No rpmlint errors.
Notice that Robert has been sponsored now, so anyone who wants to see this
package into FE can help getting it there be doing a (full) review.
Builds fine on i386 development with mock and its minimal build.
Review for release 3:
* RPM name is OK
* Source eggdrop1.6.17.tar.gz is the same as upstream
* This is the latest version
* Builds fine in mock
* rpmlint looks OK
* File list looks OK
However.. I installed it try and use it, now I might be a complete nub, but I
get this error:
[mjk@localhost ~]$ eggdrop -m eggdrop.conf
[10:52] LANG: No lang files found for section core.
Eggdrop v1.6.17 (C) 1997 Robey Pointer (C) 2004 Eggheads
[10:52] --- Loading eggdrop v1.6.17 (Sun Jun 18 2006)
[10:52] * Please make sure you edit your config file completely.
I have, addlang "English", in my config. so I am not sure if this is a broken
package or a broken config.
OK, you are probably going to need to have a wrapper script for executing
eggdrop itself, as this needs to be done:
[mjk@localhost ~]$ EGG_LANGDIR=/usr/share/eggdrop/language eggdrop
To get eggdrop to start working as expected.
Yepp, you're right. But I'll have a look into this, maybe a default directory
can be specified in the configuration file.
Unfortunately the language path is hardcoded, the issue could be resolved this
way (when accepted). Simultaneously I could open an upstream bug report to get
something like a lang-path directive...
--- eggdrop1.6.17/src/eggdrop.h 2004-07-25 13:17:34.000000000 +0200
+++ eggdrop1.6.17/src/eggdrop.h.rsc 2006-06-18 01:19:23.000000000 +0200
@@ -75,7 +75,7 @@
/* Language stuff */
-#define LANGDIR "./language" /* language file directory */
+#define LANGDIR "/usr/share/eggdrop/language" /* language file directory */
#define BASELANG "english" /* language which always gets loaded before
all other languages. You do not want to
change this. */
I am happy for that patch to be used, it should not create any issues. Not sure
if upstream will go for it, but there is no harm in trying :)
If you want to package it up again, I will do a final review.
Package was updated...
I don't see a release 4 making use of the patch.
I didn't bump version last night, but was updated anyway:
-rw-r--r-- 1 robert fedora 1028579 Jun 18 02:00 eggdrop-1.6.17-3.src.rpm
-rw-r--r-- 1 robert fedora 2344 Jun 18 02:00 eggdrop.spec
But you have not added the patch to make eggdrop function out of the box with
the language path being /usr/share/eggdrop/language.
Sorry Michael, but the patch *was* added to eggdrop-1.6.17-conf.patch...
Contents of Patch0:
Patch by Robert Scheck <firstname.lastname@example.org> for eggdrop >= 1.6.17,
which sets the correct path to the eggdrop executable in the example file.
--- eggdrop1.6.17/eggdrop.conf 2004-08-22 00:43:27.000000000 +0200
+++ eggdrop1.6.17/eggdrop.conf.conf 2006-01-22 00:16:16.000000000 +0100
@@ -1,5 +1,4 @@
-# ^- This should contain a fully qualified path to your Eggdrop executable.
# $Id: eggdrop.conf,v 1.40 2004/08/21 22:43:27 wcc Exp $
This does contain the diff as you showed in comment 8.
You must be joking - verify the md5sums, please. And now I did only for you a
rebuild and ensured that there *is* the whole patch: eggdrop-1.6.17-3.1.src.rpm
Robert. Firstly, I would not "joke" like that in a review. in release 3, the
patch is incorrect.
Secondly, you should review the package naming guidelines, as it clearly states
in there this:
In the past, Fedora.us used 0.fdr as a release prefix to identify Fedora.us
packages. In Fedora, this repository "tagging" is unnecessary, and should not be
used. The release number (referred to in some older documentation as a "vepoch")
is how the maintainer marks build revisions, starting from 1. When a minor
change (spec file changed, patch added/removed) occurs, or a package is rebuilt
to use newer headers or libraries, the release number should be incremented. If
a major change (new version of the software being packaged) occurs, the version
number should be changed to reflect the new software version, and the release
number should be reset to 1.
Following this practise is required when a package is accepted in to Extras, a
review should be considered no different.
Thirdly, you have not updated the changelog in the SPEC to reflect that you have
made a patch to the source. Accurate changelogs are important.
Lastly, the patch file's name incorrectly represents the change now that you
have added a source code patch to it.
My suggestion is that you make a eggdrop-1.6.17-4.src.rpm, with a changelog
entry reflecting the new patch and you rename the patch so it does not mislead
other people doing a quick overview as to what the patch is doing.
Thank you. You can consider this APPROVED.
Please close this bug report with NEXTRELEASE once imported into CVS.
11618 (eggdrop): Build on target fedora-development-extras succeeded.
11617 (eggdrop): Build on target fedora-5-extras succeeded.
11616 (eggdrop): Build on target fedora-4-extras succeeded.
as per http://fedoraproject.org/wiki/Extras/Contributors I'll close this
bug report with NEXTRELEASE now.