Suggestions:
- don't make separate devel package. Include 2k include-file into the main
package and add "Provides: ttywatch-devel" tag instead.

Remarks & nitpicks:
- it is possible to compile with $RPM_OPT_FLAGS, use OPTFLAGS=... for make line.
- as install pathes are hardcoded in Makefile, it is better to overwrite them at
"make install" time, setting appropriate Makefile's variables in cmdline with
appropriate rpm macros (%{_sbindir} etc.)
- %defattr(...) is enough, file attributes are set properly by "make install"
- IMO it is better to own /var/log/ttywatch by the package. The similar good
precedents are httpd and ppp ...
- IMHO it is better to use %{name} instead of hardcoded name...

Created attachment 121407 [details]
Suggested changes for the spec-file

Thanks for the suggested changes.  All is fine, except the changing 
of /var/log/ttywatch.  As this is commonly used to log all serial traffic 
(i.e. serial consoles), passwords are highly likely items to be logged, and we 
really don't want rpm mucking with the permissions on upgrade if the sysadmin 
changes them.  So I've backed out that change and applied the rest.

rpmlint now complains about it, but it's got a big fat comment.
W: ttywatch dangerous-command-in-%post install

Likewise, this change causes rpmlint to complain:
W: ttywatch devel-file-in-non-devel-package /usr/include/ttywatch.h

but if that's OK, I'm OK with it.

http://domsch.com/linux/fedora/extras/ttywatch/ttywatch.spec 
http://domsch.com/linux/fedora/extras/ttywatch/ttywatch-0.14-1.src.rpm

That should have been:
http://domsch.com/linux/fedora/extras/ttywatch/ttywatch-0.14-2.src.rpm

> we really don't want rpm mucking with the permissions on upgrade if the 
> sysadmin changes them.
I still not quite understand it.

By default, the /var/log/ttywatch permissions is "rwx------ root root". It is
the same default as, for example, /var/log/ppp and /var/log/httpd permissions.
But both "ppp" and "httpd" packages own its log directories...

What reasons can lead to necessity to change /var/log/ttywatch perms/owners? Why
the approximately similar Fedora stuff, for example /var/log/ppp, is not
affected by such reasons?

Anyway, according to http://fedoraproject.org/wiki/PackageReviewGuidelines, 
> - MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory. The exception to this are directories listed explicitly 
> in the Filesystem Hierarchy Standard


Also, IMHO it is possible to use the recommended %{_smp_mflags} for the %build
stage.

(In reply to comment #3)
> Likewise, this change causes rpmlint to complain:
> W: ttywatch devel-file-in-non-devel-package /usr/include/ttywatch.h
> 
> but if that's OK, I'm OK with it.
Not OK with me. Shipping a header without a corresponding library is non-sense.

(for comment #6):

> Shipping a header without a corresponding library is non-sense.

There is no any library at all. The header file is shipped just for building
additional plugins for the main executable.

BTW this header file is 2392-bytes only, shipping an extra "devel" package for
such amount of bytes is an ugly way. (There are precedents in FE for omitting of
separate devel package in such situations).

(In reply to comment #7)
> (for comment #6):
> 
> > Shipping a header without a corresponding library is non-sense.
> 
> There is no any library at all. The header file is shipped just for building
> additional plugins for the main executable.
Headers describe APIs to something. Therefore, shipping a header without the
corresponding implementation therefore is nonsense.

> BTW this header file is 2392-bytes only, shipping an extra "devel" package for
> such amount of bytes is an ugly way.
Shipping such a file is simply pollution.

> (There are precedents in FE for 
> omitting of > separate devel package in such situations).
Your package is missing the implementation of the devel package.

I don't want to fret about this, but unless you can provide an example or
describe how this header file is supposed to be used, I consider to veto against
this package. - I definitely won't approve it.

> Headers describe APIs to something.
Sure. "something" is /usr/sbin/ttywatch itself.

ttywatch is linked properly with -ldl, i.e. can dlopen(3) plugins at runtime. To
write a plugin, a user should follow some io-data layouts/bits, which are
specified in /usr/include/ttywatch.h .

As there are no any referenses in /usr/sbin/ttywatch usable for extern plugins,
"-rdynamic" option at link time is not needed.

> I don't want to fret about this, but unless you can provide an example or
> describe how this header file is supposed to be used
Matt, could you write some example? BTW, it can be useful to include into %doc 

> I consider to veto against this package
Could you gpg-sign a text with veto report and reasons for such veto? ;)

If you decide to keep the -devel provision, it should be made versioned, ie. 
Provides: %{name}-devel = %{version}-%{release} 

OK, I brought back the -devel package, and made ttywatch own /var/log/ttywatch.
$ rpmlint RPMS/x86_64/ttywatch-0.14-3.x86_64.rpm
E: ttywatch non-standard-dir-perm /var/log/ttywatch 0700

http://domsch.com/linux/fedora/extras/ttywatch/ttywatch.spec 
http://domsch.com/linux/fedora/extras/ttywatch/ttywatch-0.14-3.src.rpm

> I brought back the -devel package
Why?..
IMHO there was good reasons to not split devel stuff as the separate package...

If you worry about rpmlint reports, try to check some popular Fedora Core's
packages for example. You will be surprised :) Actually the recommendation to
provide clean rpmlint output is not so strict. Sometimes Fedora behaviour
differs from rpmlint preferences. Sometimes (like 0700 "non-standard-dir-perm")
it "disallows" obviously needed things. Therefore follow rpmlint until it breaks
really useful things.

I still suggest to omit the separate devel package. Just specify "Provides" tag
as described in the comment #10

The upstream author (Michael K. Johnson, former Red Hatter and former Fedora
project leader) had a separate devel package in his original spec file.  It's
13KB as a separate RPM, and not I thought worth a long debate over including it
or breaking it out.  Could go either way, but may as well stay consistent with
the original.

> It's 13KB as a separate RPM
It packages just 2.4KB header, accompanied with 18KB license file (because of
separate packaging).

Package splitting is useful for saving disk space/network traffic etc, but this
benefit takes effect when the package size is not too small. In the case the
splitted package is too small (and surely not requires extra -devel packages),
the splitting does not save the user disk space, but takes a cost as additional
file in the repository (and in the repository databases).

If you want to keep separate devel packages nevertheless, let it be so :/, it is
not a blocker.

The devel package is missing a dependency on glib2-devel (#include <glib.h>).

> glib2-devel
Oops!

Ville, thanks a lot!

Matt, save devel package! :) Just add "Requires: glib2-devel" to it.

Created attachment 121581 [details]
some little nitpicks for %post sections

IMO it is better to do "Requires(post): /sbin/chkconfig" etc., as
/sbin/chkconfig and /sbin/service are required exactly. (Theoretically it is
possible that these utilities will go to another packages).

Works fine!

MUST/SHOULD items will be OK after some required changes (comment #15 - comment #17)

Requested changes made.
http://domsch.com/linux/fedora/extras/ttywatch/ttywatch.spec 
http://domsch.com/linux/fedora/extras/ttywatch/ttywatch-0.14-4.src.rpm

APPROVED!

Builds for devel, FC-4, FC-3 complete.  Closing.