You either build it from one spec, or you will need two independent package reviews.
I suggest you to make 0ad-data subpackage of 0ad.

I suppose we can live in this specific case without uploading src.rpm and spec is just ok.

Oh sorry, I thought that those packages are build from one tar.gz. They are built from 2 different tar.gz.
In this case I reccomend you to leave this review only for 0ad. And create new review for 0ad-data. And 0ad-data should be review before 0ad.

You stated:
 License:	GPLv2+
But LICENSE.txt list more licenses used:
 MIT and what I'm worried about is "Various (unspecified)"

You have to dive deep in and dig out all licenses used.

Thanks. I will work on a new package in the weekend, as there is a newer
alpha with Hellenic race split in 3, now there are Spartans! and do
a more complete review of the licenses, and should also ask in upstream
forums for more details.
I will also add a newer review request for 0ad-data.

I did a minor conversion from the OBS sample spec files, but they indeed should
be in error, e.g. models and textures should be most if not all CC-BY-SA.

Indeed, upacking "public.zip" from the 0ad-data package, I see only data files,
but two LICENSE.txt files, in art and audio subdirectories,

art/LICENSE.txt
-%<-
The files in this directory are Copyright (C) 2009 Wildfire Games.

These files are licensed under the Creative Commons Attribution-Share Alike 3.0
(CC-by-sa) license, available at http://creativecommons.org/licenses/by-sa/3.0/

Briefly, this means:

* You may use, modify and distribute these files, for commercial and
  non-commercial purposes.

* If you distribute one of these files, you must include attribution (e.g.
  in the credits screen of a game or a video, or in a text file accompanying
  the files). The attribution must include:
  * A link to http://creativecommons.org/licenses/by-sa/3.0/
  * The name "Wildfire Games" as the original author
  * A link to http://www.wildfiregames.com/

* If you distribute one of these files, you must release it (and any
  modifications you have made to it) under the CC-by-sa license.

Some of the files in the "textures/" directory are derived from materials provided by CGTextures (http://www.cgtextures.com/). The original materials are the property of CGTextures or its contributors. Special permission has been granted by CGTextures to distribute these derived textures as CC-BY-SA. (This has no effect on the standard licensing of CGTextures materials, or on any other work derived from them.)

All CGTextures materials have been altered from their original form. To access original super-high resolution CGTextures materials, please visit http://www.cgtextures.com/ .
-%<-

audio/LICENSE.txt
-%<-
The files in this directory are Copyright (C) 2009 Wildfire Games.

These files are licensed under the Creative Commons Attribution-Share Alike 3.0
(CC-by-sa) license, available at http://creativecommons.org/licenses/by-sa/3.0/

Briefly, this means:

* You may use, modify and distribute these files, for commercial and
  non-commercial purposes.

* If you distribute one of these files, you must include attribution (e.g.
  in the credits screen of a game or a video, or in a text file accompanying
  the files). The attribution must include:
  * A link to http://creativecommons.org/licenses/by-sa/3.0/
  * The name "Wildfire Games" as the original author
  * A link to http://www.wildfiregames.com/

* If you distribute one of these files, you must release it (and any
  modifications you have made to it) under the CC-by-sa license.
-%<-

Licenses I found in sources:

------------------------------------------------------------------
  /source
    GPL version 2 (or later) - see license_gpl-2.0.txt

------------------------------------------------------------------
  /source/lib
    MIT

------------------------------------------------------------------
  /source/tools
    Various (unspecified)

* source/tools/fontbuilder2/
#   Copyright (C) 2002-2009 Nuclex Development Labs
#
#   This library is free software; you can redistribute it and/or
#   modify it under the terms of the IBM Common Public License as
#   published by the IBM Corporation; either version 1.0 of the
#   License, or (at your option) any later version.

* source/tools/springimport
MIT

...
* source/tools/atlas/
GPLv2+

* source/tools/replayprofile
MIT or dual MIT and GPL

------------------------------------------------------------------
* Not an issue as there are no binaries in the source tarball
  /binaries/system
    Various (unspecified)

* Not an issue, binaries are not in the source tarball, and this is
windows only
  /binaries/system/dbghelp.dll
    Proprietary - see license_dbghelp.txt for restrictions you must agree to
    before distributing this particular file

------------------------------------------------------------------
  /libraries
    Various - see individual directories and files for details

* libraries/cxxtest/COPYING
LGPLv2.1+

* libraries/valgrind (not built)
BSD-style (no clauses) and GPLv2+

* libraries/nvtt
MIT and public domain (a few files explicitly state public domain)

* libraries/enet
BSD-style (no clauses)

* libraries/fcollada
MIT

------------------------------------------------------------------
  /build
    Various (unspecified)

* build/bin
LGPLv2.1+  (actually only a license file, nothing else in source tarball)

* build/premake
MIT

------------------------------------------------------------------
  /docs
    Various (unspecified)

directory not in source tarball



Extra comments:
  I thought I had commented in the bug report, but did not, so, there is
an issue with fedora using a too old enet, so the package is not built
with --with-system-enet. This could only be remedied by detecting what
packages, if any require the older api enet in fedora, and add the newer
one. Upstream maintains two incompatible trees:
http://enet.bespin.org/SourceDistro.html

  nvtt could be packaged as a system package, but would require the patches
from the tarball to actually build (outdated from 2007 upstream), in README.txt
README.txt 
This is NVTT 2.0.8-1 from http://code.google.com/p/nvidia-texture-tools/
plus some patches:
  r1156.patch (from NVTT SVN r1156 - fixes build with libtiff 4.0)
  r1157.patch (from NVTT SVN r1157 - fixes build with CUDA 3.0)
  r1172.patch (from NVTT SVN r1172 - fixes memory allocator interaction with Valgrind)
  r907.patch and r1025.patch (from NVTT SVN - fixes build on FreeBSD)
  rpath.patch (fixes .so file search paths for bundled copy)
  issue139.patch (fixes http://code.google.com/p/nvidia-texture-tools/issues/detail?id=139)
  png-api.patch (partially from NVTT SVN r1248 - fixes build with libpng 1.5)
  cmake-freebsd.patch (fixes build on FreeBSD)

I just checked that rawhide ships enet-1.3.3, so, for rawhide
it can use system enet.

I am also working on a nvidia-texture-tools package, so, I guess
I will give up on packaging 0ad (and megaglest) for f16 and f17
and stick to rawhide.

New upstream alpha, and now there is also a separate 0ad-data
review request.

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-r11863-1.fc18.src.rpm

Please confirm that it is correct to tag as IBM license the code
from source/tools/fontbuilder2/Packer.py:

# Adapted from:
#   http://enichan.darksiren.net/wordpress/?p=49
# which was adapted from some version of
#   https://devel.nuclex.org/framework/browser/game/Nuclex.Game/trunk/Source/Packing/CygonRectanglePacker.cs
# which has the following license:
#
#   Copyright (C) 2002-2009 Nuclex Development Labs
#
#   This library is free software; you can redistribute it and/or
#   modify it under the terms of the IBM Common Public License as
#   published by the IBM Corporation; either version 1.0 of the
#   License, or (at your option) any later version.

Forgot to specify, the two patches were reported upstream at:
http://trac.wildfiregames.com/ticket/1420
http://trac.wildfiregames.com/ticket/1421

Also, just in case, I tested the package and it works :-)

Updated package

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-r11863-2.fc18.src.rpm

- Disable dependency on nvidia-texture-tools (#823096).
- Disable %%check as it requires nvtt.
- Add manual page.

The package appears functional with nvtt disabled. And I also requested
some information at http://trac.wildfiregames.com/ticket/1427

I just run fedora-review and this is issues sans those I waived out.

Issues:
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files section. This is OK if packaging
     for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop
[!]: MUST Rpmlint output is silent.

rpmlint 0ad-debuginfo-r11863-2.fc18.i686.rpm

0ad-debuginfo.i686: W: only-non-binary-in-usr-lib
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/tmp/818401/0ad-r11863-alpha-unix-build.tar.xz :
  MD5SUM this package     : f42a2e18515cbcd48b99f0ea3796b3a4
  MD5SUM upstream package : f577f8d3a69146cfdc988a56f3caecd8

See: http://fedoraproject.org/wiki/Packaging/SourceURL
[!]: MUST Development (unversioned) .so files in -devel subpackage, if
     present.
     Note: 0ad-r11863-2.fc18.i686.rpm : /usr/lib/0ad/libAtlasUI.so
     0ad-r11863-2.fc18.i686.rpm : /usr/lib/0ad/libCollada.so
See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages

(In reply to comment #10)
> I just run fedora-review and this is issues sans those I waived out.

  Thanks for the review!

> Issues:
> [!]: MUST Each %files section contains %defattr if rpm < 4.4
>      Note: defattr(....) present in %files section. This is OK if packaging
>      for EPEL5. Otherwise not needed
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions

  I will remove it for the next package.

> [!]: MUST Package contains a properly installed %{name}.desktop using
> desktop-
>      file-install file if it is a GUI application.
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

   I will add a call to desktop-file-validate and correct any problems
that arise.

> [!]: MUST Rpmlint output is silent.
> 
> rpmlint 0ad-debuginfo-r11863-2.fc18.i686.rpm
> 
> 0ad-debuginfo.i686: W: only-non-binary-in-usr-lib
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
> [!]: MUST Sources used to build the package match the upstream source, as
>      provided in the spec URL.
> /tmp/818401/0ad-r11863-alpha-unix-build.tar.xz :
>   MD5SUM this package     : f42a2e18515cbcd48b99f0ea3796b3a4
>   MD5SUM upstream package : f577f8d3a69146cfdc988a56f3caecd8
> 
> See: http://fedoraproject.org/wiki/Packaging/SourceURL

  I just did download it again to ensure it was not replaced by
upstream. I think your test may have donwloaded the tar.gz file,
did uncompress it, or did download some .html file...

$ LC_ALL=C wget http://releases.wildfiregames.com/0ad-r11863-alpha-unix-build.tar.xz
--2012-05-22 11:50:53--  http://releases.wildfiregames.com/0ad-r11863-alpha-unix-build.tar.xz
Resolving releases.wildfiregames.com (releases.wildfiregames.com)... 92.243.18.55
Connecting to releases.wildfiregames.com (releases.wildfiregames.com)|92.243.18.55|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://sourceforge.net/projects/zero-ad/files/releases/0ad-r11863-alpha-unix-build.tar.xz/download [following]
--2012-05-22 11:50:54--  http://sourceforge.net/projects/zero-ad/files/releases/0ad-r11863-alpha-unix-build.tar.xz/download
Resolving sourceforge.net (sourceforge.net)... 216.34.181.60
Connecting to sourceforge.net (sourceforge.net)|216.34.181.60|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: http://downloads.sourceforge.net/project/zero-ad/releases/0ad-r11863-alpha-unix-build.tar.xz?r=&ts=1337699972&use_mirror=tenet [following]
--2012-05-22 11:50:55--  http://downloads.sourceforge.net/project/zero-ad/releases/0ad-r11863-alpha-unix-build.tar.xz?r=&ts=1337699972&use_mirror=tenet
Resolving downloads.sourceforge.net (downloads.sourceforge.net)... 216.34.181.59
Connecting to downloads.sourceforge.net (downloads.sourceforge.net)|216.34.181.59|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: http://tenet.dl.sourceforge.net/project/zero-ad/releases/0ad-r11863-alpha-unix-build.tar.xz [following]
--2012-05-22 11:50:55--  http://tenet.dl.sourceforge.net/project/zero-ad/releases/0ad-r11863-alpha-unix-build.tar.xz
Resolving tenet.dl.sourceforge.net (tenet.dl.sourceforge.net)... 155.232.191.245, 2001:4200:fffc::245
Connecting to tenet.dl.sourceforge.net (tenet.dl.sourceforge.net)|155.232.191.245|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 8657204 (8.3M) [application/octet-stream]
Saving to: `0ad-r11863-alpha-unix-build.tar.xz'

100%[======================================>] 8,657,204   1.13M/s   in 12s     

2012-05-22 11:51:09 (720 KB/s) - `0ad-r11863-alpha-unix-build.tar.xz' saved [8657204/8657204]

$ md5sum 0ad-r11863-alpha-unix-build.tar.xz 
f42a2e18515cbcd48b99f0ea3796b3a4  0ad-r11863-alpha-unix-build.tar.xz

> [!]: MUST Development (unversioned) .so files in -devel subpackage, if
>      present.
>      Note: 0ad-r11863-2.fc18.i686.rpm : /usr/lib/0ad/libAtlasUI.so
>      0ad-r11863-2.fc18.i686.rpm : /usr/lib/0ad/libCollada.so
> See: http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages

  I can check if reasonable to add a soversion to those libraries. But
they are not really meant to be in -devel packages, as no headers are
installed, and they are not in %_libdir on purpose.

I addressed the fedora-review issues, but the unversioned .so
in %{_libdir}/0ad. If this really needs to be rpmlint silent,
I can work on making a fake -devel package and patch the
package to generate a versioned .so and link to it.

Note that this would not the only package that has "modules" or
libraries in %{_libdir}/*/lib*.so

Actually, I already reported a related issue about rpath to
upstream, to avoid needing special cases :-) at
http://trac.wildfiregames.com/ticket/1421

Updated package:

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-r11863-3.fc18.src.rpm

They are since many time a spec for 0ad at http://repos.fedorapeople.org/repos/bioinfornatics/0ad/
(not yet updated to latest release alpha X)

but i do not think 0ad can go to official repo as he is many bundle library and this it is a no go.

(In reply to comment #12)
> I addressed the fedora-review issues, but the unversioned .so
> in %{_libdir}/0ad. If this really needs to be rpmlint silent,
> I can work on making a fake -devel package and patch the
> package to generate a versioned .so and link to it.
> 
> Note that this would not the only package that has "modules" or
> libraries in %{_libdir}/*/lib*.so

Private unversioned libraries in %{_libdir}/0ad/ are perfectly fine, no need to do downstream patches to change this. It's also common to have an RPATH set that points to %{_libdir}/0ad/ :
http://fedoraproject.org/wiki/Packaging/Guidelines#Rpath_for_Internal_Libraries

rpmlint is just a tool to warn about possible problems in the spec file. Sometimes its heuristics are right, sometimes not. There are often good reasons to ignore rpmlint warnings/errors, it doesn't have to be silent always.

(In reply to comment #13)
> They are since many time a spec for 0ad at
> http://repos.fedorapeople.org/repos/bioinfornatics/0ad/
> (not yet updated to latest release alpha X)

  Thanks for the information. I was only aware of OpenSUSE obs builds for
Fedora, and got somewhat involved with it after waiting several months
checking from time to time if the Mandriva package was functional...

  In case the package is approved, I would be glad to have it assigned
to a maintainer group :-)

> but i do not think 0ad can go to official repo as he is many bundle library
> and this it is a no go.

  The only issue with bundled packages now is fcollada, that either
following links in the embedded sources, or the "Download" link from
http://collada.org/mediawiki/index.php/FCollada are not working
(lead to a link, to information about "formerly Feeling Software",
the original authors...). But searching a bit more, I see
http://www.wildfiregames.com/forum/index.php?showtopic=14539

  But for now I prefer not to add extra review requests, and would
be better if there was some legal information about nvidia-texture-tools
issue with possibly encumbered s3tc decompression (and compression, but
AFAIK compression can be done in non infringing, and faster ways, the
problem is decompression that cannot be fully done without infringing
the patent).

s3tc lib are in rpm-fusion if it is legal in your state

You mean squish? Then would need to check if can use that instead of the bundled version in nvidia-texture-tools.

Maybe the proper approach would be to just package 0ad in rpm-fusion :-)

I do not see 0ad at http://download1.rpmfusion.org/free/fedora/development/rawhide/source/SRPMS/repoview/amusements.games.group.html so, it should be better to move to there... I will try to create an account and submit a review request in the weekend, but I already got plenty in my plate, and 0ad was a side review request, hoping to not consume too much time on it...

About being legal, afaik this patent is a non issue in all the world but US, Japan, Germany and possibly a few other countries (but US is pushing hard to have the rest of the world to use US laws). Apple was sued recently due to s3tc; unlikely they would sue "minor" distros, but Red Hat could be a target...

if can to add the french desciption given in my spec file and create a subpackage data as i do (take example) that is ok for me

Ok. I have another review request, and actually updated it to
build in fedora 16 and 17, and rawhide, making it optional to
build with system nvtt or internal nvtt, and also have the
option to disable nvtt (default), so that it should not build
any s3tc handling code. The rpmfusion review request is at:
https://bugzilla.rpmfusion.org/show_bug.cgi?id=2342

I made 0ad-data another package. This is useful if needs to
rebuild only the binary, as there is no need to generate
another (very large) 0ad-data with the same contents.

Why rpmfusion? As I understand it, the issue that was blocking 0ad from Fedora inclusion was s3tc patents; if the package works without nvidia-texture-tools, can't this go to Fedora proper?

The option to build with nvtt disabled is not documented,
and need a patch to "forward" it to the premake lua script,
and also to not fail before.

I would also prefer to have it in Fedora, but I am afraid
it may at some point no longer build without nvtt, and AFAIK
the code the patent cover is somewhat simple and I am not sure
there is no "inline" implementation of it somewhere else in the
sources. This is partly why I asked for some information at
http://trac.wildfiregames.com/ticket/1427

I think for this game this would qualify as "fair use", but
IANAL, and if doing this, would it also be "fair use" to have
s3tc enabled in Mesa? Maybe could also have a s3tc enabled
Mesa in rpmfusion; AFAIK Ubuntu has it as a PPA,
https://launchpad.net/~oibaf/+archive/graphics-drivers/

Fair use is for copyright, not patents. And not all places have fair use as a defense.

just to add every patch should be reported to the upstream

Patches were reported upstream as specified in #c8. The patch to disable nvtt was not clearly stated as reported upstream in #c9, but was attached to the related trac.
But now I have the package review request still here, and opened another one in rpmfusion :-) Because I think it should use nvtt, actually the issue with s3tc, that is the, behind a patent, "standard" for texture compression.

st3c isn't enabled in the Fedora version of mesa. Is having this enabled for 0 AD in rpmfusion going to help?

I asked for some upstream comment at http://trac.wildfiregames.com/ticket/1427 when providing a patch to disable nvtt. The patch appears to work and I did not notice problems in a simple game.
I just noticed in 0ad logs it says my computer supports s3tc (the output does not match glxinfo), so, I changed /usr/share/0ad/config/default.cfg, that should also include a patch for no s3tc, to:

;nos3tc = false
nos3tc = true

and

;force_s3tc_enable = true
force_s3tc_enable = false

but still did not see any problems.

I know for sure it is required for "make check" and game modding, and should be of use at least for people using the nvidia and amd drivers, otherwise, mesa with s3tc enabled could be provided in the rpmfusion repository.
Maybe with recent "rumours" about valve steam ported to linux this would be more of an issue.

I am happy enough with the comment at http://trac.wildfiregames.com/ticket/1427
so, with nvtt disabled, it should be ok to have 0ad in fedora, not rpmfusion.

I will close the nvtt review requests and rpmfusion review requests and only wait/hope :-) for a review here.

Reverse dependency. It should be 0ad-data that depends on 0ad for review.

0ad is available in debian, see http://packages.debian.org/unstable/main/0ad and a license review can be viewed at http://anonscm.debian.org/viewvc/pkg-games/packages/trunk/0ad/debian/copyright?view=markup

The package I am proposing also removes the nvtt files before starting build and patches the build to not compile code that uses S3TC.

I messed things a bit by making a review request at rpmfusion, then later closing it and choosing to only make a review request with S3TC disabled, sorry...

I really would like to have a Legal response about it, as, besides in alpha, it is a "state of the art" game, very high quality and quite playable, and would be very valuable in fedora game spin.

Blame me if I forgot some step :-) but I believe I followed all recommendations in this report, and the latest package is at

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-r11863-5.fc18.src.rpm

and should build cleanly for f16 or newer.

Alpha 11 should be due out any day now, and it looks like there are some significant improvements. (I think gates are supposed to work now, which will make walls more useful.)

Just to be clear here, what question(s) are you asking of Fedora Legal?

(In reply to comment #31)
> Just to be clear here, what question(s) are you asking of Fedora Legal?

Some comment about s3tc, and if just not building it is enough (and removing sources at %build time is ok), otherwise there is a bundled nvtt, that bundles an old squish library, that implements code covered by the S3TC patent.

Other than that, I just tried to get some attention to this review request :-)

PS: debian appears to also package nvidia-texture-tools, see http://packages.debian.org/source/sid/nvidia-texture-tools

You may not ship any code in Fedora that implements S3TC. You will need to make a clean source tarball that has any such code removed, it is not sufficient to remove these sources at %build.

Sorry. :/

But that should not be problem. Simly manualy remove it from that tar.gz and put in spec in comment before Source0 document what and how it was removed.
We will not be able to verify md5 of tar.gz, but that is not blocker.

Thanks for the comments. I remade the package for review request, and just confirmed it works with fc17 and up to date rawhide. For rawhide, it required to adapt a patch already in upstream trac to build with boost 1.50, and I made a patch to build with libxml2 2.9 and submitted it for upstream review at http://trac.wildfiregames.com/ticket/1656

I made a few standard cleanups to the spec (it was one of the first packages I submitted for review), confirmed it builds in a mock chroot, and changed it to add a comment of how the tarball was regenerated, and removed the s3tc implementation from it, but left it easy to use a single %global to support user builds with s3tc enabled (but now needs to redownload upstream source or regenerate it from a svn checkout).

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-r11863-6.fc19.src.rpm

Updated to latest upstream alpha release:

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-0.0.11-1.fc19.src.rpm

Thanks for updating to alpha 11. It looks like it has lots of improvements over alpha 10. Hopefully alpha 12 will have the AI performance fixed.

rpmlint warning:
0ad.x86_64: W: no-manual-page-for-binary pyrogenesis

Although, this is not blocker, but Debian has man page (written under GPLv2) so you can use that.

Oh, it should by just symlink to:
/usr/share/man/man1/pyrogenesis.1.gz -> 0ad.1.gz

And more precise it should not be in group:
  1 - Executable programs or shell commands
but in group
  6 - Games

Thanks for the review!

Updated package:

- Corrected the manual page group
- Made a symlink from 0ad to pyrogenesis manual page
- Followed my own advice in comments and reviewed/updated the manual page :-)
- Corrected some typos in the (manually written) manual page

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-0.0.11-2.fc19.src.rpm

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== C/C++ ====
[x]: MUST Header files in -devel subpackage, if present.
[x]: MUST Package does not contain any libtool archives (.la)
[x]: MUST Package does not contain kernel modules.
[x]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.
[!]: MUST Development (unversioned) .so files in -devel subpackage, if
     present.
     Note: Unversioned so-files in non-devel package (fix or
     explain):0ad-0.0.11-2.fc17.x86_64.rpm : /usr/lib64/0ad/libAtlasUI.so
     0ad-0.0.11-2.fc17.x86_64.rpm : /usr/lib64/0ad/libCollada.so
But this can be waived as no one but only this package will use this .so files.


==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[!]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Sources contain only permissible code or content.
     Yes, and documented in comments before Source0
[x]: MUST Package contains desktop file if it is a GUI application.
[x]: MUST Package installs a %{name}.desktop using desktop-file-install if
     there is such a file.
     desktop-file-validate is used, which is allowed.
[-]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[-]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[-]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "zlib/libpng", "BSD (2 clause) GPL (v2 or later)", "BSD (3 clause)", "GPL
     (v2 or later)", "MIT/X11 (BSD like)" For detailed output of licensecheck
     see file: /home/msuchy/818401-0ad/licensecheck.txt
[x]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[!]: MUST If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[-]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: MUST Package must own all directories that it creates.
[!]: MUST Package does not own files or directories owned by other packages.
     /usr/share/0ad is owned by both 0ad and 0ad-data. But I do not see problem here.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
     I compared tar.xz in src.rpm and downloaded tar.xz striped from directories according comments in spec and diff is empty
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL5 is required
[!]: 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.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
     Yes, it works. And pretty fine. I lost 30 minutes of review just on this point :)
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SHOULD SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[x]: SHOULD SourceX / PatchY prefixed with %{name}.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[!]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Package contains no bundled libraries.
     Can you please ellaborate on AtlasUI and libCollada? At least in collada case:
     http://collada.org/mediawiki/index.php/FCollada
     it seems to me that is should be packed separately. But I maybe wrong.
[!]: MUST If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
     Please document which part is license under what license. You may very probably copy it from Debian license file. Or maybe just point to LICENSE.txt, but there are those annonying "Various"
[!]: 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.
     Please ask upstream to include BSD, MIT and IBM license.
[!]: SHOULD Packages should try to preserve timestamps of original installed
     files.
     You may use install -p to achive this.

(In reply to comment #41)

Thanks for the review. I will upload a newer srpm tonight, but may
need some extra feedback also :-)

[...]
> Issues:
> [!]: MUST Package contains no bundled libraries.
>      Can you please ellaborate on AtlasUI and libCollada? At least in
> collada case:
>      http://collada.org/mediawiki/index.php/FCollada
>      it seems to me that is should be packed separately. But I maybe wrong.

  libAtlasUI.so is 0ad specific for the map editor, e.g. "0ad -editor".

  libCollada.so is from the 0ad "fork" of fcollada. From what I understand,
the last opensource version is from 2008, and all links at
http://collada.org/mediawiki/index.php/FCollada lead to
http://www.fortem.com/

  The closest to a common open source collada I know of is
https://github.com/fcolladaCE/fcolladaCE, following the information at
http://trac.wildfiregames.com/ticket/562, but the debian 0ad package
also uses the bundled one in 0ad sources.

> [!]: MUST If the package is under multiple licenses, the licensing breakdown
>      must be documented in the spec.
>      Please document which part is license under what license. You may very
> probably copy it from Debian license file. Or maybe just point to
> LICENSE.txt, but there are those annonying "Various"

  I made a breakdown in #c5, but I am not an specialist :-), it may be
desirable a third opinion, maybe ask FE-LEGAL again. The spec states:

License:	GPLv2+ and BSD and MIT and IBM

> [!]: 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.
>      Please ask upstream to include BSD, MIT and IBM license.

  I can open a trac asking for clarification on this. Possibly asking
to use the debian copyright file? The debian copyright file already
have a very detailed description for every file and special cases.
LICENSE.txt states:

"""
Some files don't yet have licensing details specified - if you care about any
in particular, let us know and we can try to clarify it.
"""

that should refer to the "various".

> [!]: SHOULD Packages should try to preserve timestamps of original installed
>      files.
>      You may use install -p to achive this.

I will correct it, in a quick view it is not preserving 2 time
stamps, of a png file and from the manual page I wrote. The
others should not matter as they are generated during package
build.

ad bundled libraries: ok then.
 So libAtlasUI.so is 0ad specific
 and libCollada.so is fork with some differencies from old library and until fcolladaCE is mature and upstream of 0ad will migrate I'm fine with having it packaged together.

ad licenses:
  it should be easyily distinguishable in .spec which part is under which license
put in .spec something like:

# MIT license:
#   source/lib
#   librariries/fcollada
# BSD license
#   ....
# GPLv2+:
#    everything else
License:    GPLv2+ and BSD and MIT and IBM

I could not log in <http://trac.wildfiregames.com/login> right now,
tried resetting password too, so did not make a suggestion of using
debian/copyright information in LICENSE.txt.

I did not add LGPLv2+ neither MPL-1.1 to the License tag, but now
added it in the comments because the source is in the tarball,
besides not used/built. Should the license tag be extended?

New package:

- Clarify source tree licenses information in spec (#818401)
- Preserve time stamp of installed files (#818401)

Spec URL: http://fedorapeople.org/~pcpa/0ad.spec
SRPM URL: http://fedorapeople.org/~pcpa/0ad-0.0.11-3.fc19.src.rpm

The License tag should reflect the binary package, not the SRPM, so this is correct.

All my comments have been addressed. Approved.
But please during every rebase do licence check.

APPROVED

(In reply to comment #46)
> All my comments have been addressed. Approved.
> But please during every rebase do licence check.
> 
> APPROVED

Thanks. Ok, I will also make a comment in the spec
file about the recomendation to license check on
updates.

New Package SCM Request
=======================
Package Name: 0ad
Short Description: Cross-Platform RTS Game of Ancient Warfare
Owners: pcpa
Branches: f16 f17 f18
InitialCC:

Git done (by process-git-requests).

(In reply to comment #46)
> All my comments have been addressed. Approved.
> But please during every rebase do licence check.

While making final adjustments I remembered I commented I
would open a trac ticket about LICENSE issues, but trac was
not working at that time... Now done :-)
http://trac.wildfiregames.com/ticket/1682

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

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

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

Don't forget to add it as a member to the games group in comps. You can already do it for rawhide. It should be also done for other releases as it makes it to stable / updates.
Thanks for packaging it.

0ad-0.0.11-3.fc18 has been pushed to the Fedora 18 testing repository.

(In reply to comment #54)
> Don't forget to add it as a member to the games group in comps. You can
> already do it for rawhide. It should be also done for other releases as it
> makes it to stable / updates.
> Thanks for packaging it.

Added to, and pushed comps-f19.xml.in
Once it reaches stable/updates of f16, f17 and f18 I will update those.

0ad-data-0.0.13-1.fc18,0ad-0.0.13-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/0ad-data-0.0.13-1.fc18,0ad-0.0.13-1.fc18

0ad-0.0.13-1.fc17,0ad-data-0.0.13-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/0ad-0.0.13-1.fc17,0ad-data-0.0.13-1.fc17

0ad-0.0.13-1.fc19,0ad-data-0.0.13-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/0ad-0.0.13-1.fc19,0ad-data-0.0.13-1.fc19

0ad-0.0.13-1.fc17, 0ad-data-0.0.13-1.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

0ad-data-0.0.13-1.fc18, 0ad-0.0.13-1.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

0ad-0.0.13-1.fc19, 0ad-data-0.0.13-1.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.