This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours

Bug 853694 (mate-control-center)

Summary: Review Request: mate-control-center - MATE Desktop control center
Product: [Fedora] Fedora Reporter: Dan Mashal <dan.mashal>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: leigh123linux, notting, package-review, rdieter
Target Milestone: ---Flags: rdieter: fedora‑review+
limburgher: 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-10-19 03:16:22 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On: 853692    
Bug Blocks: 840149    

Comment 1 Rex Dieter 2012-09-15 20:45:10 EDT
naming: ok

sources: ok
9eebb9972d1759dd001ba78fe66bde09  mate-control-center-1.4.0.tar.xz

scriptlets: ok

1. licensing: there is a mixture of LGPLv2+ and GPLv2+ sources, SHOULD simplify to just:
License: GPLv2+

2. MUST omit all stuff under %{_datadir}/mime, except for anything matching
%{_datadir}/mime/packages/*.xml
that's generated content from scriptlets (and the current stuff you list would conflict with shared-mime-info), see also item 3

3. MUST add mimeinfo-related scriptlets:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo

4.  MUST add .desktop mime-related scriptlets:
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database

5. MUST drop not-needed
BuildRequires: gtk+-devel
Comment 2 Rex Dieter 2012-09-16 07:30:36 EDT
since you'd asked in chat what item 2 was about, the .spec's %files section currently includes:
%{_datadir}/mime/XMLnamespaces
%{_datadir}/mime/aliases
%{_datadir}/mime/application/x-mate-theme-package.xml
%{_datadir}/mime/generic-icons
%{_datadir}/mime/globs
%{_datadir}/mime/globs2
%{_datadir}/mime/icons
%{_datadir}/mime/magic
%{_datadir}/mime/mime.cache
%{_datadir}/mime/packages/mate-theme-package.xml
%{_datadir}/mime/subclasses
%{_datadir}/mime/treemagic
%{_datadir}/mime/types
%{_datadir}/mime/version

whereas you only want to include
%{_datadir}/mime/packages/mate-theme-package.xml
Comment 3 Dan Mashal 2012-09-25 19:58:11 EDT
If I drop those files I get the following:

RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/share/mime/XMLnamespaces
   /usr/share/mime/aliases
   /usr/share/mime/application/x-mate-theme-package.xml
   /usr/share/mime/generic-icons
   /usr/share/mime/globs
   /usr/share/mime/globs2
   /usr/share/mime/icons
   /usr/share/mime/magic
   /usr/share/mime/mime.cache
   /usr/share/mime/subclasses
   /usr/share/mime/treemagic
   /usr/share/mime/types
   /usr/share/mime/version
Comment 4 Dan Mashal 2012-09-25 19:58:52 EDT
Anyways please check the following updates spec/srpm:

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-2.fc17.src.rpm
Description: MATE Desktop control center
Comment 5 Rex Dieter 2012-09-26 10:32:33 EDT
In order of preference, you'll need to:
* find a way for the buildsys not to create those files (patching maybe)
* manually delete them
or 
* use %exclude on those files.
Comment 6 Dan Mashal 2012-09-26 17:50:42 EDT
%exclude ftw
Comment 8 Dan Mashal 2012-09-30 17:47:01 EDT
I've fixed all of the build issues, per our chat, and removed about me (pretty useless).

Please check it:

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-4.fc17.src.rpm
Description: MATE Desktop control center

Successful koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=4544916
Comment 9 leigh scott 2012-10-02 19:41:52 EDT
(In reply to comment #8)
> I've fixed all of the build issues, per our chat, and removed about me
> (pretty useless).
> 
> Please check it:
> 
> Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
> SRPM URL:
> http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-4.
> fc17.src.rpm
> Description: MATE Desktop control center
> 
> Successful koji build:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=4544916

Please remove --libexecdir=/usr/libexec from


%configure --disable-static --disable-schemas-install --enable-aboutme --disable-scrollkeeper --libexecdir=/usr/libexec

The %configure macro already defines it plus you used a hard path instead of a macro ( correct way  --libexecdir=%{_libexecdir} ).
Comment 10 leigh scott 2012-10-02 19:47:16 EDT
you also have a spelling mistake in

%configure --disable-static --disable-schemas-instalp --disable-scrollkeeper --libexecdir=/usr/libexec


--disable-schemas-instalp   


should be

--disable-schemas-install


Do you ever check the buildlogs?

http://kojipkgs.fedoraproject.org//work/tasks/4917/4544917/build.log

+ export LDFLAGS
+ ./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --disable-static --disable-schemas-instalp --disable-scrollkeeper --libexecdir=/usr/libexec
configure: WARNING: unrecognized options: --disable-schemas-instalp
checking for a BSD-compatible install... /usr/bin/install -c


note --libexecdir=/usr/libexe is in there twice!
Comment 11 Dan Mashal 2012-10-03 01:02:52 EDT
Thanks Leigh,

Yes, learned the hard way about looking at build logs. 

The reason why libexec is there is to avoid the conflict with gnome... what would you suggest? It does work, yes I know rpmbuild specifies a lot of this stuff but as per upstream using that flag avoids conflicts with Gnome 3. Maybe I should use export for the libexec path dir instead?

I have fixed the spelling error. Thank you for pointing this out.
Comment 12 Dan Mashal 2012-10-03 01:23:20 EDT
Updated:

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-3.fc17.src.rpm
Description: MATE Desktop control center
Comment 13 leigh scott 2012-10-03 01:25:25 EDT
(In reply to comment #11)
> Thanks Leigh,
> 
> Yes, learned the hard way about looking at build logs. 
> 
> The reason why libexec is there is to avoid the conflict with gnome... what
> would you suggest? It does work, yes I know rpmbuild specifies a lot of this
> stuff but as per upstream using that flag avoids conflicts with Gnome 3.
> Maybe I should use export for the libexec path dir instead?
> 
> I have fixed the spelling error. Thank you for pointing this out.

Well if you insist on adding the libexec bit you MUST use a macro 

eg:

--libexecdir=%{_libexecdir}

Or your package will fail the review.
Comment 14 leigh scott 2012-10-03 01:27:14 EDT
(In reply to comment #12)
> Updated:
> 
> Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
> SRPM URL:
> http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-3.
> fc17.src.rpm
> Description: MATE Desktop control center

This still fails the review process.

1. use of hard path in configure command
Comment 15 Dan Mashal 2012-10-03 01:31:08 EDT
Yes, good catch, not needed for this package. Fixing and removing the requires field.
Comment 16 Dan Mashal 2012-10-03 03:11:18 EDT
Fixed a bunch of stuff. Removed libexecdir from configure flag. Please let me know if you find any other issues. Thanks.

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-6.fc17.src.rpm
Description: MATE Desktop control center
Comment 17 leigh scott 2012-10-03 05:35:10 EDT
Ok you have claimed ownership of 

%{_datadir}/mime/XMLnamespaces
%{_datadir}/mime/aliases

you shouldn't own them. please fix 



Please remove the noconfig from all these files.

%config(noreplace) %{_sysconfdir}/mateconf/schemas/control-center.schemas
%config(noreplace) %{_sysconfdir}/mateconf/schemas/fontilus.schemas
%config(noreplace) %{_sysconfdir}/mateconf/schemas/mate-control-center.schemas
%config(noreplace) %{_sysconfdir}/xdg/autostart/mate-at-session.desktop
%config(noreplace) %{_sysconfdir}/xdg/menus/matecc.menu

They aren't user config files
Comment 18 leigh scott 2012-10-03 05:36:05 EDT
(In reply to comment #17)

> Please remove the noconfig from all these files.
> 
> %config(noreplace) %{_sysconfdir}/mateconf/schemas/control-center.schemas
> %config(noreplace) %{_sysconfdir}/mateconf/schemas/fontilus.schemas
> %config(noreplace)
> %{_sysconfdir}/mateconf/schemas/mate-control-center.schemas
> %config(noreplace) %{_sysconfdir}/xdg/autostart/mate-at-session.desktop
> %config(noreplace) %{_sysconfdir}/xdg/menus/matecc.menu
> 
> They aren't user config files

I mean remove the noreplace bit
Comment 19 Dan Mashal 2012-10-03 05:39:19 EDT
I figured as much personally. Will do.
Comment 20 leigh scott 2012-10-03 05:42:59 EDT
Also remove the

Requires(post): desktop-file-utils
Requires(postun): desktop-file-utils




Note: For FC5+, this scriptlet follows the same convention as mimeinfo files and gtk-icon-cache. Namely, the spec file should not Require desktop-file-utils for this. For older releases, one should

Requires(post): desktop-file-utils
Requires(postun): desktop-file-utils
Comment 22 leigh scott 2012-10-03 06:01:08 EDT
(In reply to comment #21)
> Fixed.
> 
> Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
> SRPM URL:
> http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-7.
> fc17.src.rpm
> Description: MATE Desktop control center

You haven't fixed the ownership issue

%{_datadir}/mime/XMLnamespaces/
%{_datadir}/mime/aliases/


Please fix
Comment 23 Rex Dieter 2012-10-03 07:45:23 EDT
building with --disable-update-mimedb option as I'd suggested in chatting will make most of that mime stuff (which you shouldn't own) go away.
Comment 24 Rex Dieter 2012-10-03 18:37:56 EDT
Still haven't addressed item 1, though that was only a SHOULD

6. MUST  still need to delete or %exclude %{_datadir}/applications/mimeinfo.cache

7. SHOULD not run desktop-file-validate on
%{_sysconfdir}/xdg/autostart/mate-at-session.desktop
it's only required for stuff under %{_datadir}/applications/


fixup item 6, and looks like we have a winner
Comment 25 Dan Mashal 2012-10-03 18:46:42 EDT
I actually just did that for the must.. 

was just about to update before you commented.

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-control-center.spec 
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-control-center-1.4.0-8.fc17.src.rpm
Description: MATE Desktop control center

will fix the "shoulds" before importing the packages.
Comment 26 Rex Dieter 2012-10-03 19:08:56 EDT
mostly harmless but,
%{_datadir}/mime/packages/mate-theme-package.xml
is listed twice

and mimeinfo.cache isn't deleted or %excluded, so will likely lead to build failure (unpackaged file)



but, you can fix that later, APPROVED.
Comment 27 Dan Mashal 2012-10-03 20:54:20 EDT
Thanks, fill fix before import as well
Comment 28 Dan Mashal 2012-10-03 21:10:36 EDT
New Package SCM Request
=======================
Package Name: mate-control-center
Short Description: MATE Desktop control center
Owners: rdieter vicodan
Branches: f16 f17 f18
Comment 29 Jon Ciesla 2012-10-04 07:20:16 EDT
Git done (by process-git-requests).
Comment 30 Fedora Update System 2012-10-05 06:25:27 EDT
mate-control-center-1.4.0-8.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-control-center-1.4.0-8.fc17
Comment 31 Fedora Update System 2012-10-05 06:25:41 EDT
mate-control-center-1.4.0-8.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/mate-control-center-1.4.0-8.fc18
Comment 32 Fedora Update System 2012-10-05 14:00:16 EDT
mate-control-center-1.4.0-8.fc18 has been pushed to the Fedora 18 testing repository.