Bug 226101 - Merge Review: lm_sensors
Merge Review: lm_sensors
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Karel Klíč
Fedora Package Reviews List
:
: 197864 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:34 EST by Nobody's working on this, feel free to take it
Modified: 2013-03-03 17:59 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-12-17 10:37:34 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kklic: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:34:37 EST
Fedora Merge Review: lm_sensors

http://cvs.fedora.redhat.com/viewcvs/devel/lm_sensors/
Initial Owner: pknirsch@redhat.com
Comment 1 Hans de Goede 2007-04-05 15:50:09 EDT
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

Comment 2 Hans de Goede 2007-04-05 15:50:57 EDT
*** Bug 197864 has been marked as a duplicate of this bug. ***
Comment 3 Hans de Goede 2007-11-14 10:14:45 EST
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.
Comment 4 Karel Klíč 2009-12-09 14:10:21 EST
[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@GLIBC_2.0
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
Comment 5 Hans de Goede 2009-12-10 14:05:25 EST
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@GLIBC_2.0
> 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
Comment 6 Nikola Pajkovsky 2009-12-10 16:01:49 EST
Hey Hans,

  sure I will tomorrow ;)
Comment 7 Nikola Pajkovsky 2009-12-17 08:45:10 EST
(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@GLIBC_2.0
> > 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
Comment 8 Karel Klíč 2009-12-17 10:35:57 EST
I am giving fedora-review+. Thanks.

Note You need to log in before you can comment on or make changes to this bug.