Spec Name or Url: http://domsch.com/linux/fedora/extras/ttywatch/ttywatch.spec SRPM Name or Url: http://domsch.com/linux/fedora/extras/ttywatch/ttywatch-0.14-1.src.rpm Description: ttywatch was originally designed to log serial console output from lots of Linux machines on a single monitor machine. It handles log rotation correctly and can be configured both in a configuration file and on the command line -- and you can mix-and-match at your convenience. It can be set up to allow users to interact via the network with any of the ports being logged. It can also log output in arbitrary ways via modules, which can be built with the ttywatch-devel package. Builds clean in mock for {FC3,FC4,devel}x{i386,x86_64}.
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.