Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226101 - Merge Review: lm_sensors
Summary: Merge Review: lm_sensors
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Karel Klíč
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
: 197864 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:34 UTC by Nobody's working on this, feel free to take it
Modified: 2013-03-03 22:59 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-12-17 15:37:34 UTC
kklic: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:34:37 UTC
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 19:50:09 UTC
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 19:50:57 UTC
*** Bug 197864 has been marked as a duplicate of this bug. ***

Comment 3 Hans de Goede 2007-11-14 15:14:45 UTC
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 19:10:21 UTC
[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 19:05:25 UTC
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 21:01:49 UTC
Hey Hans,

  sure I will tomorrow ;)

Comment 7 Nikola Pajkovsky 2009-12-17 13:45:10 UTC
(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 15:35:57 UTC
I am giving fedora-review+. Thanks.


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