Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at

Bug 212894 (libopm)

Summary: Review Request: libopm - Blitzed open proxy monitor library
Product: [Fedora] Fedora Reporter: Robert Scheck <redhat-bugzilla>
Component: Package ReviewAssignee: Christopher Stone <chris.stone>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhide   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-11-08 21:54:52 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 163779    

Description Robert Scheck 2006-10-30 00:45:08 UTC
Spec URL:
Description: An open proxy detection library, developed by the blitzed IRC 
network team. Its original use was to detect open proxies running on clients 
connecting to various IRC servers, but it has evolved to become a generic
open proxy detection library.

libopm has no version number, because it never got released and there isn't any 
plan to do so, when I talked with libopm upstream earlier this year. And as you 
can see from the changes in the libopm repository, there are only very rarely 
minor changes, so using the date as version number should fit (like practised at 
bash-completion, dumpasn1, geda, gnubg, mbuffer and further packages in Fedora 

Comment 1 Christopher Stone 2006-10-31 03:02:12 UTC
Some initial comments before I can do a formal review:

- Source0 should list the full download URL and use %{name}
- make should use $RPM_OPT_FLAGS
- use %defattr(-,root,root,-)
- You need to mention in a comment how to download the source if there is not a
direct URL
- I think the group should be Development/Libraries for the main package as
well, although I cannot find anything which specifies how the groups are defined
- You should use --disable-static on the %configure
- I think the version number should be done like so:

Version: 0.0.0
Release: 0.2.20050731cvs%{dist}


Comment 2 Robert Scheck 2006-10-31 13:10:12 UTC
> - Source0 should list the full download URL and use %{name}
> - You need to mention in a comment how to download the source if there is
    not a direct URL

Fixed; you're right of course.

> - make should use $RPM_OPT_FLAGS

Hum? gcc -DHAVE_CONFIG_H -I. -I. -I. -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 
-fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -
mtune=generic -fasynchronous-unwind-tables -c config.c -MT config.lo -MD -MP -MF
 .deps/config.TPlo  -fPIC -DPIC -o .libs/config.lo -- IMHO $RPM_OPT_FLAGS seems 
to be used. What should be wrong here?

> - use %defattr(-,root,root,-)

I'm using %defattr(-,root,root) which fits and does the same. I wasn't able to 
find sth else e.g. at 
(which says, that %defattr(...) must be used).

> - I think the group should be Development/Libraries for the main package as
>   well, although I cannot find anything which specifies how the groups are 
>   defined

Why? libtiff, libjpeg, libidn, libselinux, libesmtp, librsync are using the same 
like mine. Why should System Environment/Libraries be wrong? It decribes simply 
perfect the main package of libopm - IMHO.

> - You should use --disable-static on the %configure

Hum? MUST: Header files or static libraries must be in a -devel package. This is 
what I'm following (

> - I think the version number should be done like so:

Looks whether you're right. But I found AC_INIT() in, which tells 
version 0.1, So it would be the following, right? IMO it must be a post release 
of 0.1, because 0.1 was always set when browsing through viewcvs of Blitzed.

Version: 0.1
Release: 2.20050731cvs%{dist}

I'll push a new build when all things are clarified.

Comment 3 Christopher Stone 2006-11-02 17:26:12 UTC
-For the RPM_OPT_FLAGS, I did not check the actual flags being used.  I simply
noted it because I did not see it explicitly mentioned in the spec file.  I will
take a closer look at this issue when I do the formal review.  You can leave it
out for now if you think it's not needed.

- I suggested you use %defattr(-,root,root,-) because all of the examples given
use this.  It probably doesn't matter much and I wont block the review because
of it, but I don't see any harm in adding it either.


- The group doesn't matter to me, I can't find anything anywhere defining the
groups so you can leave it as is if you like.

- I suggested you use --disable-static because of this:

Which states that static libraries should be disable whenever possible.  So
unless you got a *really* good reason to keep the static library, then I will
allow you to keep it in the package, but you must add a comment in the spec file
explaining the *really* good reason for you to keep it.  Otherwise this is a
blocker and the static library must be removed before I can approve it.

- For the version number, I suggested 0.0.0 because this was the version number
use in the .so filename.  However, if you want to use 0.1, that is fine too. :)

Comment 4 Robert Scheck 2006-11-02 19:25:06 UTC
Okay, hopefully fixed everything; new:

Spec URL:

Comment 5 Christopher Stone 2006-11-05 21:50:12 UTC
======== REVIEW CHECKLIST ========
- rpmlint output clean
- package named according to package naming guidelines
- spec filename matches package name
- package meets packaging guidelines
- package licensed with opensource compatible license
- license in spec matches actual license
- license in %doc
- spec file written in American English
- spec file is legible
- package successfully compiles and builds on FC5 x86_64
- no build dependencies required
- no locales required
- %post and %postun properly run ldconfig
- package is not relocatable
- package owns all directories it creates
- no duplicate files in %files
- permissions are set properly
- spec file contains proper %clean section
- macro usage is consistent
- package contains code
- no large documentation files
- files in %doc do not affect runtime
- header files are in devel subpackage
- no pkgconfig used
- .so file in devel
- devel package requires base package
- package does not contain .la file
- not a GUI app, no .desktop file needed
- package does not own files or directories owned by other pacakges

======== MUST FIX ========
  - 5537327b4e76bd2c5074996b16cad361  ../SOURCES/libopm-0.1.tar.gz
  - c3da807c0d90cc89c301d83ac5669238  cvs/libopm-0.1.tar.gz
  - Please provide specific tar instruction used, I don't want to check the
md5sum of each source file individually.  I believe the difference is caused by
the order of the files in the tarball.  The command I used to tar the source
files was: tar czf libopm-0.1.tar.gz libopm-0.1/
  - In addition, your cvs co command should include a specific date so that if
something is added to the cvs in the future, someone can still pull the exact
sources used in this package.

Comment 6 Robert Scheck 2006-11-05 22:32:28 UTC
Adding date to the checkout command should be no problem (try `cvs -z3 -d: co -D "20050731 23:59" libopm`), but how to 
change the ordering of the files in the tarball I absolutely don't know. All
I did was a "tar cvfz libopm-0.1.tar.gz libopm-0.1" as non-root user. But is 
this really important? 

[22:56:14] < mitr> rsc: Perhaps (tar czf foo.tar.gz $(find libopm-0.1 | LC_ALL=C 
sort)) will make the file order consistent - but I don't know about other 
attributes of the tarball, e. g. owner&group.
[22:56:30] < rsc> hm.
[22:57:21] < mitr> rsc: IMHO it should be enough to compare a checkout and the 
tarball by preparing them and using (diff -urN)
[22:57:23] < ensc> what's the sense of the md5sum check?
[22:57:59] < ensc> packager can rpm-import everything what he wants

Finally the suggested command adds every file twice here...

Comment 7 Christopher Stone 2006-11-06 05:09:14 UTC
$ diff -urN libopm-0.1 ../rpmbuild/SOURCES/libopm-0.1
Output clean.

Please add -D "20050731 23:59" to the comments after checking into CVS.


Comment 8 Robert Scheck 2006-11-08 21:54:52 UTC
21226 (libopm): Build on target fedora-development-extras succeeded.
21227 (libopm): Build on target fedora-6-extras succeeded.
21228 (libopm): Build on target fedora-5-extras succeeded.