Bug 226101

Some intial comments, taken from a bug I've filed against lm_sensors long ago:

2 small packaging issues:
1) lm_sensors-devel ships a static lib, afaik shipping static libs is concidered
  deprecated and should no longer be done unless there is a specific reason (like
  initrd needing it)
2) /usr/share/man/man3/libsensors.3.gz must be in lm_sensors-devel not in 
  lm_sensors

*** Bug 197864 has been marked as a duplicate of this bug. ***

To all interested reviewers, I've become a lm_sensors co-maitainer recently and
I would like to push gnome-games through its merge review. I've taken an initial
look and the specfile looks ok. Please review and tell me what needs fixing.

[YES] source files match upstream: 613d7cfa23b70c0abae3fabb0a72ff5f  lm_sensors-3.1.1.tar.bz2
[YES] package meets naming and versioning guidelines 
      (lm_sensors has an exception that it can use underscore in its name)
[YES] specfile is properly named
[YES] specfile is cleanly written 
[NO] specfile uses macros consistently: %{SOURCEx} should probably be %{sourcex}, "Buildroot:" -> "BuildRoot:"
[YES] dist tag is present
[YES] build root is correct
[YES] license field matches the actual license
[YES] license is open source-compatible
[YES] license text included in package
[YES] latest version is being packaged
[YES] BuildRequires are proper
[YES] compiler flags are appropriate
[YES] %clean is present
[YES] package builds in mock
[YES] debuginfo package looks complete
[NO] rpmlint is silent

$ rpmlint *.rpm
lm_sensors.i686: W: dangerous-command-in-%pre mv
lm_sensors.i686: W: dangerous-command-in-%trigger mv
lm_sensors.i686: W: dangerous-command-in-%trigger mv
lm_sensors.i686: W: one-line-command-in-%trigger /usr/bin/sysconfig-lm_sensors-convert
lm_sensors-libs.i686: W: summary-not-capitalized lm_sensors core libraries
lm_sensors-libs.i686: W: shared-lib-calls-exit /usr/lib/libsensors.so.4.2.0 exit
lm_sensors-libs.i686: W: no-documentation
lm_sensors-libs.i686: E: library-without-ldconfig-postin /usr/lib/libsensors.so.4.2.0
lm_sensors-libs.i686: E: library-without-ldconfig-postun /usr/lib/libsensors.so.4.2.0
lm_sensors-sensord.i686: E: incoherent-subsys /etc/rc.d/init.d/sensord lm_sensors
lm_sensors-sensord.i686: W: incoherent-init-script-name sensord ('lm_sensors-sensord', 'lm_sensors-sensordd')
6 packages and 0 specfiles checked; 3 errors, 8 warnings.

Imho the following lines should be added to the spec file:
%post libs -p /sbin/ldconfig
%postun libs -p /sbin/ldconfig


[YES] final provides and requires look sane
[???] Please consider using "Requires: dmidecode" instead of "Requires: /usr/sbin/dmidecode"
[OK] %check is not present
[YES] no shared libraries are added to the regular linker search paths in app package
[YES] owns the directories it creates (no dirs)
[YES] doesn't own any directories it shouldn't
[YES] no duplicates in %files
[YES] file permissions are appropriate
[???] %defattr(-,root,root,-) should be used instead of %defattr(-,root,root)
[YES] scriptlets ok
[YES] code, not content
[YES] documentation is small, so no -docs subpackage is necessary
[YES] %docs are not necessary for the proper functioning of the package
[YES] no headers
[YES] no pkgconfig files
[YES] no libtool .la droppings
[YES] not a GUI app

npajkovs, may I assume you will take care if this ?

As for the review, Thanks!

Here is my take on things which need fixing:

(In reply to comment #4)
> [NO] specfile uses macros consistently: %{SOURCEx} should probably be
> %{sourcex}, "Buildroot:" -> "BuildRoot:"

Writing SOURCE with all caps is quite normal in spec files (most do
it this way), and is allowed as long as it is in all caps everywhere
inside the specfile, which it is.

The buildroot thingie should be fixed.
> [NO] rpmlint is silent
> 
> $ rpmlint *.rpm
> lm_sensors.i686: W: dangerous-command-in-%pre mv
> lm_sensors.i686: W: dangerous-command-in-%trigger mv
> lm_sensors.i686: W: dangerous-command-in-%trigger mv
> lm_sensors.i686: W: one-line-command-in-%trigger
> /usr/bin/sysconfig-lm_sensors-convert

These can all be ignored

> lm_sensors-libs.i686: W: summary-not-capitalized lm_sensors core libraries

Should be fixed

> lm_sensors-libs.i686: W: shared-lib-calls-exit /usr/lib/libsensors.so.4.2.0
> exit
> lm_sensors-libs.i686: W: no-documentation

Can be ignored
> lm_sensors-libs.i686: E: library-without-ldconfig-postin
> /usr/lib/libsensors.so.4.2.0
> lm_sensors-libs.i686: E: library-without-ldconfig-postun

Oops, see below.

> /usr/lib/libsensors.so.4.2.0
> lm_sensors-sensord.i686: E: incoherent-subsys /etc/rc.d/init.d/sensord
> lm_sensors
> lm_sensors-sensord.i686: W: incoherent-init-script-name sensord
> ('lm_sensors-sensord', 'lm_sensors-sensordd')

Can be ignored.

> Imho the following lines should be added to the spec file:
> %post libs -p /sbin/ldconfig
> %postun libs -p /sbin/ldconfig
> 

Correct, and the ld_config from the main package %post should removed

And the main package's:
%postun -p /sbin/ldconfig

Should be removed completely.

> [???] Please consider using "Requires: dmidecode" instead of "Requires:
> /usr/sbin/dmidecode"

No need to not use file requires when the files are in one of /bin, /sbin,
/usr/bin, /usr/sbin.

> [???] %defattr(-,root,root,-) should be used instead of %defattr(-,root,root)

Can / should be fixed.

Regards,

Hans

Hey Hans,

  sure I will tomorrow ;)

(In reply to comment #5)
> npajkovs, may I assume you will take care if this ?
> 
> As for the review, Thanks!
> 
> Here is my take on things which need fixing:
> 
> (In reply to comment #4)
> > [NO] specfile uses macros consistently: %{SOURCEx} should probably be
> > %{sourcex}, "Buildroot:" -> "BuildRoot:"
> 
> Writing SOURCE with all caps is quite normal in spec files (most do
> it this way), and is allowed as long as it is in all caps everywhere
> inside the specfile, which it is.
> 
> The buildroot thingie should be fixed.
> > [NO] rpmlint is silent
> > 
> > $ rpmlint *.rpm
> > lm_sensors.i686: W: dangerous-command-in-%pre mv
> > lm_sensors.i686: W: dangerous-command-in-%trigger mv
> > lm_sensors.i686: W: dangerous-command-in-%trigger mv
> > lm_sensors.i686: W: one-line-command-in-%trigger
> > /usr/bin/sysconfig-lm_sensors-convert
> 
> These can all be ignored
> 
ignored

> > lm_sensors-libs.i686: W: summary-not-capitalized lm_sensors core libraries
> 
> Should be fixed
> 
fixed

> > lm_sensors-libs.i686: W: shared-lib-calls-exit /usr/lib/libsensors.so.4.2.0
> > exit
> > lm_sensors-libs.i686: W: no-documentation
> 
> Can be ignored

ignored

> > lm_sensors-libs.i686: E: library-without-ldconfig-postin
> > /usr/lib/libsensors.so.4.2.0
> > lm_sensors-libs.i686: E: library-without-ldconfig-postun
> 
fixed

> Oops, see below.
> 
> > /usr/lib/libsensors.so.4.2.0
> > lm_sensors-sensord.i686: E: incoherent-subsys /etc/rc.d/init.d/sensord
> > lm_sensors
> > lm_sensors-sensord.i686: W: incoherent-init-script-name sensord
> > ('lm_sensors-sensord', 'lm_sensors-sensordd')
> 
> Can be ignored.
ignored. we will use same name as upstream

> > Imho the following lines should be added to the spec file:
> > %post libs -p /sbin/ldconfig
> > %postun libs -p /sbin/ldconfig
> > 
> 
> Correct, and the ld_config from the main package %post should removed
> 
> And the main package's:
> %postun -p /sbin/ldconfig
> 
> Should be removed completely.
> 
fixed

> > [???] Please consider using "Requires: dmidecode" instead of "Requires:
> > /usr/sbin/dmidecode"
> 
> No need to not use file requires when the files are in one of /bin, /sbin,
> /usr/bin, /usr/sbin.
> 
ignored

> > [???] %defattr(-,root,root,-) should be used instead of %defattr(-,root,root)
> 
> Can / should be fixed.
> 
fixed

> Regards,
> 
> Hans

I am giving fedora-review+. Thanks.