[Bug 190343] Review Request: VDR - Video Disk Recorder

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: VDR - Video Disk Recorder


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=190343





------- Additional Comments From ville.skytta at iki.fi  2006-05-29 14:49 EST -------
(In reply to comment #2)
> Sorry for the delay...

NP, should the blocker be moved from FE-NEW to FE-REVIEW? ;)

> use everywhere the '%__XXXX' macros, or everywhere only 'XXXX'.

Done (the latter).

>   | BuildRequires:  glibc-kernheaders >= 2.4-9.1.94
>   is unneeded; every target system has this version of the
>   glibc-kernheaders package

Dropped altogether per
https://www.redhat.com/archives/fedora-maintainers/2006-May/msg00071.html

>   | Requires: udev
>   should be
>   | Requires(pre):    udev
>   | Requires(postun): udev

Disagreed.  Even if for some obscure reason udev would be installed after vdr
(already a very unlikely case), neither will automatically start during the
transaction, and after the transaction udev is available and has changed the dir
ownerships so it doesn't matter.

>   (or '/etc/udev/rules.d' instead of 'udev')

We really need udev instead of the dir, so I'm leaving that as is.

> please use 'fedora-usermgt' for the 'vdr' user and 'video' group.

I'm not going to add a dependency on fedora-usermgmt (not parroting my opinions
about it here, as I've already done that several times on mailing lists).  If
you think testing for availability of the f-u scripts and using them in %pre if
available would be useful, I could be persuaded to do that.  But I'd rather
leave that stuff out altogether.

>   On the other side, 'vdr' has some history and not using 'vdr' might
>   cause other problems

Seconded, left as is.

> * ownership/location of the vdr configuration directory is another
>   problem. I dislike the vdr:video owned /etc/vdr directory somehow
>   because:
>   * some configuration data is modified by the 'vdr' daemon (channels.conf,
>     remote.conf, setup.conf) so it should be located in /var/lib/vdr

Kind of agreed, but this package has an existing user base and I don't see too
good ways to migrate it on upgrades and would not at all want to break existing
setups either.  Do you have ideas how that could be done?

>   * not all configuration data should be modifiable by the daemon
>     (e.g. commands.conf) so permissions should should be root:video.

Do you mean for the config dir, and if yes, root:video 0755 or 0775?  What about
subdirs there?  The former would require going through quite a bit of code and
ensuring that the daemon and its plugins operate in a way that they'd still work
without having write access to the affected dirs, and the latter might be too
relaxed.  Doing the former and patching where needed would be a good thing to
do, but I think it's somewhat out of scope for the package's acceptance.

>   | SUBSYSTEM=="input", SYSFS{../name}=="DVB on-card IR receiver",
SYMLINK+="input/event-remote", GROUP="video", MODE="0660"

Added as an example to the udev rules file.

One thing outside of this list I'm considering to do is to rename the "runvdr"
script to something else; while it's somewhat compatible with the upstream
runvdr example, it doesn't implement the implied functionality (auto-restart on
certain exit codes, reloading of DVB modules).  Or implementing at least some of
that in the version shipped with the package.  Thoughts?

http://cachalot.mine.nu/5/SRPMS/vdr-1.4.0-5.src.rpm

* Mon May 29 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.4.0-5
- Address some review notes in #190343 comment 2:
- Add example udev rule for predictable remote control device naming.
- Drop glibc-kernheaders build dependency.
- Specfile cleanups.

* Sun May 28 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.4.0-4
- Apply upstream 1.4.0-2 maintenance patch.

* Sun May 14 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.4.0-3
- Apply upstream 1.4.0-1 maintenance patch.
- Drop unneeded version check from %%check.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.