Bug 246748

- Please use a full source url

- You probably need some requires(post) (pre), etc for
  ldconfig, useradd, etc

- I don't think it is a good idea to use / as the homedir, the wiki says: 
    should usually be a directory created and owned by the package, with 
    appropriately restrictive permissions. One good choice for the location of the 
    directory is the package's data directory in case it has one.

- have you requested an official user id for ohm ?

- please don't mix %{buildroot} and $RPM_BUILD_ROOT in one .spec file

- %dir %{_sysconfdir}/dbus-1/system.d
  should certainly be owned by dbus, no ?

- are libohm.so.* shared libs or modules ? if they are dlopened, they
  should better live in /usr/lib/ohm
 

Created attachment 158616 [details]
new spec file

Attached is a new spec file with all the changes.

libohm_*.so.* are dlopened modules so I've made then un-versioned and put them
in /usr/lib/ohm/

Richard.

It misses at least

BuildRequires: perl(XML::Parser)
BuildRequires: dbus-glib-devel 

Still trying to get it to build in mock

one more:

BuildRequires: hal-devel

Ok, with these, I finally got it to build in mock. 

Initial impression from the built packages:

/usr/share/ohm-0.1.1/index.html

This looks unusual. Typically, datadirs are not versioned, and
if index.html is documentation, it should probably go into
/usr/share/doc/ohm-0.1.1 instead. The way to do that is to add

%doc index.html 

to the file list

Translations need to be handled with %find_lang

Created attachment 158629 [details]
new spec file

Here you go, this should be everything okay. Cheers for the quick review.

rpmlint warns about

W: ohm incoherent-version-in-changelog 0.1.1-0.fc7.hughsie 0.1.1-0.fc8

and it might be a nice idea to add the LSB header to the initscript, to make
Harald feel warm and fuzzy...

Apart from that, I had to do a few more specfile tweaks to get it to build in mock:

Add a 

BuildRequires: libtool

Add 

rm -f %{buildroot}%{_datadir}/ohm-%{version}/index.html

Otherwise mock complains about an unpackaged file.

What about the following? Comments?

### BEGIN INIT INFO
# Provides:          ohmd
# Required-Start:    haldaemon
# Required-Stop:     
# Should-Start:	     
# Should-Stop:	     
# Default-Start:     2 3 4 5
# Default-Stop:      1 6
# Short-Description: Loads OHM Open Hardware Manager
# Description:       Loads Open Hardware Manager to enforce power policy on
#                    embedded and small form factor devices.
### END INIT INFO

Looks fine to me. You can omit the empty fields. And I think you probably want to
add 0 to Default-Stop, not sure if it makes any difference.

How about http://people.freedesktop.org/~hughsient/temp/ohm-0.1.1-0.fc7.src.rpm

Should be okay, and seems to build in mock now.

rpmlint output:

[mclasen@localhost Desktop]$ rpmlint
/var/lib/mock/fedora-development-i386/result/ohm-0.1.1-0.fc8.i386.rpm 
W: ohm service-default-enabled /etc/rc.d/init.d/ohmd
[mclasen@localhost Desktop]$ rpmlint
/var/lib/mock/fedora-development-i386/result/ohm-devel-0.1.1-0.fc8.i386.rpm 
W: ohm-devel no-documentation

both are ignorable, imo

package name: ok
spec file name: ok
packaging guidelines: ok
license: ok
license file: ok
license file included: ok
spec file language: ok
spec file readability: excellent
upstream sources: ok
buildable: ok
BRs: ok
locale handling: ok
ldconfig: ok
relocatable: n/a
directory ownership: BAD, -devel must require pkgconfig for /usr/lib/pkgconfig
file list dupes: ok
defattr: ok
%clean: ok
macro use: ok
permissible content: ok
doc package: n/a
%doc: ok
headers: ok
static libs: n/a
.pc files:  BAD, see above
shared libs: ok
-devel requires base: ok
la files: ok
desktop file: n/a
directory ownership: ok
%install: BAD, must do rm -rf %{buildroot} at the beginning of %install
utf8 filenames: ok


Two mustfix items, and one whishlist item, then you are good to go:

- Must require pkgconfig in -devel
- Must clean buildroot in %install
- Should include AUTHORS and README in %doc  

Cool, thanks for that. I've fixed all the issues you mentioned.

New SRPM here: http://people.freedesktop.org/~hughsient/temp/ohm-0.1.1-0.fc7.src.rpm

Ok, approved.

Package Name: ohm
Short Description: Open Hardware Manager
Owners: richard,johnp,mpg
Branches: F-7 OLPC-2
InitialCC: richard,johnp,mpg

cvs done.