Bug 853784

Here are a couple of initial notes:

- Add a short comment above the License field explaining why we have a 
  multiple licensing scenario here (tiled/libtiled binary: GPLv2+, tmxviewer: 
  BSD).

- Since the package doesn't provide tiled only but also tmxviewer, you should
  add this information to the %description (and add a final period to the 
  sentence).

- As there are no development files (headers etc.) installed, you probably
  don't need to provide libtiled.so either. So I recommend to remove the file 
  and drop the devel package.

- The tarball contains a copy of zlib (src/zlib). Remove this folder in %prep
  and ensure that Fedora's zlib is linked.

- The locale files must be installed using %find_lang:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files

  Thus, add "%find_lang %{name} --with-qt" to the %install section, and append
  "-f %{name}.lang" to %files. Also, in the %files section, replace 
  %{_datadir}/tiled with %{_datadir}/tiled/images/ and add the following lines
  for proper directory ownership:
  %dir %{_datadir}/tiled/
  %dir %{_datadir}/tiled/translations

- Always try to avoid plain asterisks in the %files section. Instead it's much
  better to be more specific in order to avoid adding unexpected files to the
  package:
  %{_bindir}/tiled
  %{_bindir}/tmxviewer
  %{_mandir}/man1/tiled.1*
  %{_mandir}/man1/tmxviewer.1*

- Replace %{_libdir}/libtiled.so.1* with %{_libdir}/libtiled.so.* to simplify
  future updates.

Thanks for notes

Ok i dropped the devel package though i will need to reintroduce it for the next release because it will contain the currently missing headers.

Updated the SPEC (same link): https://dl.dropbox.com/u/45541625/tiled.spec
New SRPM: https://dl.dropbox.com/u/45541625/tiled-0.8.1-2.fc17.src.rpm
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4448480

regards
Erik

(In reply to comment #2)
> Ok i dropped the devel package though i will need to reintroduce it for the
> next release because it will contain the currently missing headers.

OK, that's fine. 

Some additional remarks:
- Unlike mentioned in your spec, libtiled is not licensed under BSD at the 
  moment. According to the boilerplates of maprenderer.cpp, gidmapper.cpp, and 
  gidmapper.h, these files are GPLv2+. This is probably a mistake. Please ask 
  upstream for clarification. Currently, BSD- and GPLv2+-licensed code is linked 
  together, and the resulting binary must be licensed under GPLv2+ as the latter 
  it more restrictive than BSD.

- Preserve the timestamps by adding -p to the 3rd and 4th "install" statement.

- In the first two "install" statements, change the file permissions 664 to 644.
  Sorry, the 664 was a typo in one of my comments about your mana package and 
  should be fixed there as well. ;)

Ok i talked with upstream. The license headers were outdated (was already fixed in master). I did a patch based on the git commit.

Fixed the other issues too:

SPEC: https://dl.dropbox.com/u/45541625/tiled.spec
SRPM: https://dl.dropbox.com/u/45541625/tiled-0.8.1-3.fc17.src.rpm

regards,
Erik

Here's the formal review of tiled. The package looks good now and is ready for check-in. Please keep in mind to adapt the License field and to update the license files in %doc once you update the package to a new upstream version that doesn't contain any GPLv2+ code any longer. 

$ rpmlint tiled*
tiled.src: W: name-repeated-in-summary C Tiled
tiled.src: W: spelling-error %description -l en_US platformer -> platformed, plat former, plat-former
tiled.src: W: spelling-error %description -l en_US tmxviewer -> interviewer
tiled.x86_64: W: name-repeated-in-summary C Tiled
tiled.x86_64: W: spelling-error %description -l en_US platformer -> platformed, plat former, plat-former
4 packages and 0 specfiles checked; 0 errors, 8 warnings.

The above warnings are expected and can be ignored.

---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - BSD according to boilerplates
    - some headers still mention GPLv2+ by mistake (already fixed in upstream repo)

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ sha256sum tiled-qt-0.8.1.tar.gz*
    e5be7c38ceb24fbe0043648e8bc639804f3df5a60beb313eb039b2bcd56ad76c  tiled-qt-0.8.1.tar.gz
    e5be7c38ceb24fbe0043648e8bc639804f3df5a60beb313eb039b2bcd56ad76c  tiled-qt-0.8.1.tar.gz.upstream

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied.
[+] MUST: The spec file MUST handle locales properly.
[.] MUST: If a package installs files below %{_datadir}/icons, the icon cache must be updated.
[+] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: .so (without suffix) must go in a -devel package.
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[+] MUST: Packages must NOT contain any .la libtool archives.
[+] MUST: Packages containing GUI applications must include a %{name}.desktop file. 
[+] MUST: .desktop files must be properly installed with desktop-file-install in the %install section.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: Patch files should be prefixed with %{name}-
[+] SHOULD: All patches should be commented in the spec file
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[+] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[+] SHOULD: Your package should contain man pages for binaries/scripts.

----------------
Package APPROVED
----------------

Note that tiled itself is still GPL. Only libtiled got relicensed

Thanks a lot for review.

(In reply to comment #6)
> Note that tiled itself is still GPL. Only libtiled got relicensed

Ah OK. Thanks for the clarification.

 
> Thanks a lot for review.

You're welcome

Thanks a lot for all!


New Package SCM Request
=======================
Package Name: tiled
Short Description: Tiled Map Editor
Owners: ablu
Branches: f16 f17 f18
InitialCC:

Git done (by process-git-requests).

tiled-0.8.1-3.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/tiled-0.8.1-3.fc16

tiled-0.8.1-3.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/tiled-0.8.1-3.fc17

tiled-0.8.1-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/tiled-0.8.1-3.fc18

tiled-0.8.1-3.fc18 has been pushed to the Fedora 18 stable repository.