Bug 924333 - Review Request: mate-sensors-applet - MATE panel applet for hardware sensors
Summary: Review Request: mate-sensors-applet - MATE panel applet for hardware sensors
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dan Mashal
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-03-21 14:57 UTC by Wolfgang Ulbrich
Modified: 2013-05-06 06:18 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-05-02 04:35:42 UTC
dan.mashal: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
autogen.sh output (9.97 KB, text/plain)
2013-04-26 17:54 UTC, Anthony Name
no flags Details
output of make (48.79 KB, text/plain)
2013-04-26 17:54 UTC, Anthony Name
no flags Details

Description Wolfgang Ulbrich 2013-03-21 14:57:18 UTC
Spec URL: http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec
SRPM URL: http://raveit65.fedorapeople.org/Mate/SRPM/mate-sensors-applet-1.5.1-2.fc20.src.rpm
Description: MATE Sensors Applet is an applet for the MATE Panel to display readings from hardware sensors, including CPU and system temperatures, fan speeds and voltage readings under Linux.
Can interface via the Linux kernel i2c modules, or the i8k kernel module (for
Dell Inspirion 8000 Laptops).
Includes a simple, yet highly customization display and intuitive 
user-interface.
Alarms can be set for each sensor to notify the user once a certain value
has been reached, and can be configured to execute a given command at given
repeated intervals.
Fedora Account System Username: raveit65

Comment 1 Jens Petersen 2013-04-02 07:15:50 UTC
I ran "fedora-review -b 924333" and see:

[!]: Sources can be downloaded from URI in Source: tag
     Note: Could not download Source0

I guess you mean 1.5/ not 1.4/ in the url.
I think this should be fixed.

# rpmlint mate-sensors-applet mate-sensors-applet-devel
mate-sensors-applet.x86_64: W: undefined-non-weak-symbol /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 plugin_name
mate-sensors-applet.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 /lib64/libgio-2.0.so.0
mate-sensors-applet.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 /lib64/libgobject-2.0.so.0
mate-sensors-applet-devel.x86_64: W: no-documentation
2 packages and 0 specfiles checked; 0 errors, 4 warnings.
# echo 'rpmlint-done:'

May be good to check these, though perhaps they can be waived.

## Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29
## Buildroot used: fedora-rawhide-x86_64

Comment 2 Wolfgang Ulbrich 2013-04-02 10:00:48 UTC
Thanks for you hints.
Are you doing the review?
(In reply to comment #1)
> I ran "fedora-review -b 924333" and see:
> 
> [!]: Sources can be downloaded from URI in Source: tag
>      Note: Could not download Source0
> 
> I guess you mean 1.5/ not 1.4/ in the url.
> I think this should be fixed.
Opps, done
> 
> # rpmlint mate-sensors-applet mate-sensors-applet-devel
> mate-sensors-applet.x86_64: W: undefined-non-weak-symbol
> /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 plugin_name
Do you know a solution to fix this warning?
> mate-sensors-applet.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 /lib64/libgio-2.0.so.0
done
> mate-sensors-applet.x86_64: W: unused-direct-shlib-dependency
> /usr/lib64/libmate-sensors-applet-plugin.so.0.0.0 /lib64/libgobject-2.0.so.0
done
> mate-sensors-applet-devel.x86_64: W: no-documentation
> 2 packages and 0 specfiles checked; 0 errors, 4 warnings.
> # echo 'rpmlint-done:'
> 
> May be good to check these, though perhaps they can be waived.
> 
> ## Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29
> ## Buildroot used: fedora-rawhide-x86_64

Updated source links.

%changelog
* Thu Mar 21 2013 Wolfgang Ulbrich <chat-to-me@raveit.de> - 1.5.2-1
- correct source0 download link
- update to 1.5.2 release
- remove unused-direct-shlib-dependency
- remove upstreamed patch
- switch to use libnotify instead of libmatenotify
- fix bogus date in %%changelog:

Spec URL: http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec
SRPM URL: http://raveit65.fedorapeople.org/Mate/SRPM/mate-sensors-applet-1.5.2-1.fc20.src.rpm

Comment 4 Anthony Name 2013-04-23 17:40:04 UTC
Wolfgang am I correct in assuming that this is now only targetted for release in Fedora Core 20?  (i.e. about Januaryish next year)?

I'm assuming this due to the fc20 suffix in the naming structure.

Comment 5 Wolfgang Ulbrich 2013-04-23 17:55:19 UTC
(In reply to comment #4)
> Wolfgang am I correct in assuming that this is now only targetted for
> release in Fedora Core 20?  (i.e. about Januaryish next year)?
> 
> I'm assuming this due to the fc20 suffix in the naming structure.

Anthony,
no, of course not, i always build against rawhide with mock.
I'm waiting for a reviewer action since 4 weeks.
If you're able to do this, please catch this review and re assign it.

Comment 6 Jens Petersen 2013-04-24 01:12:39 UTC
=====
INFO: Processing bugzilla bug: 924333
INFO: Getting .spec and .srpm Urls from : 924333
INFO:   --> SRPM url: http://raveit65.fedorapeople.org/Mate/SRPM/mate-sensors-applet-**1.6.0**-1.fc20.src.rpm
INFO:   --> Spec url: http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec
INFO: Using review directory: /home/petersen/pkgreview/924333-mate-sensors-applet
INFO: Downloading .spec and .srpm files
INFO: Downloading (Source0): http://pub.mate-desktop.org/releases/1.5/mate-sensors-applet-**1.5.2**.tar.xz
=====

The versions ^^ look a bit weird.

Comment 7 Jens Petersen 2013-04-24 01:14:32 UTC
> http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec

Ah I guess you forgot to upload the new spec file...

Comment 8 Dan Mashal 2013-04-24 01:28:53 UTC
Hi Wolfgang,

We are on 1.6 now can you please update the spec file for 1.6 and let's we'll get this in? :)

Thanks,
Dan

Comment 9 Wolfgang Ulbrich 2013-04-24 07:24:01 UTC
Opps,
links are updated now, ready to rumble.

Comment 11 Dan Mashal 2013-04-25 04:13:32 UTC
Naming: OK
License: OK
Packages installs and works: Yes.
Package compiles on f18 and rawhide: Yes.

f18:
https://koji.fedoraproject.org/koji/taskinfo?taskID=5300293
f20:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5300099

MUST: 

===========
BuildRequires
===========

Add:
1) gtk2-devel
2) gsettings-desktop-schemas-devel
3) rarian-compat (for disabling scrollkeeper)

Drop:

1) dbus-glib-devel (provided by gtk2-devel)
2) drop specific versioning for libatasmart-devel (is this really needed?)

===========
configure flags
===========

MUST:
1) Please add --disable-schemas-compile to the configure flags
2) Is the nvidia flag required? Is there a way to get the aticonfig bit working?

SHOULD:

===========
rpmlint
===========

1) Please fix usage of spaces and tabs in spec file. Preferably use spaces over tabls.


===========
Macros
===========

1) Under postun you can put all of the scriplets under one if/fi section.


===========
Other
===========

Is the sed/libtool command really needed? If so can you submit a pull request upstream so that it's not? If not, why?

Comment 12 Wolfgang Ulbrich 2013-04-25 08:38:39 UTC
(In reply to comment #11)
> Naming: OK
> License: OK
> Packages installs and works: Yes.
> Package compiles on f18 and rawhide: Yes.
> 
> f18:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=5300293
> f20:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=5300099
> 
> MUST: 
> 
> ===========
> BuildRequires
> ===========
> 
> Add:
> 1) gtk2-devel
Both gtk2-devel and dbus-glib-devel are called in configure.ac
GLIB_REQUIRED=2.26.0
GTK_REQUIRED=2.14.0
LIBPANEL_REQUIRED=1.1.0
LIBNOTIFY_REQUIRED=0.7.0
LIBCAIRO_REQUIRED=1.0.4
LIBDBUSGLIB_REQUIRED=0.80
LIBATASMART_REQUIRED=0.16
and gtk2-devel is also provide by dbus-glib-devel.
So what's the different?
But i will change it.
> 2) gsettings-desktop-schemas-devel
Why?
gsettings-desktop-schemas is an collection package of gsettings schemas for gnome.
And the -devel subpackage is only needed for applications who needs the schemas from gsettings-desktop-schemas.
I don't thing that mate-sensor-applet needs gnome gesettings files.
It compiles the gsettings schemas fine without it.
And i don't see this dependency in any debian package from mate upstream.
I know you do this in in a lot of packages.
So pls lighten me up, and explain more detailed why this is needed.

> 3)  (for disabling scrollkeeper)
I use --disable-scrollkeeper and i don't use BR scrollkepper.
And i see no file in /var/scrollkeeper.
IMO, rarian-compat is only needed if a package doesn't compile without
/usr/bin/scrollkeeper-config
which is provide by rarian-compat.
But maybe i'm wrong, pls correct me.

> 
> Drop:
> 
> 1) dbus-glib-devel (provided by gtk2-devel)
> 2) drop specific versioning for libatasmart-devel (is this really needed?)
np, i will remove specific versioning
> 
> ===========
> configure flags
> ===========
> 
> MUST:
> 1) Please add --disable-schemas-compile to the configure flags
will do it.
> 2) Is the nvidia flag required? Is there a way to get the aticonfig bit
> working?
It compiles the nvida part also without using the flag if BR libXNVCtrl-devel is set, i use it so that everybody can see that the package is compiled for nvidia usage.
Why is this a MUST not to do this?
I can't set the aticonfig bit because for this i need rpmfusion.
[root@mother rave]# yum provides */aticonfig
<snip>
xorg-x11-drv-catalyst-12.11-0.3.beta11.fc18.x86_64 : AMD's proprietary driver
                                                   : for ATI graphic cards
Repo        : rpmfusion-nonfree
Übereinstimmung von:
Dateiname     : /usr/bin/aticonfig

xorg-x11-drv-catalyst-13.1-1.fc18.x86_64 : AMD's proprietary driver for ATI
                                         : graphic cards
Repo        : rpmfusion-nonfree-updates
Übereinstimmung von:
Dateiname     : /usr/bin/aticonfig

xorg-x11-drv-catalyst-legacy-13.1-2.fc18.x86_64 : AMD's proprietary driver for
                                                : ATI legacy graphic cards
Repo        : rpmfusion-nonfree-updates
Übereinstimmung von:
Dateiname     : /usr/bin/aticonfig

> 
> SHOULD:
> 
> ===========
> rpmlint
> ===========
> 
> 1) Please fix usage of spaces and tabs in spec file. Preferably use spaces
> over tabls.
will do it.
> 
> 
> ===========
> Macros
> ===========
> 
> 1) Under postun you can put all of the scriplets under one if/fi section.
thx, for the hint.
> 
> 
> ===========
> Other
> ===========
> 
> Is the sed/libtool command really needed? If so can you submit a pull
> request upstream so that it's not? If not, why?
Yes, this is a valid command for fedora's packages to avoid a rpmlint warning.
https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dependency

Comment 13 Dan Mashal 2013-04-25 19:34:43 UTC
(In reply to comment #12)

> Both gtk2-devel and dbus-glib-devel are called in configure.ac
> But i will change it.

Thanks

> > 2) gsettings-desktop-schemas-devel
> Why?
> gsettings-desktop-schemas is an collection package of gsettings schemas for
> gnome.
> And the -devel subpackage is only needed for applications who needs the
> schemas from gsettings-desktop-schemas.
> I don't thing that mate-sensor-applet needs gnome gesettings files.
> It compiles the gsettings schemas fine without it.
> And i don't see this dependency in any debian package from mate upstream.
> I know you do this in in a lot of packages.
> So pls lighten me up, and explain more detailed why this is needed.

You're right, I'll remove that from the rest of the mate packages that don't need it. glib2 provides glib-compile-schemas.

> 
> > 3)  (for disabling scrollkeeper)
> I use --disable-scrollkeeper and i don't use BR scrollkepper.
> And i see no file in /var/scrollkeeper.
> IMO, rarian-compat is only needed if a package doesn't compile without
> /usr/bin/scrollkeeper-config
> which is provide by rarian-compat.
> But maybe i'm wrong, pls correct me.


" Scrollkeeper
In all current Fedora, rarian has replaced scrollkeeper."

I know it may compile without it but I add it out of habit because 9/10 it will fail without it.


> > 2) Is the nvidia flag required? Is there a way to get the aticonfig bit
> > working?
> It compiles the nvida part also without using the flag if BR
> libXNVCtrl-devel is set, i use it so that everybody can see that the package
> is compiled for nvidia usage.
> Why is this a MUST not to do this?

Sorry, that was not a must just a question.

> I can't set the aticonfig bit because for this i need rpmfusion.

OK

> > Is the sed/libtool command really needed? If so can you submit a pull
> > request upstream so that it's not? If not, why?
> Yes, this is a valid command for fedora's packages to avoid a rpmlint
> warning.
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-
> dependency

OK

Comment 14 Wolfgang Ulbrich 2013-04-25 20:55:13 UTC
scratch build at koji.
http://koji.fedoraproject.org/koji/taskinfo?taskID=5303230

%changelog
* Wed Apr 25 2013 Wolfgang Ulbrich <chat-to-me@raveit.de> - 1.6.0-2
- add --disable-schemas-compile configure flag
- organized %%postun scpriptlet section
- droped specific versioning from BR's
- fix usage of spaces and tabs in spec file
- change BR dbus-glib-devel to gtk2-devel

Spec URL: http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec
SRPM URL: http://raveit65.fedorapeople.org/Mate/SRPM/mate-sensors-applet-1.6.0-2.fc20.src.rpm

@ ati
maybe we can create a subpackage at rpmfusion which contains
mate-sensors-applet-ati = (full)mate-sensors-applet - mate-sensors-applet
I'm willing to create and maintain this subpackage, but i have no ati graphics hardware to test it.
Do you have an ati hardware?

PS:
Do you have time to review the other open reviews or should i search for another reviewer?

Comment 15 Dan Mashal 2013-04-25 20:57:42 UTC
Yes I have an ATI Radeon HD 5770 I can test with I will start work on the other reviews.

Comment 16 Dan Mashal 2013-04-26 02:46:04 UTC
Looks good.

Please fix bogus date in change log before import (Thu not Wed).

$ rpmlint mate-sensors-applet.spec
mate-sensors-applet.spec: E: specfile-error warning: bogus date in %changelog: Wed Apr 25 2013 Wolfgang Ulbrich <chat-to-me@raveit.de> - 1.6.0-2
0 packages and 1 specfiles checked; 1 errors, 0 warnings.

APPROVED

Comment 17 Anthony Name 2013-04-26 17:52:16 UTC
Guys

Sorry to be a damp squib......

F18x64, brand new clean install.

Unpacking the src.rpm referenced in comment 20.

after.....

./autogen.sh

make

su -c 'make install'

Logout and log back on to make sure all defaults are reset.

sensors-applet is NOT in the add to panel list (where I'd expect to find it).

output of ./autogen.sh and make attached.

Comment 18 Anthony Name 2013-04-26 17:54:20 UTC
Created attachment 740562 [details]
autogen.sh output

Comment 19 Anthony Name 2013-04-26 17:54:52 UTC
Created attachment 740563 [details]
output of make

Comment 20 Wolfgang Ulbrich 2013-04-26 18:26:12 UTC
Sorry this a package review and not a user help support.
If you forgot to install build dependencies and compile it for yourself, why do you think this can work?
Than it is better for you to rebuild the srpm for testing for f18.
rpmbuild --rebuild <srpm>
http://koji.fedoraproject.org/koji/taskinfo?taskID=5303230
Pls, do not use this review for asking questions how to compile a package!

Comment 21 Wolfgang Ulbrich 2013-04-26 19:35:06 UTC
New Package SCM Request
=======================
Package Name: mate-sensors-applet
Short Description: MATE panel applet for hardware sensors
Owners: raveit65
Branches: f17 f18 f19
InitialCC:

Comment 22 Gwyn Ciesla 2013-04-26 19:44:39 UTC
Git done (by process-git-requests).

Comment 23 Fedora Update System 2013-04-26 21:01:28 UTC
mate-sensors-applet-1.6.0-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-sensors-applet-1.6.0-3.fc18

Comment 24 Fedora Update System 2013-04-26 21:02:04 UTC
mate-sensors-applet-1.6.0-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-sensors-applet-1.6.0-3.fc17

Comment 25 Fedora Update System 2013-04-26 21:02:17 UTC
mate-sensors-applet-1.6.0-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/mate-sensors-applet-1.6.0-3.fc19

Comment 26 Fedora Update System 2013-04-27 17:28:00 UTC
mate-sensors-applet-1.6.0-3.fc19 has been pushed to the Fedora 19 testing repository.

Comment 27 Fedora Update System 2013-05-02 04:35:44 UTC
mate-sensors-applet-1.6.0-3.fc19 has been pushed to the Fedora 19 stable repository.

Comment 28 Fedora Update System 2013-05-06 03:50:44 UTC
mate-sensors-applet-1.6.0-3.fc17 has been pushed to the Fedora 17 stable repository.

Comment 29 Fedora Update System 2013-05-06 03:51:44 UTC
mate-sensors-applet-1.6.0-3.fc18 has been pushed to the Fedora 18 stable repository.


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