Bug 924333

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

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> - 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

Update for MATE 1.6 release

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-1.fc20.src.rpm

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.

(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.

=====
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.

> http://raveit65.fedorapeople.org/Mate/SPECS/mate-sensors-applet.spec

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

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

Opps,
links are updated now, ready to rumble.

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-1.fc20.src.rpm

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?

(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

(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

scratch build at koji.
http://koji.fedoraproject.org/koji/taskinfo?taskID=5303230

%changelog
* Wed Apr 25 2013 Wolfgang Ulbrich <chat-to-me> - 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?

Yes I have an ATI Radeon HD 5770 I can test with I will start work on the other reviews.

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> - 1.6.0-2
0 packages and 1 specfiles checked; 1 errors, 0 warnings.

APPROVED

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.

Created attachment 740562 [details]
autogen.sh output

Created attachment 740563 [details]
output of make

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!

New Package SCM Request
=======================
Package Name: mate-sensors-applet
Short Description: MATE panel applet for hardware sensors
Owners: raveit65
Branches: f17 f18 f19
InitialCC:

Git done (by process-git-requests).

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

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

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

mate-sensors-applet-1.6.0-3.fc19 has been pushed to the Fedora 19 testing repository.

mate-sensors-applet-1.6.0-3.fc19 has been pushed to the Fedora 19 stable repository.

mate-sensors-applet-1.6.0-3.fc17 has been pushed to the Fedora 17 stable repository.

mate-sensors-applet-1.6.0-3.fc18 has been pushed to the Fedora 18 stable repository.