Bug 194481

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)
libtcl8.4.so (tcl)
libz.so.1 (zlib)

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.
[mjk@localhost ~]$

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 <robert> 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 @@
-#! /path/to/executable/eggdrop
-# ^- This should contain a fully qualified path to your Eggdrop executable.
+#!/usr/bin/eggdrop
 #
 # $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:

----

Package Release

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. 

eggdrop-1.6.17-4.src.rpm

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.