Spec URL: http://home.bawue.de/~ixs/ngircd/ngircd.spec SRPM URL: http://home.bawue.de/~ixs/ngircd/ngircd-0.10.1-2.src.rpm Description: ngIRCd is a free open source daemon for Internet Relay Chat (IRC), developed under the GNU General Public License (GPL). It's written from scratch and is not based upon the original IRCd like many others.
Is this possibly a duplicate of: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228955 ?
Uhh, right. I missed that one. Okay, it's a dupe. I'll talk with Sean about it. His package doesn't look as polished though.
*** Bug 228955 has been marked as a duplicate of this bug. ***
If no one else is interested in reviewing, I'll try to take a look tomorrow. (I notice libident is still waiting approval, so you'd have to fix that up first).
Noone else seems to want to take a look, so assigning to myself. I'll have a review based on the current status of libident's review tomorrow, but until libident is accepted I can't give a definate approval yet. Sound okay Andreas?
If it is any consolidation, I'll just remove the libident requirement. I honestly do not care.
In the end it's up to you, but I feel (as a seasoned IRC user) that libident would be useful in a package such as ngircd. One possible work around is: provide a .spec file for ngircd without the libident requirement, once ngircd and libident is approved, readd the dependency, (after lots of checking, assuming all the static linking issues can be sorted).
Okay, new package without libident at the usual location. have fun. :>
Created attachment 153508 [details] update to the writebuf patch
Mock Log: Installing /builddir/build/SRPMS/ngircd-0.10.1-3.fc7.src.rpm Building target platforms: x86_64 Building for target x86_64 Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.97021 + umask 022 + cd /builddir/build/BUILD + LANG=C + export LANG + unset DISPLAY + cd /builddir/build/BUILD + rm -rf ngircd-0.10.1 + /bin/gzip -dc /builddir/build/SOURCES/ngircd-0.10.1.tar.gz + tar -xf - + STATUS=0 + '[' 0 -ne 0 ']' + cd ngircd-0.10.1 ++ /usr/bin/id -u + '[' 509 = 0 ']' ++ /usr/bin/id -u + '[' 509 = 0 ']' + /bin/chmod -Rf a+rX,u+w,g-w,o-w . + echo 'Patch #0 (ngircd-writebuf-stupidity.diff):' Patch #0 (ngircd-writebuf-stupidity.diff): + patch -p1 -s missing header for unified diff at line 13 of patch The text leading up to this was: -------------------------- |ngircds ancient WRITEBUFFER_LEN legacy crap actively sabotages |server connections (where a lot of stuff has to be sent at once) | |Work around this stupidity by tolerating pending writes of up to 512kb. | |Index: conn.c |=================================================================== |RCS file: /srv/cvs/ngircd/ngircd/src/ngircd/conn.c,v |retrieving revision 1.198.2.2 |diff -u -r1.198.2.2 conn.c |--- conn.c 17 Dec 2006 23:06:29 -0000 1.198.2.2 |+++ conn.c 3 Apr 2007 19:34:00 -0000 -------------------------- File to patch: Skip this patch? [y] 1 out of 1 hunk ignored error: Bad exit status from /var/tmp/rpm-tmp.97021 (%prep) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.97021 (%prep) The patch file does not work because of the additional text in the header, I have updated the patch so that it should apply correctly. (See Comment #9)
Okay, here is the state of things: I've done a rough review at the moment, but it the spec file needs some changes first: 1. The diff needs replacing (See Comment #9 and Comment #10) 2. The dependency on tcp_wrappers is correct for fc6, but not f7 (devel), the development libraries got moved to tcp_wrappers-devel. I'm not totally sure how other reviewers have counteracted this in the past, but my suggestion is for review change the builddep to: /usr/include/tcpd.h this will counteract the problem, the problem here is it puts increased demands on yum, so when you commit to CVS you will need to alter the BR's to specific packages (tcp_wrappers for FC-5 & 6, tcp_wrappers-devel for F7/devel) (I have confirmed that this workaround works under mock okay) Once the two changes are made, I should be able to build it fine without altering the src.rpm myself, and should be able to give it a review and test it.
Thanks, I'll change the patch accordingly. About the tcp_wrappers requirement, the correct way of solving this is left up to the developer. Adding a file-dependency is what I do not want to do. I might either have a different specfile for the devel tree or I'd use conditionals like "%{fedora}" < "7" or something similar. I haven't decided yet, but I guess I'll go with the former.
(In reply to comment #12) > Thanks, I'll change the patch accordingly. > > About the tcp_wrappers requirement, the correct way of solving this is left up > to the developer. I agree, I was just suggesting one way. > Adding a file-dependency is what I do not want to do. I don't like file based dependencies myself to tell the truth. > The reason why I suggested a file dependency for review only, is because once reviewed, the spec file in FC6 can BR on tcp_wrappers and devel can BR on tcp_wrappers-devel Just for the sake of been complete with suggestions, the another appropriate way is: As mentioned with the dependency altered to -devel, it can be built in RAWHIDE obviously your using a FC-6 machine when creating packages (like myself), so another method to settle the problem is to change the spec file with rawhide compatible dependencies, and when testing on FC-6 use --nodeps. Although not specified in the PackageReviewGuidelines I'm pretty sure that most reviews agree that packages must be buildable on the current devel at the very least. The first solution I've provided will allow you build on both FC-6 and devel while under review, the second will make the package only buildable on devel during review, or with --nodeps on FC-6 As an after thought, depending on changes to the spec file, I know that it will build on rawhide, and can approve a FC-6 compat spec file. > I might either have a different specfile for the devel tree or I'd use > conditionals like "%{fedora}" < "7" or something similar. > I haven't decided yet, but I guess I'll go with the former. I'd be happy to review a devel spec file, and a FC-6 spec file as another option I've leave the choice up to you, but just remember, any workarounds for review will be changed by the time it enters cvs.
Package uses obsolete networking functions. Please update it to use the current networking API. See the 'Example Server Code' section at http://people.redhat.com/drepper/userapi-ipv6.html
(In reply to comment #12) > I might either have a different specfile for the devel tree or I'd use > conditionals like "%{fedora}" < "7" or something similar. > I haven't decided yet, but I guess I'll go with the former. This will actually be the best way for the review. (In reply to comment #14) > Package uses obsolete networking functions. Please update it to use the current > networking API. > > See the 'Example Server Code' section at > http://people.redhat.com/drepper/userapi-ipv6.html > Quite important in todays 'IPv6' Fedora, I've actually discussed this issue with David and I think it is something that should be concerntrated on. I've noticed that upstream have neither said yay or nay regarding IPv6, so it might be an idea to create a patch for this, and send upstream.
Ping? Is there any update regarding the status of ngircd?
(In reply to comment #16) > Ping? > > Is there any update regarding the status of ngircd? Ping #2, I havn't heard anything for the last 6 weeks or so, regarding this bug, can you respond please. I'm not too keen on closing this as a dead review per http://fedoraproject.org/wiki/Extras/Policy/StalledReviews, so I would appreciate any form or response.
Package name: PASS (ngircd) License: PASS (GPL) Spec Legible: PASS (en_US) md5sum matches: PASS rpmlint clean: NOTES Builds correctly: PASS (i386) RPaths removed: PASS Spec has %clean: PASS Macro use consistant: PASS Contains code/content: PASS -doc subpackage: NA -devel subpackage: NA -static subpackage: NA pkgconfig depend: NA Contains %doc: PASS Library suffix: NA No .la files: NA Use desktop-file-install: NA No duplicate ownerships: PASS rm -rf %{buildroot}: PASS RPM uses valid UTF-8: PASS %defattr is set: PASS No duplicate %files: PASS Not relocatable: PASS Calls ldconfig: PASS Supports Locales: NA BR's are correct: NOTES I believe the tcp_wrappers issue still exists, so does the patch issue (please fix at some point) E: ngircd non-standard-gid /var/run/ngircd ngircd E: ngircd non-standard-dir-perm /var/run/ngircd 0775 E: ngircd non-standard-gid /etc/ngircd.motd ngircd E: ngircd non-readable /etc/ngircd.motd 0660 E: ngircd non-standard-gid /etc/ngircd.conf ngircd E: ngircd non-readable /etc/ngircd.conf 0660 E: ngircd use-tmp-in-%pre non-standard/non-readable messages can be ignored, use-tmp-in-%pre appears to be a bug. The IPv6 stuff isn't actually policy like I was led to believe, sorry for holding it up though. (Although you might want to file a bug blocking against bug 195271 if you want). Approved
Ohhkay, this completely fell under the table. Thanks for the approval, I'm pushing a current package into CVS now.
New Package CVS Request ======================= Package Name: ngircd Short Description: Next Generation IRC Daemon Owners: ixs Branches: F-7 F-8 EPEL-5 EPEL-4 Cvsextras Commits: no
cvs done. Note that there was a security alert in the last few days that all versions except the current one are vulnerable to. Make sure you import the latest upstream version.
(In reply to comment #21) > cvs done. Thx. > Note that there was a security alert in the last few days that all versions > except the current one are vulnerable to. Make sure you import the latest > upstream version. I know, the main coder is nearly (sitting) next to me. :)
Andreas, note that there is yet another one: CVE-2008-0285 (Bugzilla bug #428935). So be sure to fix also that one. Thanks!
checked in and build 0.11.0, fixing CVE-2008-0285. thx for the review.