Bug 226577

Well, this the first driver (alphabetically) which will build on x86_64, so I
figured I'd take a look.

A cursory inspection only turns up two issues:

W: xorg-x11-drv-apm invalid-license MIT/X11
  which is kind of an odd way to state it, but I guess it's not incorrect.

The other issue is ownership of /usr/lib64/xorg/modules and
/usr/lib64/xorg/modules/drivers.  I thought there was a discussion at Fudcon
about this between you and spot, but I paid only enough attention to look out
for airborne furniture.

Minor stuff:

%define cvsdate xxxxxxx
  Not used in this spec, and aren't these things in git anyway?

Jesse stuck in a weird changelog line:
  * Wed Jul 12 2006 Jesse Keating <jkeating> - sh: line 0: fg: no job
control
Looks like a script took a bit of a dump there.

Umm, I guess that's about it.  Needless to say, I have absolutely no way to test
this; I'm not even sure what "apm" means in the context of video.

Alliance ProMotion.  You don't have one, no one does.  Even _I_ don't, and I've
got nearly everything.

I was eventually convinced of the directory ownership thing; server owns,
drivers don't.  The license should be "MIT" instead, and the cvsdate thing was
just boilerplate from the initial creation.

The changelog thing, yeah, bad script, not my fault.

Fixed all but the changelog thing in 1.1.1-4 since I don't know what %revision
that corresponded to.  Once this passes review I'll mass-update all the other
drivers, which should make subsequent reviews more automatic.

I'll take a closer look later today.

OK, I pulled the latest source and built.  This BS rpmlint complaint squeaked in:
  W: xorg-x11-drv-apm mixed-use-of-spaces-and-tabs (spaces: line 5, tab: line 3)
I usually just ignore these.

Blame Jesse for the bogus changelog, I guess.  It's certainly not anything to
block on.

I recall from the vermillion review that you decided these drivers should grow a
dependency on hwdata.

At first I was going to complain about the COPYING file not getting into the
final package, but then I read the file:
  This is a stub file.  This package has not yet had its complete licensing
  information compiled.
so I don't suppose there would be much point.

I'm kind of surprised xorg-x11-server-sdk doesn't have a dependency on
pkgconfig.  It looks like it should, since it included a .pc file.  That would
mean that you don't need to have the separate build-time dependency in all of
these drivers.

So really this is fine except for /usr/share/hwdata/videoaliases being unowned.

* source files match upstream:
   09d7e6cf30b94f141f8ebe2560b301a058f645e74108edc28c908e750865dcec  
   xf86-video-apm-1.1.1.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint has only ignoreable errors.
* final provides and requires are sane:
   apm_drv.so()(64bit)
   xorg-x11-drv-apm = 1.1.1-4.fc7
  =
   xorg-x11-server-Xorg >= 1.0.99.901
* %check is not present; not possible to automatically test, and hardware to
  test this is hard to come by in any case.
* no shared libraries are added to the regular linker search paths.
X places a file in /usr/share/hwdata/videoaliases but doesn't own it or depend 
  on anything that does.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la droppings.

I fixed xorg-x11-server-sdk 1.3.0.0-6 to Requires: pkgconfig, so now this driver
doesn't have to.  Also added a requires on hwdata.

So, 1.1.1-5 should be golden.

Yes, indeed, I think everything's good now.

APPROVED

Just ping (email, IRC, whatever) when you think the other drivers are ready and
I'll start going through them.

Can we close this?

Yes we can!