New package: icmpdn

[Michael Schwendt]

On Tue, 28 Jun 2005 16:22:11 +0200, Fredrik Tolf wrote:

> > * 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? ;-)

That's what documentation files are for. Upstream projects can put contact
information, bugzilla databases, e-mail addresses there. The vendor of
the package is the person/organisation who builds the binary packages.
At Fedora Extras, both Vendor and Packager tag are left empty in the
spec file. Vendor information could be filled in by the build system.

> > * 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?

They are the same. Just like "Patch0" and "Patch" is the same. Don't worry
about such recommendations. You are free to ignore them. It's unimportant
spec file beautiness. Lobbying for using Source0 and Patch0 even if there
is only one source file and one patch has been a recent move by some
packagers.

> fonts anyway), but is there anything wrong with using UTF-8 in
> specfiles?

Spec files ought to be UTF-8 for consistency with system defaults.

> > * 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.

You can override %configure's defaults like this:

  %configure --libdir=%{_lib}

Note that on 64-bit multilib systems, the NSS modules are located
in /lib64, not /lib.

> > * make is better written as: make %{?_smp_mflags}
> 
> OK, will fix.

And be aware of SMP make flags breaking sometimes. ;)

Adding smp_mflags would be something a packager would documented in the
spec %changelog, so when a new tarball fails in "make" in weird ways, the
changelog entry would help when looking for the possible culprit.

> > * 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?

See yesterday's/today's thread about the "fish" package. Please don't
decrease readability of your spec file with insane checks which are
advertised as adding security. They don't add any real security. And btw,
I doubt you have ever before seen somebody building rpms as root with an
explicit build-root set to /. Let's get rid of such checks, please.

> > * %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?

Well, it's the command that refreshes the run-time dynamic linking
loader's cache. The libs in %_lib and %_libdir are found without running
it. But you want to update the cache.

> 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.

dlopen accesses the same mechanisms.
 
> > * Missing %changelog section
> 
> Forgive me my ignorance, but what is that supposed to be? Should I just
> insert the contents of the ChangeLog file?

No, document (i.e. sum up) relevant changes applied in the spec file and
packaging.
 
> > * 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.

Explicit %attr is redundant if the file was installed into build
root with proper permissions, which don't interfere with the %defattr
default attributes.
 
-- 
Michael Schwendt <mschwendt at users.sf.net>
Fedora Core release 4 (Stentz) - Linux 2.6.11-1.1369_FC4
loadavg: 1.34 1.67 1.55