Updated to latest version

SRPM: http://pbrobinson.fedorapeople.org/moblin-icon-theme-0.7-1.fc11.src.rpm

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1572941

I will review this shortly

How did you determine that the license is CC-By-SA? The COPYING file is empty. Where does create-icon-theme.sh come from?

You should also drop the explicit requires on gtk2. 

https://fedoraproject.org/wiki/Packaging/Guidelines#Requires

(In reply to comment #3)
> How did you determine that the license is CC-By-SA? The COPYING file is empty.
> Where does create-icon-theme.sh come from?  

There are a couple of different COPYING files. The one in the root of the tarball that is installed in the 0.7 version is as follows:

# more /usr/share/doc/moblin-icon-theme-0.7/COPYING

  Copyright (C) 2009 Intel Corporation

This work is licenced under the Creative Commons Attribution-Share Alike 3.0
United States License. To view a copy of this licence, visit
http://creativecommons.org/licenses/by-sa/3.0/ or send a letter to Creative
Commons, 171 Second Street, Suite 300, San Francisco, California 94105, USA.

The create-icon-theme.sh is also in the root of the tarball.

(In reply to comment #4)
> You should also drop the explicit requires on gtk2. 
> 
> https://fedoraproject.org/wiki/Packaging/Guidelines#Requires  

The reason I added it because the rpm itself doesn't have libraries in it that can be used for determining requirements but the pre/post scripts use gtk-update-icon-cache which requires gtk2.

Ah, I just realized I had downloaded the previous source instead of the latest.
Sorry for the confusion. 

You are APPROVED

Thanks :-)

New Package CVS Request
=======================
Package Name: moblin-icon-theme
Short Description: Moblin icon theme
Owners: pbrobinson
Branches: F-11
InitialCC:

(In reply to comment #6)
 
> The reason I added it because the rpm itself doesn't have libraries in it that
> can be used for determining requirements but the pre/post scripts use
> gtk-update-icon-cache which requires gtk2.  

"Note that no dependencies should be added for this. If gtk-update-icon-cache is not available, there's nothing that would be needing the cache update, ..."
Read the whole paragraph at 
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

BTW: You are not preserving timestamps while copying the files, see
https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
I'd rather use install instead of cp to make sure permissions are correct. If you want to use cp, please use %attr to verify the permissions.

CVS done.

Peter Robinson,

Please fix the issues noted by Christopher Wickert and then import

> Please fix the issues noted by Christopher Wickert and then import  

I will when I actually get the time to read what's written :)

I've updated the spec file using 'cp -p' as per the above guidelines it looks like from my reading and testing install is only usable for individual files rather than directory structure like in an icon theme.

I've also removed the Makefile.am files that are copied as well.

A side point I'm not sure how this script from the packaging guidelines works

%postun
if [ $1 -eq 0 ] ; then
    touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

As post uninstall the theme.index wouldn't exist. I think it needs a -t option.

Thanks for the pointers.

> As post uninstall the theme.index wouldn't exist. I think it needs a -t option.

I meant index.theme

(In reply to comment #13)
> I've updated the spec file using 'cp -p' as per the above guidelines it looks
> like from my reading and testing install is only usable for individual files
> rather than directory structure like in an icon theme.

This is correct. Just stick with cp, create-icon-theme.sh takes care of the permissions. Sorry for the noise.

> As post uninstall the theme.index wouldn't exist. I think it needs a -t option.

No, if index.theme doesn't exist, gtk-update-icon-cache is not run - this is what we want. It will complain about the missing dir, but this is why we have " &>/dev/null || :" at the end. (Well, at least we *should* have this at the end, you only have "|| :" because your scriptlets are outdated. See 
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
for the most recent version.)

BTW: icon-theme.cache is not part of this package, so it wont get removed when the package uninstalled. You need to
touch %{buildroot}/%{_datadir}/icons/moblin/icon-theme.cache
during install and include it ghosted in the files section.

Another thing I realized while looking at the source: You are not creating any symlinks for the icons, so many apps will have no stock icons. Add the following to your spec at the end of %install, when the Makefile.am files are already removed:

# create symlinks for gtk stock icons, these are not really 'legacy'
# this uses the legacy-icon-mapping.xml file
cd $RPM_BUILD_ROOT/usr/share/icons/moblin
for size in 16x16 24x24 48x48; do
  (
  cd $size
  for context in *; do
    if [ -d $context ]; then
      (
      cd $context
      INU_DATA_DIR=%{_builddir}/%{name}-%{version} /usr/bin/icon-name-mapping -c $context
      )
    fi
  done
  )
done

Voila, now the icon theme contains more than 3 times more files.

Last but not least create-icon-theme.sh should be running during %build. Not that it makes a difference, I'm just pedantic. ;)

Created attachment 356041 [details]
updated spec to incorporate all changes from comment # 15

Well, all changes except preserving the timestamps. Sorry.

Excellent, I'll have a look a this shortly. Thanks :)

One more thing: Please don't use a disttag here. This is a noarch theme with no dependencies, so it is not necessary to update it during an release upgrade.

As the package was already built today, you should untag the old version:
$ koji untag-pkg dist-f12 moblin-icon-theme-0.7-1.fc12

Package updated.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1577021

Looks good, thanks.

I just saw your commit in moblin's git. I didn't know that you are involved upstream. Sorry if I tried to explain you things you already know. ;)

> I just saw your commit in moblin's git. I didn't know that you are involved
> upstream. Sorry if I tried to explain you things you already know. ;)  

I'm not involved :-). I'm packaging it up for Fedora and send patches for things that I find so I assume they use the git feature to assign the credit when they apply the patches.

Built and on its way to rawhide.

Built and now in rawhide