Bug 478852

- Avoid use of %makeinstall macros if "make install DESTDIR=%{buildroot}" working:
https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

- Add desktop-file-utils to BuildRequires.

- Remove "--vendor fedora" from desktop-file-install call as per guidelines.

- Running update-desktop-database in %post and %postun is not needed as desktop entry doesn't has 'MimeType' key:
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database

> Requires:	SDL >= 1.0

Ancient. Really should be removed in favour of the automatic libsdl soname dependency.


> %configure
[...]
> %makeinstall inst_dir="%{buildroot}%{_datadir}/%{name}"

Rather:

%configure inst_dir="%{_datadir}/%{name}"
[...]
make DESTDIR=%{buildroot} inst_dir="%{_datadir}/%{name}" install


> %defattr(0644,root,root,0755)

%defattr(-,root,root,-) won't work? Why?


* desktop-file-install warns about the Icon= entry

Thanks guys for your suggestion.

I made new SPEC with following changes:
 - replaced %makeinstall
 - added desktop-file-utils to BuildRequires
 - removed "--vendor fedora"
 - removed update-desktop-database
 - removed SDL from Requires

http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs.spec
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs-1.0.4-3.src.rpm

> %defattr(-,root,root,-) won't work? Why?

Probably works. I have never used them. I started using RPMs when original packages were made only by RedHat and there were many user pages with packages. I was safer in my opinion to force mentioned rights (especially for suided files).
Are there situations where (-,root,root,-) is better? Automatic builds uses (0644,root,root,0755) anyway.

> * desktop-file-install warns about the Icon= entry

Probably I have too old desktop-file-install (Fedora 8), but I didn't see any warning. Is it about .png extension?
If yes I would have to patch original file or create a new one. Is it worth to do that?

(In reply to comment #3)
> > %defattr(-,root,root,-) won't work? Why?
> 
> Probably works. I have never used them. I started using RPMs when original
> packages were made only by RedHat and there were many user pages with packages.
> I was safer in my opinion to force mentioned rights (especially for suided
> files).
> Are there situations where (-,root,root,-) is better? Automatic builds uses
> (0644,root,root,0755) anyway.

Well, by guidelines you should use (-,root,root,-) "unless you have a very good reason to deviate from that":
https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions

> > * desktop-file-install warns about the Icon= entry
> 
> Probably I have too old desktop-file-install (Fedora 8), but I didn't see any
> warning. Is it about .png extension?
> If yes I would have to patch original file or create a new one. Is it worth to
> do that?

Yes, short name should be without extension.


Add "CC-BY-SA" to License tag as package includes some of tango icons.

License status of fonts and sounds must be clarified.

%configure inst_dir="%{_datadir}/%{name}"

makes Patch0 obsolete, btw.

Hmm, (In reply to comment #5)
> %configure inst_dir="%{_datadir}/%{name}"
> 
> makes Patch0 obsolete, btw.

Unfortunately it seems to be not so easy. A package without a patch is build, but there is problem, because an application tried to read grapthic file from "/usr/share/games/lpairs/gfx/".
There is an entry "inst_dir=$datadir/games/lpairs" in configure.in which is used by the application which I don't know how to override without patch.

Current SPEC:
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs.spec
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs-1.0.4-4.src.rpm


I also sent an email to the author with a question about fonts and sounds license.

> - added desktop-file-utils to BuildRequires
That was placed in Requires, not BuildRequires.

> - changed defattr to those recomended by packaging guidelines
%attr(0755,root,root) is unnecessary now too.

> If yes I would have to patch original file or create a new one. Is it worth to
> do that?
Not fixed?

Will do full review after fonts&sounds license clarified.

> That was placed in Requires, not BuildRequires.

Ehh, probably it was late...

> %attr(0755,root,root) is unnecessary now too.

Ok, I will change it as well.

>> If yes I would have to patch original file or create a new one. Is it worth to
>> do that?
> Not fixed?

As I wrote earlier I'm sure if does it make sense to patch an old .desktop file/attach additional one to make an icon entry without .svg extension which doesn't make any effect, because in a package there is a svg file only anyway.


I've got an answer from the author:
<QUOTE>
Sounds stem from LBreakout2 which in turn took them from a CD with
non-copyrighted stuff I once bought: "Web Clip Empire 50.000",
NovaMedia Verlag, Germany. Some are also made by myself.

Fonts have been made by myself by printing X standard fonts and
modifying the bitmaps for shades and colors.

Graphics now com from Tango Desktop Project, thus are licensed under
"Creative Commons Attribution-Share Alike". Before 1.0.4 I used KDE
icons for the cards and pictures from http://www.grsites.com/textures
as backgrounds.
</QUOTE>

(In reply to comment #8)
> I've got an answer from the author:

Ok. Then, in my envision, correct License tag would be adding license for sounds: "GPLv2+ and CC-BY-SA and Freely redistributable without restriction".

> As I wrote earlier I'm sure if does it make sense to patch an old .desktop
> file/attach additional one to make an icon entry without .svg extension which
> doesn't make any effect, because in a package there is a svg file only anyway.

By guidelines it should be either short icon without extension or full path
https://fedoraproject.org/wiki/Packaging/Guidelines#Icon_tag_in_Desktop_Files

In the newest version I clarified a license tag, removed attr, patched .desktop file and fixed desktop-file-utils.

http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs.spec
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs-1.0.4-5.src.rpm

- rpmlint output:

lpairs.src:77: W: macro-in-%changelog pre
lpairs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 13, tab: line 1)
lpairs.src: W: non-coherent-filename lpairs-1.0.4-5.src.rpm
lpairs-1.0.4-5.fc8.src.rpm
3 packages and 0 specfiles checked; 0 errors, 3 warnings.

Please clean up mixed spaces and tabs; and macros in changelog.

+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format
  %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ File, containing the text of the licenses for the package is included in
  %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as
  provided in the spec URL.

5eb00da9f7fc15dc5ce840c312e76cfa  lpairs-1.0.4.tar.gz
5eb00da9f7fc15dc5ce840c312e76cfa  lpairs-1.0.4.tar.gz.orig

+ The package successfully compiles and builds into binary rpms on at least
  one primary architecture (x86_64).
+ All build dependencies are listed in BuildRequires.
+ The spec file handles locales properly.
+ Does not contain shared libraries.
+ The package does not designed to be relocatable.
+ A package owns all directories that it creates.
+ A package does not contain any duplicate files in the %files listing.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot}.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
+ Does not contain large documentation files.
+ Includes only doc files in %doc.
+ No headers.
+ No static libraries.
+ The package does not contain pkgconfig(.pc) files.
+ The package does not contain library files with a suffix (e.g.
  libfoo.so.1.1).
+ No devel packages.
+ The package does not contain any .la libtool archives.
+ Includes %{name}.desktop file. Properly installed with desktop-file-install.
+ The package does not own files or directories already owned by other
  packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot}.
+ All filenames in the package are valid UTF-8.

SHOULD:
Source package does not include CC-BY-SA license text and license for sounds as
a separate file from upstream, should query upstream to include it.


This package is APPROVED.

The final version:
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs.spec
http://timeoff.wsisiz.edu.pl/rpms/lpairs/lpairs-1.0.4-6.src.rpm

New Package CVS Request
=======================
Package Name: lpairs
Short Description: Classical memory game with cards
Owners: szpak
Branches: F-9 F-10
InitialCC:

cvs done.

Imported and built into devel, F-9 and F-10 branches. Package updates will be requested soon.

Thanks for your help!

Don't forget to add comps.xml.

Thanks for a hint. I added it also there.