Bug 847419 (mate-polkit)
| Summary: | Review request: mate-polkit - Integrates polkit with the MATE Desktop environment | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Dan Mashal <dan.mashal> |
| Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | misc, notting, package-review, rdieter |
| Target Milestone: | --- | Flags: | rdieter:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2012-09-02 00:28:23 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 840149 | ||
|
Description
Dan Mashal
2012-08-11 00:58:26 UTC
I can review 1. SHOULD remove
BuildRequires: gtk+-devel
it's not needed
naming: ok
licensing: ok
sources: ok
md5sum *.xz
7eccae36af5f9ee0e38a569fcd34e395 mate-polkit-1.4.0.tar.xz
macros: ok
scriptlets: mostly ok
SHOULD: remove
update-desktop-database &> /dev/null || :
scriptlets, not needed here.
2. -devel MUST own %{_includedir}/polkit-gtk-mate-1/ (and polkitgtkmate subdir)
imo, replace
%{_includedir}/polkit-gtk-mate-1/polkitgtkmate/polkitgtkmate.h
%{_includedir}/polkit-gtk-mate-1/polkitgtkmate/polkitgtkmatetypes.h
%{_includedir}/polkit-gtk-mate-1/polkitgtkmate/polkitlockbutton.h
with just
%{_includedir}/polkit-gtk-mate-1/
3. SHOULD drop dup'd
%doc
statement in -devel subpkg
4. SHOULD add
Provides: PolicyKit-authentication-agent
to match other policy kit providers (like gnome-polkit, polkit-kde)
Fixed: Spec URL: http://vicodan.fedorapeople.org/matespec/mate-polkit.spec SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-polkit-1.4.0-2.fc17.src.rpm there's still an extra update-desktop-database &> /dev/null || : scriptlet, please remove that before doing any builds. APPROVED. Fixed that. Thanks Rex. New Package SCM Request ======================= Package Name: mate-polkit Short Description: Integrates polkit with the MATE Desktop environment Owners: vicodan rdieter raveit65 Branches: f16 f17 18 InitialCC: pls use this before you import.
Provides: polkit-mate-authentication-agent-1
Fedora switch to polkit a long time ago.
And for -devel subpackage
Provides: mate-polkit-demo%{?_isa} = %{version}-%{release}
For the 2 provides you suggest adding, please explain why you think they are needed. There is still an outstanding issue, move this to -devel where it belongs
%{_datadir}/gir-1.0/PolkitGtkMate-1.0.gir
(In reply to comment #8) > For the 2 provides you suggest adding, please explain why you think they are > needed. If i read this request , you are one wanted to add 4. SHOULD add Provides: PolicyKit-authentication-agent to match other policy kit providers (like gnome-polkit, polkit-kde) I use this also in my spec files, but i use polkit because current releases use polkit not policykit. Maybe yum will nevertheless find polkit-mate-authentication-agent-1 with provide 'PolicyKit-authentication-agent', I can not estimate this. If you say this will work......... Forget the other provide in -devel, no need for it. Git done (by process-git-requests). Corrected branch name. mate-polkit-1.4.0-2.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/mate-polkit-1.4.0-2.fc16 mate-polkit-1.4.0-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/mate-polkit-1.4.0-2.fc18 mate-polkit-1.4.0-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/mate-polkit-1.4.0-2.fc17 mate-polkit-1.4.0-2.fc18 has been pushed to the Fedora 18 testing repository. >leigh scott 2012-08-19 06:12:53 PDT
>There is still an outstanding issue, move this to -devel where it belongs
>%{_datadir}/gir-1.0/PolkitGtkMate-1.0.gir
Done.
The review didn't catch some issues : the same multiple license issues problem as others, I will not repeat what i said in others bug reports : https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios in fact, looking at it, there is no GPL licensed code in the tarball, except ltmain.sh, which is part of the build system and so do not count in distribution. So the license tag is wrong, and the reviewer should have caught that. There is also the fact that the desktop file is not checked explicitly : https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage The Buildrequires are also likely wrong, since there is no need for cairo there ( that's a polkit agent in gtk2 ), and there is surely a bug somewhere else. Removing it make build fails, and while adding it is a work around, that's likely not the right fix since nothing in the source code requires cairo, and the real fix should be to report this to the maintainer of the gtk package so it requires what is needed instead. Fwiw, desktop file validation is only required for items appearing in menus (ie, stuff under /usr/share/applications) I'll look into the other items you raise now. Confirmed cairo is indeed needed somewhere for introspection. %changelog * Mon Aug 27 2012 Rex Dieter <rdieter> - 1.4.0-4 - drop extraneous update-desktop-database scriptlet - don't mark .desktop file %%config - License: LGPLv2+ should address the rest. mate-polkit-1.4.0-2.fc17 has been pushed to the Fedora 17 stable repository. mate-polkit-1.4.0-2.fc18 has been pushed to the Fedora 18 stable repository. |