[Bug 243187] Review Request: edac-utils - user space utilities for EDAC subsystem

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: edac-utils - user space utilities for EDAC subsystem


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





------- Additional Comments From jwilson at redhat.com  2007-06-07 15:31 EST -------
Suggested changes from initial pass through the spec:

1) Use the %{?dist} tag in the Release: field

2) Typical format for sf.net downloads is to use dl.sf.net, i.e.:
http://dl.sourceforge.net/sourceforge/edac-utils/%{name}-%{version}.tar.bz2

3) Remove the CFLAGS= from the make line, its superfluous, as it gets set and
exported by the %configure macro, and make is picking that up fine.

4) Be consistent with use of spaces or tabs for indentation (see %post and %preun).

5) %preun should probably try stopping the edac service before removing the
initscript (although not having looked at the exact nature of the initscript,
I'm guessing this may be essentially a no-op anyhow).

6) The .so and .h files really should go in an edac-utils-devel sub-package.

7) ldconfig needs to be run in %post and %postun

8) static libs and libtool archives shouldn't be installed

9) _smp_mflags should be prefixed with a ? so it doesn't cause build errors
and/or spew on systems that don't have it defined

10) ideally, rpmlint *should* be silent. I've patched up the spec with changes
for all of the above, but its still complaining a bit about the initscript:
$ rpmlint /build/RPMS/x86_64/edac-utils-*
W: edac-utils service-default-enabled /etc/init.d/edac
E: edac-utils missing-mandatory-lsb-tag Short-Description
W: edac-utils service-default-enabled /etc/init.d/edac
W: edac-utils no-reload-entry /etc/init.d/edac
E: edac-utils subsys-not-used /etc/init.d/edac
W: edac-utils incoherent-init-script-name edac
W: edac-utils-devel no-documentation
W: edac-utils-devel no-dependency-on edac-utils

For the most part, the W: bits are fine, but it'd be good to address the two
E:'s there. Both are relatively simple to fix.

Index: edac-utils.spec
===================================================================
RCS file: /cvs/dist/rpms/edac-utils/devel/edac-utils.spec,v
retrieving revision 1.4
diff -u -p -r1.4 edac-utils.spec
--- edac-utils.spec     6 Jun 2007 20:10:50 -0000       1.4
+++ edac-utils.spec     7 Jun 2007 19:31:05 -0000
@@ -1,12 +1,12 @@
 Name:          edac-utils
 Version:       0.9
-Release:       1
+Release:       1%{?dist}
 Summary:       Userspace helper for kernel EDAC drivers
 
 Group:         System Environment/Base
 License:       GPL
 URL:           http://edac-utils.sourceforge.net/
-Source0:      
http://prdownloads.sourceforge.net/edac-utils/%{name}-%{version}.tar.bz2
+Source0:      
http://dl.sourceforge.net/sourceforge/edac-utils/%{name}-%{version}.tar.bz2
 Patch0:                edac_ctl-remove_driver_loading.patch
 Patch1:                edac_init-fix_messages.patch
 
@@ -23,44 +23,62 @@ of an init script which makes sure EDAC 
 are loaded at system startup, as well as a library and utility
 for reporting current error counts from the EDAC sysfs files.
 
+%package devel
+Summary:       Development files for %{name}
+Group:         Development/Libraries
+
+%description devel
+This package contains the development headers and libraries
+for %{name}.
+
 %prep
 %setup -q
 %patch0 -p1
 %patch1 -p1
 
 %build
-%configure
-make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS"
+%configure --disable-static
+make %{?_smp_mflags} 
 
 %install
 rm -rf $RPM_BUILD_ROOT
 make install-exec install-data DESTDIR="$RPM_BUILD_ROOT"
+# Remove libtool archive
+rm -f $RPM_BUILD_ROOT/%{_libdir}/*.la
 
 %clean
 rm -rf $RPM_BUILD_ROOT
 
 %post
+/sbin/ldconfig
 if [ $1 = 1 ]; then
-  /sbin/chkconfig --add edac
+       /sbin/chkconfig --add edac
 fi
 
 %preun
 if [ $1 = 0 ]; then
+       /sbin/service edac stop >/dev/null 2>&1 || :
        /sbin/chkconfig --del edac
 fi
 
+%postun -p /sbin/ldconfig
+
 %files 
-%defattr(-,root,root,0755)
+%defattr(-,root,root,-)
 %doc README NEWS ChangeLog DISCLAIMER
 %{_sbindir}/edac-ctl
 %{_bindir}/edac-util
-%{_libdir}/*
+%{_libdir}/*.so.*
 %{_mandir}/*/*
-%{_includedir}/edac.h
 %dir %attr(0755,root,root) %{_sysconfdir}/edac
 %config(noreplace) %{_sysconfdir}/edac/*
 %{_sysconfdir}/init.d/edac
 
+%files devel
+%defattr(-,root,root,-)
+%{_libdir}/*.so
+%{_includedir}/edac.h
+
 %changelog
 * Wed Jun 06 2007 Aristeu Rozanski <arozansk at redhat.com> 0.9-1
 - Updated version to 0.9, separate project now


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.