New package: icmpdn

[Fredrik Tolf]

On Tue, 2005-06-28 at 15:34 +0200, Oliver Falk wrote:
> On 06/28/2005 02:46 PM, Fredrik Tolf wrote:
> > <http://www.dolda2000.com/~fredrik/icmp-dn/icmpdn.spec>
> > <http://www.dolda2000.com/~fredrik/icmp-dn/icmpdn-0.3-1.src.rpm>
> 
> The first look at it, the following things I see:
> 
> * Remove Vendor Tag

I'm sure you're right, but what is there supposed to be instead? Surely,
the author is supposed to mentioned so that people know who to
blame? ;-)

> * I'm not sure if the license is OK this way...

As far as I know, NSS modules must be LGPL, since they are dynlinked
into glibc, which is LGPL. As for the binaries, I don't see any reason
why they shouldn't be GPL:ed. Since idnlookup is only called by command
from the NSS module (it isn't linked or anything), their respective
licenses shouldn't matter.

Then again though, IANAL.

> * Source is better written as Source0 and is missing the full URI to the 
> tarball

Again, I'm sure you're right, but what's the difference between Source
and Source0?

> * Strange characters show up in %description: – (after icmpdnd, 
> idnlookup, libnss_icmp.so.2) in firefox; In vim: â~@~S

Hm. It seems that Oron Peled copied my homepage directly into the README
and specfile when he created them, so those are probably en-dashes in
UTF-8. I never noticed, since all my systems handle UTF-8 correctly.

I might as well fix it (dashes and en-dashes look the same in terminal
fonts anyway), but is there anything wrong with using UTF-8 in
specfiles?

> * In %build section, you should use %configure - it should set 
> everything correctly

As I mentioned, I don't know much about building RPMs, so you may well
be right, but does this "%configure" set the libdir correctly to /lib?
AFAIK, that's pretty important for NSS modules.

> * make is better written as: make %{?_smp_mflags}

OK, will fix.

> * In %install section do only use rm -rf "$RPM_BUILD_ROOT" (same for the 
> %clean section)

Again, I'm sure you're right, but it seems weird to me not to check so
that the root dir doesn't get removed. Will fix, though. You are
referring to just removing the checks, right?

> * %post and %postun should run ldconfig
>    And therefor PreReq: /sbin/ldconfig

Would fix, but I have no idea what "PreReq" refers to. Also, are you
sure that running ldconfig really is necessary? After all, this library
is only dlopen()'ed by the nameswitch, not used by ld.so. For sure, this
RPM has worked without ldconfig previously.

> * Missing %changelog section

Forgive me my ignorance, but what is that supposed to be? Should I just
insert the contents of the ChangeLog file?

> * In %files section icmpdnd is missing %attr to be sure the files are 
> executable
> * In %files section, init.d/ipcmpdnd is missing %config

Will fix.

> * In %files section use %{_libdir} instead of /lib/, since those libs 
> will be installed in %{_libdir} (/usr/lib), if you use %configure and 
> this is the correct place for 'em.

Are you really sure that /usr/lib is the correct place for NSS modules?
On all my systems (both RH, FC and Gentoo), all /usr/lib/libnss* are
just symlinks to /lib, and even post-installed NSS modules, like
nss_ldap, are installed in /lib. Thus, it seems weird if this would be
the only NSS module to be installed in /usr/lib.

> * In %install do:
> 
>     mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/init.d
>     install -m755 admin/icmpdnd $RPM_BUILD_ROOT%{_sysconfdir}/init.d
> 
>    instead. At least I would do so...

Yes, that seems better. Will fix.

> * Remove INSTALL from %doc, as this is not of interesst for end-users.
> * Remove NEWS from %doc, as it is EMTPY
> * README also contains the above mentioned strange characters...

Will fix.

Thanks for your comments!

Fredrik Tolf