Bug 216355 - Review Request: vdr-skins - Collection of OSD skins for VDR
Review Request: vdr-skins - Collection of OSD skins for VDR
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Axel Thimm
Fedora Package Reviews List
:
Depends On: 216353
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-19 15:51 EST by Ville Skyttä
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-01 17:11:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
axel.thimm: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ville Skyttä 2006-11-19 15:51:02 EST
http://cachalot.mine.nu/6/SRPMS/vdr-skins.spec
http://cachalot.mine.nu/6/SRPMS/vdr-skins-20061119-1.cmn6.src.rpm

This package contains a collection of skins for VDR's on-screen display.
Comment 1 Axel Thimm 2007-04-22 08:48:20 EDT
- rpmlint checks return:
E: vdr-skins non-standard-gid /etc/vdr/themes/[...].theme video
E: vdr-skins non-standard-uid /etc/vdr/themes/[...].theme vdr
W: vdr-skins dangerous-command-in-%trigger rm
W: vdr-skins dangling-symlink /usr/share/vdr/text2skin/Enigma/FontMonoSpaced.ttf
/usr/share/fonts/bitstream-vera/VeraMono.ttf
W: vdr-skins incoherent-version-in-changelog 20061119-3 20061119-1
W: vdr-skins non-conffile-in-etc /etc/vdr/themes/[...].theme
W: vdr-skins no-url-tag
W: vdr-skins symlink-should-be-relative
/usr/share/vdr/text2skin/Enigma/FontMonoSpaced.ttf
/usr/share/fonts/bitstream-vera/VeraMono.ttf


- package meets naming guidelines
- package meets packaging guidelines
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source doesn't match upstream (see below)
- package compiles on devel (x86_84)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

o Some parts under /etc should probably move into /usr/share.

o Some URLs are broken like http://www.magoa.net/sttng-blue.theme and 
  http://vdr.pfroen.de/download/DeepBlue-0.1.4.tar.gz.

o Due to the latter the prepare script does not work till the end.

o The ones that were created by the script had different md5sums (but maybe
  that's expected due to tar/bz2 timestamping, in that case: do we have some
  tarball comparison tool?). I could only verify EgalSimple-1.0-demo.tar.bz2
  and SilverGreen-0.1.7.tar.bz2

[side note: why are the logos removed? Legal issues?]
Comment 2 Ville Skyttä 2007-04-22 16:31:39 EDT
(In reply to comment #1)
> o Some parts under /etc should probably move into /usr/share.

Makes sense, although only the *.theme files remain in /etc/vdr/themes and that
dir can't be changed without changing it in the main vdr package as well.  Added
to my TODO list for future vdr package revisions.

> o Some URLs are broken like http://www.magoa.net/sttng-blue.theme and 
>   http://vdr.pfroen.de/download/DeepBlue-0.1.4.tar.gz.
> 
> o Due to the latter the prepare script does not work till the end.

Yeah, I've been trying to ping upstreams of skins whose download URLs no longer
work, but haven't got replies from all of them :(

Archive.org appears to have a copy of sttng-blue.theme:
http://web.archive.org/web/*/http://www.magoa.net/sttng-blue.theme

So I guess that leaves us only DeepBlue-0.1.4.tar.gz MIA.  Suggestions?  I've
had it in this package for ages, and IMHO upstream availablity/verification is
not *that* critical because it's only data.  If you feel more comfortable with
dropping it for now, will do and reintroduce later if upstream reappears.

> o The ones that were created by the script had different md5sums (but maybe
>   that's expected due to tar/bz2 timestamping, in that case: do we have some
>   tarball comparison tool?). I could only verify EgalSimple-1.0-demo.tar.bz2
>   and SilverGreen-0.1.7.tar.bz2

bunzip2'ing and comparing the uncompressed tarballs could work.  And rpmdev-diff
might be useful as well.

> [side note: why are the logos removed? Legal issues?]

They are logos of various TV and radio channels, most of which are likely
trademarked, and I haven't found any evidence that they'd be included in
upstream tarballs with the trademark holders' consent.
Comment 3 Axel Thimm 2007-04-22 17:02:52 EDT
(In reply to comment #2)
> > o Some parts under /etc should probably move into /usr/share.
> 
> Makes sense, although only the *.theme files remain in /etc/vdr/themes and
> that dir can't be changed without changing it in the main vdr package as well. 
> Added to my TODO list for future vdr package revisions.

Maybe just a symlink in /etc pointing to a /usr/share location?

> I've had it in this package for ages, and IMHO upstream
> availablity/verification is not *that* critical because it's only data.
> If you feel more comfortable with dropping it for now, will do and
> reintroduce later if upstream reappears.

No, that's not a blocker, at least not for me, and I hope that the guidelines
can be interpreted as such.

Verification of sources is needed for novice errors and untrusted parties, I'd
say in this case we can outrule both. :)

I just wanted to mention these issues to keep them in mind, and to have this in
the review in case someone digs this up in the future and thinks we didn't do a
thorough job, after all I need to write something about whether the sources
could be verified or not.

I'd say, if you like move the non-config parts out of /etc leaving a symlink
behind, but that's just a mild recommendation. I'll just go ahead and approve as
is. :)

BTW: Perhaps you want to remove the %{?dist} tag, as the package wouldn't change
from release to release. But I don't know how to make the exact same package
appear in FC5/FC6/F7 simultaneously w/ either plague or brew, so maybe you can't
drop it.
Comment 4 Ville Skyttä 2007-04-22 17:31:29 EDT
Thanks!

I'll think about the /etc -> /usr/share change for a bit before the first build;
however the symlink approach doesn't sound tempting as symlinks to dirs may
cause various annoyances later.  Maybe I'll just roll a new vdr package with the
themes dir moved before the first build of this one.

Looks like a bunch of not that large noarch packages have been accepted for
disttagless copying between branches recently (such as various -firmware
packages), so I suppose it's ok here as well, will drop the disttag.

New Package CVS Request
=======================
Package Name: vdr-skins
Short Description: Collection of OSD skins for VDR
Owners: ville.skytta@iki.fi
Branches: FC-6
InitialCC: 
Comment 5 Ville Skyttä 2007-04-23 15:48:25 EDT
Actually, in some situations VDR writes to the *.theme files, so I'll relocate
them to /var/lib/vdr/themes instead of /usr/share.
Comment 6 Ville Skyttä 2007-05-01 17:11:02 EDT
Built for devel a week ago, repocopied to FC6 today, should appear in the next
push along with a vdr revision which relocates themes to /var/lib/vdr/themes.

Note You need to log in before you can comment on or make changes to this bug.