This looks pretty much like it can be closed. There are four little points we
should be changed:

* change buildroot to be guidelines compliant
* add comment why %{?_smp_mflags} cannot be used
* add comment why make install DESTDIR=%{buildroot} cannot be used
* add noreplace to the config (to silence rpmlint)

--- fbset.spec  18 Jan 2007 15:37:11 -0000      1.21
+++ fbset.spec  29 Jun 2007 12:24:40 -0000
@@ -10,7 +10,7 @@
 Patch0: fbset-2.1-makefile.patch
 Patch1: fbset-2.1-fixmode.patch
 Patch2: fbset-2.1-manfix.patch
-BuildRoot: %{_tmppath}/%{name}-root
+BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
 ExcludeArch: s390 s390x
 
 %description
@@ -27,11 +27,13 @@
 %patch2 -p1 -b .man
 
 %build
+# %{?_smp_mflags} breaks building
 make CFLAGS="$RPM_OPT_FLAGS"
 
 %install
 rm -rf ${RPM_BUILD_ROOT}
 
+# make install DESTDIR=%{buildroot} does not work here
 %makeinstall
 
 %clean
@@ -41,7 +43,7 @@
 %defattr(-,root,root)
 %{_sbindir}/*
 %{_mandir}/man[58]/*
-%config        %{_sysconfdir}/fb.modes
+%config(noreplace) %{_sysconfdir}/fb.modes
 
 %changelog
 * Thu Jan 18 2007 Jindrich Novy <jnovy> - 2.1-24

Zdenek Prikryl is the owner according the PackageDB.  Added as cc.

(In reply to comment #1)
> * change buildroot to be guidelines compliant

Fixed.

> * add comment why %{?_smp_mflags} cannot be used

fbset doesn't use autotool's stuff, it is written by hand. So, you have to use CFLAGS variable to pass additional options to the gcc.

> * add comment why make install DESTDIR=%{buildroot} cannot be used

This is a similar problem like above. In the makefile, there is no such variable. There is a $(PREFIX) variable which is set by %makeinstall

> * add noreplace to the config (to silence rpmlint)

Fixed.

I committed the new spec into cvs. Please, review it, so I can bump a new release.

Zdenek

There is something in the guidelines about %makeinstall:

http://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

The right thing, to conform with the guidelines would be to use:

make install sysconfdir=%{buildroot}%{_sysconfdir} sbindir=%{buildroot}%{_sbindir} mandir=%{buildroot}%{_mandir}

Although, that makes it pretty unreadable. I would say it is up to you which one you like better.

And I have seen this:

MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line.

There is tracker bug for s390 at:
https://bugzilla.redhat.com/show_bug.cgi?id=467765

Please create a bug blocking that one and mention the bug near the ExcludeArch line.

(In reply to comment #4)
> There is something in the guidelines about %makeinstall:
> 
> http://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
> 

Since "make install DESTDIR=%{buildroot}" doesn't work and fbset doesn't use libtool or autotools stuff, I'd not remove the %makeinstall macro.

> And I have seen this:
> 
> MUST: If the package does not successfully compile, build or work on an
> architecture, then those architectures should be listed in the spec in
> ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in
> bugzilla, describing the reason that the package does not compile/build/work on
> that architecture. The bug number MUST be placed in a comment, next to the
> corresponding ExcludeArch line.
> 

I created the bug and write ID to the spec file. Check cvs.

I am just mentioning the ExcludeArch bug here:

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

Rest looks good.

Source matches upstream.
Patches look good.

I am not 100% happy with the %makeinstall, but it seems that it creates no problems in this case. So:

APPROVED.