Spec URL: http://lucilanga.fedorapeople.org/alevt.spec SRPM URL: http://lucilanga.fedorapeople.org/alevt-1.6.2-1.fc9.src.rpm Description: This program decodes and displays Videotext/Teletext from a /dev/vbi device. Notes: Default target for pixmap file was wrong so it had to be patched. Also default build didn't produce any debug info so I had to override OPT variable.
Informal package review: ======================== -Summary: Teletext decoder/browser Name: alevt Version: 1.6.2 Release: 1%{?dist} -License: GPLv2 +Summary: Teletext decoder/Browser Group: Applications/Multimedia +License: GPLv2 +URL: http://goron.de/~froese Source: http://goron.de/~froese/%{name}/%{name}-%{version}.tar.gz Source1: alevt.desktop Patch0: alevt-1.6.2-pixmap.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -URL: http://goron.de/~froese * Please, keep the usual order of fields. * e.g. Name: Version: Release: Summary: Group: License: URL: Source0: BuildRoot: BuildRequires: Requires: %build -#overwrite $OPT to produce debuginfo -make %{?_smp_mflags} -e OPT="-O2 -w" +#overwrite $OPT to produce standard Fedora build with propper FLAGS + +# alevt does not have standard build system, so we call configure +# to populate CFLAGS, then we move them to another var which persists +# and we *unset CFLAGS* and have alevt build system to populate it +%configure || true +FLAGS=${CFLAGS} +unset CFLAGS +# will produce lot of garbage on output +make %{?_smp_mflags} -e OPT="${FLAGS}" * Hope it's selfdescriptive. * The debuginfos were somewhat useless without the "-g" option. Now it uses Fedora's own CFLAGS automatically. %install rm -rf %{buildroot} -mkdir -p $RPM_BUILD_ROOT/%{_bindir} -mkdir -p $RPM_BUILD_ROOT/%{_mandir}/man1 +mkdir -p %{buildroot}%{_bindir} +mkdir -p %{buildroot}%{_mandir}/man1 make USR_X11R6=/usr MAN=share/man rpm-install desktop-file-install --vendor="fedora" \ - --dir=${RPM_BUILD_ROOT}%{_datadir}/applications %{SOURCE1} + --dir=%{buildroot}%{_datadir}/applications %{SOURCE1} * Be consistent in using macro style v. variable style %clean @@ -42,9 +50,7 @@ rm -rf %{buildroot} %{_bindir}/alevt-date %{_bindir}/alevt-cap %{_datadir}/applications/*%{name}.desktop -%doc %{_mandir}/man1/alevt.1x.gz -%doc %{_mandir}/man1/alevt-date.1.gz -%doc %{_mandir}/man1/alevt-cap.1.gz +%{_mandir}/man?/%{name}* %{_datadir}/pixmaps/mini-alevt.xpm %doc README CHANGELOG COPYRIGHT * The one line will rule them all :) @@ -62,6 +68,7 @@ rm -rf %{buildroot} * Sun May 23 1999 Karsten Hopp <karsten> - several minor patches of Marios spec-file: - german descriptions - buildroot (patched Makefile) - some changed install-paths +- german descriptions +- buildroot (patched Makefile) +- some changed install-paths + * Just typo-enhancements -- Incorporate the changes, please, so the package is closer to formal review.
Debuginfos: =========== * See the differences in sizes (first column in Bytes), the more is "better" :). Your old OPT="...": ------------------- 8348 /usr/lib/debug/usr/bin/alevt-cap.debug 6352 /usr/lib/debug/usr/bin/alevt-date.debug 12780 /usr/lib/debug/usr/bin/alevt.debug With "... -g": ---------- 71760 /usr/lib/debug/usr/bin/alevt-cap.debug 37412 /usr/lib/debug/usr/bin/alevt-date.debug 149152 /usr/lib/debug/usr/bin/alevt.debug Fedora CFLAGS: -------------- 75376 /usr/lib/debug/usr/bin/alevt-cap.debug 39556 /usr/lib/debug/usr/bin/alevt-date.debug 156104 /usr/lib/debug/usr/bin/alevt.debug
(In reply to comment #1) > * The debuginfos were somewhat useless without the "-g" option. Now it uses > Fedora's own CFLAGS automatically. My intention was to use -g from the beginning -w was there by mistake. Anyway the best aproach is fedora's own CFLAGS. I've modified the file and bump release: http://lucilanga.fedorapeople.org/alevt.spec http://lucilanga.fedorapeople.org/alevt-1.6.2-2.fc9.src.rpm
Proposing this patch, which is more fedora best practices aware: * still populates the OPT variable but without invoking %configure (which fails right after) * less variable playing/messing * updated comment --- alevt.spec 2008-06-29 19:26:04.000000000 +0200 +++ alevt.spec_new 2008-06-30 10:36:39.000000000 +0200 @@ -23,14 +23,11 @@ This program decodes and displays Videot %build #overwrite $OPT to produce standard Fedora build with propper FLAGS -# alevt does not have standard build system, so we call configure -# to populate CFLAGS, then we move them to another var which persists -# and we *unset CFLAGS* and have alevt build system to populate it -%configure || true -FLAGS=${CFLAGS} -unset CFLAGS +# alevt does not have standard build system, so we populate OPT, +# which is internal build variable to accomodate Fedora opt flags + # will produce lot of garbage on output -make %{?_smp_mflags} -e OPT="${FLAGS}" +make %{?_smp_mflags} -e OPT="%{optflags}" %install
Updated spec file and bump release: http://lucilanga.fedorapeople.org/alevt.spec http://lucilanga.fedorapeople.org/alevt-1.6.2-3.fc9.src.rpm
> make USR_X11R6=/usr MAN=share/man rpm-install Omit hardcoded paths like "/usr". Better is to use RPM macros http://fedoraproject.org/wiki/Packaging/RPMMacros This is mandatory. > desktop-file-install --vendor="fedora" \ > --dir=%{buildroot}%{_datadir}/applications %{SOURCE1} '--vendor="fedora"' is going to be obsoleted in short time, remove it (even that Package guidelines says otherwise)
(In reply to comment #6) > Omit hardcoded paths like "/usr". Better is to use RPM macros > http://fedoraproject.org/wiki/Packaging/RPMMacros This is mandatory. updated to use %{prefix} (USR_X11R6 is mainly used for for includes, package's install will correctly pickup buildroot.) > '--vendor="fedora"' is going to be obsoleted in short time, remove it (even that > Package guidelines says otherwise) dropped it. ... and bumped version new files: http://lucilanga.fedorapeople.org/alevt.spec http://lucilanga.fedorapeople.org/alevt-1.6.2-4.fc9.src.rpm
Thanks for prompt changes, will do official review soon.
[root@dhcp-lab-192 SPECS]# diff -pruN alevt.spec_bak alevt.spec --- alevt.spec_bak 2008-07-28 17:43:35.364713214 +0200 +++ alevt.spec 2008-07-28 18:13:27.216716710 +0200 @@ -1,32 +1,36 @@ Name: alevt Version: 1.6.2 Release: 4%{?dist} -Summary: Teletext decoder/browser +Summary: This program decodes and displays Videotext/Teletext from a /dev/vbi device Group: Applications/Multimedia License: GPLv2 URL: http://goron.de/~froese Source: http://goron.de/~froese/%{name}/%{name}-%{version}.tar.gz Source1: alevt.desktop Patch0: alevt-1.6.2-pixmap.patch +Patch1: alevt-1.6.2-manpath.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: libX11-devel BuildRequires: libpng-devel BuildRequires: desktop-file-utils %description -This program decodes and displays Videotext/Teletext from a /dev/vbi device. +AleVT is a teletext/videotext decoder and browser for the +bttv driver (/dev/vbi) and X11. It features multiple windows, +a page cache, regexp searching, built-in manual, and more. +There's also a program to get the time from teletext and +one to capture teletext pages from scripts. + %prep %setup -q %patch0 -p1 -b .pixmap +%patch1 -p1 -b .manpath %build -#overwrite $OPT to produce standard Fedora build with propper FLAGS - # alevt does not have standard build system, so we populate OPT, -# which is internal build variable to accomodate Fedora opt flags - -# will produce lot of garbage on output +# which is internal build variable to accommodate Fedora opt flags +# This will produce lot of garbage on output. make %{?_smp_mflags} -e OPT="%{optflags}" @@ -35,7 +39,7 @@ rm -rf %{buildroot} mkdir -p %{buildroot}%{_bindir} mkdir -p %{buildroot}%{_mandir}/man1 -make USR_X11R6=%{_prefix} MAN=share/man rpm-install +make USR_X11R6=%{_prefix} MAN=%{_mandir} rpm-install desktop-file-install \ --dir=%{buildroot}%{_datadir}/applications %{SOURCE1} * It's mostly description, summary and comments changes. Feel free to tweak them. * There's bigger change according to hard-coded man path in MAN variable, now, with patch, it's in Fedora-macro-way. [root@dhcp-lab-192 BUILD]# cat ../SOURCES/alevt-1.6.2-manpath.patch --- alevt-1.6.2/Makefile 2008-07-28 18:08:04.778713433 +0200 +++ alevt-1.6.2/Makefile_manpath 2008-07-28 18:09:28.572715966 +0200 @@ -66,9 +66,9 @@ rpm-install: all install -m 0755 alevt ${RPM_BUILD_ROOT}$(USR_X11R6)/bin install -m 0755 alevt-date ${RPM_BUILD_ROOT}$(USR_X11R6)/bin install -m 0755 alevt-cap ${RPM_BUILD_ROOT}$(USR_X11R6)/bin - install -m 0644 alevt.1x ${RPM_BUILD_ROOT}$(USR_X11R6)/$(MAN)/man1 - install -m 0644 alevt-date.1 ${RPM_BUILD_ROOT}$(USR_X11R6)/$(MAN)/man1 - install -m 0644 alevt-cap.1 ${RPM_BUILD_ROOT}$(USR_X11R6)/$(MAN)/man1 + install -m 0644 alevt.1x ${RPM_BUILD_ROOT}$(MAN)/man1 + install -m 0644 alevt-date.1 ${RPM_BUILD_ROOT}$(MAN)/man1 + install -m 0644 alevt-cap.1 ${RPM_BUILD_ROOT}$(MAN)/man1 install -d 0755 $(RPM_BUILD_ROOT)$(USR_X11R6)/share/pixmaps install -m 0644 contrib/mini-alevt.xpm $(RPM_BUILD_ROOT)$(USR_X11R6)/share/pixmaps * When is the last point done, I'll finish the review.
I've added your patch (thanks). I altered the description: AleVT is a teletext/videotext decoder and browser for the bttv driver (/dev/vbi) and X11. to AleVT is a teletext/videotext decoder and browser for the vbi (/dev/vbi) device and X11. because alevt is not driver dependant, it works with any vbi device and 'bttv driver' seems confusing. ... and bumped version http://lucilanga.fedorapeople.org/alevt.spec http://lucilanga.fedorapeople.org/alevt-1.6.2-5.fc9.src.rpm
MUST Items: [PASS] - MUST: rpmlint must be run on every package. [PASS] - MUST: The package must be named according to the Package Naming Guidelines . [PASS] - MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption on Package Naming Guidelines . [PASS] - MUST: The package must meet the Packaging Guidelines . [PASS] - MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . [PASS] - MUST: The License field in the package spec file must match the actual license. [PASS] - MUST: If (and only if) the source package includes the text of the license(s) ... [PASS] - MUST: The spec file must be written in American English. [PASS] - MUST: The spec file for the package MUST be legible. If the reviewer is unable to read [PASS] - MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. [PASS] - MUST: The package must successfully compile and build into binary rpms on at least one supported architecture. * PASS on i386, under mock and koji [PASS] - MUST: If the package does not successfully compile, build or work on an architecture, [PASS] - MUST: All build dependencies must be listed in BuildRequires * PASS on i386, under mock and koji [ NA ] - MUST: The spec file MUST handle locales properly. [ NA ] - MUST: Every binary RPM package which stores shared library files [ NA ] - MUST: If the package is designed to be relocatable, the packager must state [PASS] - MUST: A package must own all directories that it creates. [PASS] - MUST: A package must not contain any duplicate files in the %files listing. [PASS] - MUST: Permissions on files must be set properly. Executables should be set with [PASS] - MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} [PASS] - MUST: Each package must consistently use macros, as described in the [PASS] - MUST: The package must contain code, or permissable content. [ NA ] - MUST: Large documentation files should go in a -doc subpackage. [PASS] - MUST: If a package includes something as %doc, it must not affect the runtime of the application. [ NA ] - MUST: Header files must be in a -devel package. [ NA ] - MUST: Static libraries must be in a -static package. [ NA ]- MUST: Packages containing pkgconfig(.pc) files [ NA ] - MUST: If a package contains library files with a suffix [ NA ] - MUST: In the vast majority of cases, devel packages must require [ NA ] - MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec. [PASS] - MUST: Packages containing GUI applications must include a %{name}.desktop file [PASS] - MUST: Packages must not own files or directories already owned by other packages. [PASS] - MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} [PASS] - MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: [ NA ] - SHOULD: If the source package does not include license text(s) * includes [ NA ] - SHOULD: The description and summary sections in the package spec file should contain [mock] - SHOULD: The reviewer should test that the package builds in mock. [koji] - SHOULD: The package should compile and build into binary rpms on all supported architectures. [ NA ] - SHOULD: The reviewer should test that the package functions as described. * lack of applicable HW [ NA ] - SHOULD: If scriptlets are used, those scriptlets must be sane. [ NA ] - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [ NA ] - SHOULD: The placement of pkgconfig(.pc) [ NA ] - SHOULD: If the package has file dependencies outside of /etc, /bin, --- REVIEWED
New Package CVS Request ======================= Package Name: alevt Short Description: Teletext decoder/browser Owners: lucilanga Branches: F-8 F-9 EL-5 InitialCC: Cvsextras Commits: yes
cvs done.
alevt-1.6.2-5.fc8 has been submitted as an update for Fedora 8
alevt-1.6.2-5.fc9 has been submitted as an update for Fedora 9
alevt-1.6.2-5.fc8 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.
alevt-1.6.2-5.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.