Red Hat Bugzilla – Bug 211043
Review Request: vdradmin-am - Web interface for VDR
Last modified: 2007-11-30 17:11:45 EST
VDRAdmin-AM is a web interface for managing VDR.
- non-standard-[ug]id: intentional, ignorable
- non-readable vdradmind.conf: intentional, contains eg. passwords
- log-files-without-logrotate: logging is off by default, I'd rather not have
a dependency on logrotate here (not a strong opinion though)
- dangerous-command-in-%postun rm: intentional - to clean up possibly no
longer needed compiled templates on upgrades
- incoherent-subsys: rpmlint bug/limitation
- incoherent-init-script-name: intentional, harmless
- There's no dependency on VDR because vdradmin-am does not need to be located on the same box as the VDR it accesses.
- /var/lib/vdradmin/vdradmind.conf is in /var/lib/vdradmin instead of somewhere in /etc because it's not intended to be edited by hand after initial config, but through vdradmin-am's web UI (and vdradmind needs to write to it)
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
OK - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:
OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have sane scriptlets.
OK - Should have dist tag
OK - Should package latest version
1. rpmlint says:
You've addressed these in the bug, but a few comments:
E: vdradmin-am non-standard-uid /var/cache/vdradmin vdradmin
E: vdradmin-am non-standard-gid /var/cache/vdradmin vdradmin
E: vdradmin-am non-standard-uid /var/log/vdradmin vdradmin
E: vdradmin-am non-standard-gid /var/log/vdradmin vdradmin
E: vdradmin-am non-standard-uid /var/lib/vdradmin/vdradmind.conf vdradmin
E: vdradmin-am non-standard-gid /var/lib/vdradmin/vdradmind.conf vdradmin
E: vdradmin-am non-readable /var/lib/vdradmin/vdradmind.conf 0640
E: vdradmin-am non-standard-uid /var/lib/vdradmin vdradmin
E: vdradmin-am non-standard-gid /var/lib/vdradmin vdradmin
E: vdradmin-am non-standard-uid /var/run/vdradmin vdradmin
E: vdradmin-am non-standard-gid /var/run/vdradmin vdradmin
W: vdradmin-am log-files-without-logrotate /var/log/vdradmin
If the log files were enabled would they provide any usefull information?
W: vdradmin-am dangerous-command-in-%postun rm
I wish there was a way to avoid this. Is the cache directory multi-level?
or single directory of files? If so, you can remove the -r from the rm at
W: vdradmin-am incoherent-subsys /etc/rc.d/init.d/vdradmind $prog
W: vdradmin-am incoherent-init-script-name vdradmind
2. Should this package require 'httpd' or 'webserver' ?
3. Should the description mention that you need a vdr install somewhere in
order for this package to be usefull?
(In reply to comment #1)
> If the log files were enabled would they provide any usefull information?
I guess that depends on what one considers useful. Logging is disabled by
default upstream too, and I don't remember ever enabling it myself during the
few years I've used vdradmin.
> W: vdradmin-am dangerous-command-in-%postun rm
> I wish there was a way to avoid this. Is the cache directory multi-level?
> or single directory of files? If so, you can remove the -r from the rm at
That depends on how perl-Template-Toolkit works. Currently it creates the
same dir structure as where the templates are to below /var/cache/vdradmin,
ie. /var/cache/vdradmin/usr/share/vdradmin/template/... Leaving
the "compiled" templates there should not break anything, but will leave trash
behind as upstream templates are renamed/removed on upgrades, as well as on
> 2. Should this package require 'httpd' or 'webserver' ?
No. vdradmin-am is a standalone app-specific webserver itself. There's just
an example config snippet in docs which can be used to proxy it behind Apache.
> 3. Should the description mention that you need a vdr install somewhere in
> order for this package to be usefull?
Well, it already says "VDRAdmin-AM is a web interface for managing VDR" so I
suppose it's pretty clear that in order for it to be useful, VDR is needed
somewhere. Improvement suggestions to %description are welcome, though.
Sorry for the delay in getting back to you on this review.
ok on the logging.
I looked at perl-Template-Toolkit a bit, but didn't see any clever way around
removing those template files in postun, so I guess that has to be there. ;(
On the description, perhaps add:
"You will need access to a local or remote VDR install to use this package".
I don't see any remaining blockers here, so this package is APPROVED.
Don't forget to close this review with NEXTRELEASE once it's been imported and
Consider reviewing another waiting package to help spread out the review load. ;)
Built for devel, FC- will follow soon. Thanks!