Bug 671883 - Review Request: v4l-utils - Utilities for video4linux and DVB devices
Summary: Review Request: v4l-utils - Utilities for video4linux and DVB devices
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Peter Robinson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-01-22 13:44 UTC by Hans de Goede
Modified: 2011-03-23 22:51 UTC (History)
6 users (show)

Fixed In Version: v4l-utils-0.8.3-2.fc14
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-03-21 03:28:27 UTC
Type: ---
pbrobinson: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)
Version 2 of the v4l-utils.spec (3.92 KB, application/octet-stream)
2011-01-23 17:40 UTC, Mauro Carvalho Chehab
no flags Details

Description Hans de Goede 2011-01-22 13:44:32 UTC
Spec URL: http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
SRPM URL: http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.2-1.fc15.src.rpm
Description:
v4l-utils is a collection of various video4linux (V4L) and DVB utilities. The
main v4l-utils package contains cx18-ctl, ir-keytable, ivtv-ctl, v4l2-ctl and
v4l2-sysfs-path.

Note to reviewers: The SRPM contains a snapshot package from upstream git. I'm also upstream and I plan to do a 0.8.2 release real soon now, but first I want to see if any non packaging changes pop up in this review :)

Comment 1 Ralf Corsepius 2011-01-23 06:25:47 UTC
Not a formal review, just some feedback from trying to build this package in rawhide-mock:


1. I am facing several warnings of this kind:

make[2]: Entering directory `/builddir/build/BUILD/v4l-utils-0.8.2-test/utils/libv4l2util'
make[2]: warning: jobserver unavailable: using -j1.  Add `+' to parent make rule.

... no idea about the cause. Wild guess: Broken makefiles.

2. A file does not compile:
cc -Wp,-MMD,"keytable.d",-MQ,"keytable.o",-MP -c -I../../include -I../../lib/include -D_GNU_SOURCE -O2 -g -pipe -Wall 
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -o keytable.o keytable.c
keytable.c:87:66: error: expected ',' or ';' before 'V4L_UTILS_VERSION'

3. After the error from 2. above occurs, make doesn't abort the build process, but continues.


1. + 3. together make be think, something might be basically broken with this package's makefiles.


[Removing msmp_flags seems to work-around 1., but doesn't help 2. and 3.]

Comment 2 Hans de Goede 2011-01-23 14:51:20 UTC
(In reply to comment #1)
> Not a formal review, just some feedback from trying to build this package in
> rawhide-mock:
> 

Thanks for the feedback!

> 
> 1. I am facing several warnings of this kind:
> 
> make[2]: Entering directory
> `/builddir/build/BUILD/v4l-utils-0.8.2-test/utils/libv4l2util'
> make[2]: warning: jobserver unavailable: using -j1.  Add `+' to parent make
> rule.
> 
> ... no idea about the cause. Wild guess: Broken makefiles.

Fixed by this upstream commit:
http://git.linuxtv.org/v4l-utils.git?a=commitdiff;h=ad1bfbbea5960243fdaf93a6d3386ff0c65b6337

> 
> 2. A file does not compile:
> cc -Wp,-MMD,"keytable.d",-MQ,"keytable.o",-MP -c -I../../include
> -I../../lib/include -D_GNU_SOURCE -O2 -g -pipe -Wall 
> -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
> --param=ssp-buffer-size=4 -m64 -mtune=generic -o keytable.o keytable.c
> keytable.c:87:66: error: expected ',' or ';' before 'V4L_UTILS_VERSION'
> 

Ah, this happens when specifying CFLAGS explicitly, fixed by this upstream commit:
http://git.linuxtv.org/v4l-utils.git?a=commitdiff;h=d0b2dcf42230a337943164cd0bc2ebd1c4725543

> 3. After the error from 2. above occurs, make doesn't abort the build process,
> but continues.

Oh, good one! Bad error propagation in a for $SUBDIRS loop, fixed:
http://git.linuxtv.org/v4l-utils.git?a=commitdiff;h=f51ac9f8b4dd0265575fd41ebaa36327309e4ae7

There still is one other issue which needs to be taken care of upstream. Then I'll do a real 0.8.2 release upstream and drop a link to an updated srpm here.

Comment 3 Mauro Carvalho Chehab 2011-01-23 17:40:00 UTC
Created attachment 474844 [details]
Version 2 of the v4l-utils.spec

This is the version 2 of the spec. This version, together with some patches
I've added upstream, fixes the build errors and properly installs the RC
keymaps and the udev rules.

Comment 4 Hans de Goede 2011-01-25 15:58:36 UTC
Hi All,

Here is a new version, based on the just released official 0.8.2 release, fixing the issues mentioned in comment #1.

Spec URL: http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
SRPM URL:
http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.2-2.fc15.src.rpm

A note about the rpmlint output wrt missing manpages we are working on this upstream (for example we added a ir-keytable manpage yesterday).

Regards,

Hans

Comment 5 Ralf Corsepius 2011-01-26 03:35:11 UTC
(In reply to comment #4)
> http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.2-2.fc15.src.rpm

Doesn't build for me (mock -r fedora-rawhide-x86_64 v4l-utils-0.8.2-2.fc15.src.rpm):

..
make[2]: Entering directory `/builddir/build/BUILD/v4l-utils-0.8.2/lib/libv4l1'
cc -Wp,-MMD,"libv4l1.d",-MQ,"libv4l1.o",-MP -c -I../include -fvisibility=hidden -fPIC -I../../include -I../../lib/include -D_GNU_SOURCE -DV4L_UTILS_VERSION='"0.8.2"' -O2 -g -pipe -Wall -Wp,-D_FO
cc -Wp,-MMD,"log.d",-MQ,"log.o",-MP -c -I../include -fvisibility=hidden -fPIC -I../../include -I../../lib/include -D_GNU_SOURCE -DV4L_UTILS_VERSION='"0.8.2"' -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SO
log.c:22:28: fatal error: linux/videodev.h: No such file or directory
compilation terminated.
libv4l1.c:60:28: fatal error: linux/videodev.h: No such file or directory
compilation terminated.
make[2]: *** [log.o] Error 1

Comment 6 Hans de Goede 2011-01-26 08:07:56 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.2-2.fc15.src.rpm
> 
> Doesn't build for me (mock -r fedora-rawhide-x86_64
> v4l-utils-0.8.2-2.fc15.src.rpm):
> 
> ..
> make[2]: Entering directory `/builddir/build/BUILD/v4l-utils-0.8.2/lib/libv4l1'
> cc -Wp,-MMD,"libv4l1.d",-MQ,"libv4l1.o",-MP -c -I../include -fvisibility=hidden
> -fPIC -I../../include -I../../lib/include -D_GNU_SOURCE
> -DV4L_UTILS_VERSION='"0.8.2"' -O2 -g -pipe -Wall -Wp,-D_FO
> cc -Wp,-MMD,"log.d",-MQ,"log.o",-MP -c -I../include -fvisibility=hidden -fPIC
> -I../../include -I../../lib/include -D_GNU_SOURCE -DV4L_UTILS_VERSION='"0.8.2"'
> -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SO
> log.c:22:28: fatal error: linux/videodev.h: No such file or directory
> compilation terminated.
> libv4l1.c:60:28: fatal error: linux/videodev.h: No such file or directory
> compilation terminated.
> make[2]: *** [log.o] Error 1

Oops, good point we need to deal with this upstream since linux/videodev.h is going away (deprecated) but for now adding a BuildRequires: kernel-headers fixes this:

Spec URL: http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
SRPM URL:
http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.2-3.fc15.src.rpm

I'll start a discussion upstream about how to deal with the v4l1 API stuff libv4l1 needs from videodev.h

Comment 7 Parag AN(पराग) 2011-01-27 06:50:41 UTC
oops! sorry, I am not sure what used to happen but this is second time in Chromium browser when I tried to just CC myself, package component reset to first package 0xFFFF

reverting accidental change back.

Comment 8 Hans de Goede 2011-02-10 09:27:51 UTC
I've rebased my package to the new upstream release from yesterday:
http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.3-1.fc15.src.rpm

Comment 9 Peter Robinson 2011-02-10 10:09:21 UTC
I'll review this.

Comment 10 Peter Robinson 2011-02-10 14:58:38 UTC
Mostly OK. I think the QT app needs a desktop file (but it might not be an app that shoud) and there should be versioned required on the libs at a guess for the various utils.

- non -devel packages should require fully versioned base
- packages containing GUI apps must include %{name}.desktop file


+ rpmlint output

rpmlint v4l* libv4l-0.8.3-1.fc15.x86_64.rpm libv4l-devel-0.8.3-1.fc15.x86_64.rpm qv4l2-0.8.3-1.fc15.x86_64.rpm
v4l-utils.src: W: spelling-error %description -l en_US ctl -> cyl, ct, cl
v4l-utils.src: W: spelling-error %description -l en_US ir -> IR, Ir, or
v4l-utils.src: W: spelling-error %description -l en_US keytable -> key table, key-table, marketable
v4l-utils.src: W: spelling-error %description -l en_US ivtv -> invt, Ives, Ivan
v4l-utils.src: W: spelling-error %description -l en_US sysfs -> sysops, sysop, systems
v4l-utils.x86_64: W: spelling-error %description -l en_US ctl -> cyl, ct, cl
v4l-utils.x86_64: W: spelling-error %description -l en_US ir -> IR, Ir, or
v4l-utils.x86_64: W: spelling-error %description -l en_US keytable -> key table, key-table, marketable
v4l-utils.x86_64: W: spelling-error %description -l en_US ivtv -> invt, Ives, Ivan
v4l-utils.x86_64: W: spelling-error %description -l en_US sysfs -> sysops, sysop, systems
v4l-utils.x86_64: W: no-manual-page-for-binary v4l2-sysfs-path
v4l-utils.x86_64: W: no-manual-page-for-binary ivtv-ctl
v4l-utils.x86_64: W: no-manual-page-for-binary v4l2-ctl
v4l-utils.x86_64: W: no-manual-page-for-binary cx18-ctl
v4l-utils-devel-tools.x86_64: W: spelling-error %description -l en_US dbg -> db, dg, deg
v4l-utils-devel-tools.x86_64: W: no-manual-page-for-binary decode_tm6000
v4l-utils-devel-tools.x86_64: W: no-manual-page-for-binary v4l2-dbg
v4l-utils-devel-tools.x86_64: W: no-manual-page-for-binary v4l2-compliance
qv4l2.x86_64: W: no-manual-page-for-binary qv4l2
7 packages and 1 specfiles checked; 0 errors, 19 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
+ latest version packaged

+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
  f25ad639717d7411a58f10a9e378d7db  v4l-utils-0.8.3.tar.bz2
+ package successfully builds on at least one architecture
  tested using koji scratch build
  http://koji.fedoraproject.org/koji/taskinfo?taskID=2829854
+ BuildRequires list all build dependencies
n/a %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ Package perserves timestamps on install
+ Permissions on files must be set properly 
+ %defattr line
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package runtime 
+ header files should be in -devel
n/a static libraries should be in -static
- non -devel packages should require fully versioned base
+ packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
+ libfoo.so must go in -devel
+ devel must require the fully versioned base
+ packages should not contain libtool .la files
- packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream to include it
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock/koji
+ the package should build into binary RPMs on all supported architectures
n/a review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin
- Package should have man files

Comment 11 Mauro Carvalho Chehab 2011-02-26 12:38:42 UTC
(In reply to comment #10)
> Mostly OK. I think the QT app needs a desktop file (but it might not be an app
> that shoud) and there should be versioned required on the libs at a guess for
> the various utils.
...
> - packages containing GUI apps must include %{name}.desktop file

I've created a desktop file upstream:

http://git.linuxtv.org/v4l-utils.git?a=commitdiff;h=82feef4bea16947f9621cd3061f317af2d4bbf27

The descriptions are currently on English and Portuguese only, but it should be easy to later add other descriptions. It is also providing icons on for the common resolutions found in Fedora, plus a svg icon.

Comment 12 Hans de Goede 2011-03-12 09:36:39 UTC
(In reply to comment #10)
> Mostly OK. I think the QT app needs a desktop file (but it might not be an app
> that shoud) and there should be versioned required on the libs at a guess for
> the various utils.
> 

Thanks for the review, and sorry for the slow reply (I was swamped with work, on a business trip amongst other things.

> - non -devel packages should require fully versioned base
> - packages containing GUI apps must include %{name}.desktop file
> 

Both fixed, here is a new version:
http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.3-2.fc15.src.rpm

Comment 13 Peter Robinson 2011-03-14 06:07:23 UTC
(In reply to comment #12)
> (In reply to comment #10)
> > Mostly OK. I think the QT app needs a desktop file (but it might not be an app
> > that shoud) and there should be versioned required on the libs at a guess for
> > the various utils.
> > 
> 
> Thanks for the review, and sorry for the slow reply (I was swamped with work,
> on a business trip amongst other things.
> 
> > - non -devel packages should require fully versioned base
> > - packages containing GUI apps must include %{name}.desktop file
> > 
> 
> Both fixed, here is a new version:
> http://people.fedoraproject.org/~jwrdegoede/v4l-utils.spec
> http://people.fedoraproject.org/~jwrdegoede/v4l-utils-0.8.3-2.fc15.src.rpm

Build fails. You need to add a dep on desktop-file-utils for the desktop file processing.

http://koji.fedoraproject.org/koji/getfile?taskID=2910226&name=build.log

But other than that it all looks OK so APPROVED!

Comment 14 Hans de Goede 2011-03-14 08:13:09 UTC
(In reply to comment #13)
> Build fails. You need to add a dep on desktop-file-utils for the desktop file
> processing.

Oh, duh, thanks for catching this. And thanks for the review!

Comment 15 Hans de Goede 2011-03-14 08:14:21 UTC
New Package SCM Request
=======================
Package Name: v4l-utils
Short Description: Utilities for video4linux and DVB devices
Owners: jwrdegoede
Branches: f14 f15
InitialCC:

Comment 16 Jason Tibbitts 2011-03-14 15:37:39 UTC
Git done (by process-git-requests).

Comment 17 Fedora Update System 2011-03-15 08:13:05 UTC
v4l-utils-0.8.3-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/v4l-utils-0.8.3-2.fc15

Comment 18 Fedora Update System 2011-03-15 08:14:34 UTC
v4l-utils-0.8.3-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/v4l-utils-0.8.3-2.fc14

Comment 19 Hans de Goede 2011-03-15 12:05:47 UTC
Package Change Request
======================
Package Name: v4l-utils
New Branches: el6
Owners: mchehab
InitialCC: jwrdegoede

Hi,

Mauro (mchehab) would like to see an epel version of v4l-utils, and is willing
to maintain it. This is fine with me.

Regards,

Hans

Comment 20 Jason Tibbitts 2011-03-15 15:24:39 UTC
Git done (by process-git-requests).

Comment 21 Fedora Update System 2011-03-15 17:06:37 UTC
v4l-utils-0.8.3-2.fc15 has been pushed to the Fedora 15 testing repository.

Comment 22 Fedora Update System 2011-03-21 03:28:22 UTC
v4l-utils-0.8.3-2.fc15 has been pushed to the Fedora 15 stable repository.

Comment 23 Fedora Update System 2011-03-23 22:51:29 UTC
v4l-utils-0.8.3-2.fc14 has been pushed to the Fedora 14 stable repository.


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