Bug 812758

Summary: Review Request: trader - Star Traders, a simple game of interstellar trading
Product: [Fedora] Fedora Reporter: John Zaitseff <J.Zaitseff>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: NEW --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: J.Zaitseff, msuchy, package-review, tcallawa, tibbs
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 177841    

Description John Zaitseff 2012-04-16 03:35:02 EDT
Spec URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec
SRPM URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.3.99.2-1.fc16.src.rpm
Description: Star Traders, a simple game of interstellar trading
Home page: http://www.zap.org.au/software/trader/

Star Traders is a simple game of interstellar trading, where the objective
is to create companies, buy and sell shares, borrow and repay money, in
order to become the wealthiest player (the winner).

---

I originally wrote Star Traders for the CP/M-80 operating system, back in 1990.  As an exercise in pure nostalgia, I have rewritten it for modern Linux/POSIX systems and am packaging it for a number of distributions.  The game has even been translated into a number of languages, including Russian, French, German, Finnish and Croatian!

I would like Fedora to include this simple text-console game as part of its distribution.
Comment 1 John Zaitseff 2012-04-16 03:40:35 EDT
Sorry, I forgot to mention that I need a sponsor, as this will be my first Fedora package.
Comment 2 John Zaitseff 2012-04-16 04:58:29 EDT
To possibly preempt some comments on trader.spec:

1. The file is somewhat convoluted by my desire to have just a single spec file for all RPM-based distributions.  I'm not sure whether Fedora has a policy about that: I've read through the Fedora Packaging Guidelines (as well as How to Create an RPM Package, Package Naming, Frequently Made Mistakes, and others) and could not see anything directly related to this.  As far as I know, I _do_ follow the Packaging Guidelines closely...

2. Running rpmlint on the spec file and on the resulting RPM archives returns a positive result:

    trader.i686: I: enchant-dictionary-not-found en_US
    3 packages and 1 specfiles checked; 0 errors, 0 warnings.

And, yes, I've checked the spelling of the text. :-)
Comment 3 John Zaitseff 2012-05-09 09:13:39 EDT
I have released Star Traders 7.4, and created RPMs for Fedora 16:

Spec URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec
SRPM URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.4-1.fc16.src.rpm

It would be good if someone could sponsor this package...
Comment 4 Jason Tibbitts 2012-06-30 15:19:07 EDT
This is cool.  The spec is... a bit overblown.  To be honest, I'd edit out the comments and the excessive macro-ization to get a spec that's very readable and only 32 lines (excluding the changelog).  In particular your macroization of Release: will interfere with our automatic rebuild stuff.

In any case, this builds fine and rpmlint is silent.

BuildRoot: is unnecessary in Fedora, as is the %clean section and the %defattr in the %files section.

Your source bundles gnulib. That is permitted according to policy but it is required that you include Provides: bundled(gnulib).  http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Packages_granted_exceptions

Is the dependency on gperf necessary?  I can't see anywhere in the build where gperf is called, and when I build without it the package doesn't appear any different.

I'll run through the rest of my checklist.
* source files match upstream.  sha256sum:
  b91e74b0fc5eee7dc61ca2d02e2bf04c64ac1c71bf858b5858fc408bfe9ad33a
   trader-7.4.tar.gz
* package meets naming and versioning guidelines.
X specfile is is a bit messy and has excessive macroization.
* summary is OK.
* description is OK.
* dist tag is present.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
? BuildRequires has extra gperf.
* compiler flags are appropriate.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
X final provides and requires missing bundled(gnulib):

   trader = 7.4-1.fc18
   trader(x86-64) = 7.4-1.fc18
  =
   libncursesw.so.5()(64bit)  
   libtinfo.so.5()(64bit)  

* %check is not present; no test suite upstream.  Manually tests OK.
X bundles gnulib without indicating such.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no static libraries.
* no libtool .la files.
Comment 5 Jason Tibbitts 2012-06-30 15:45:09 EDT
By the way, if I were to submit this, the spec would look something like the following, which is about as simple as a spec can get for a package with translations:

http://www.math.uh.edu/~tibbs/rpms/trader.spec
Comment 6 John Zaitseff 2012-06-30 17:31:28 EDT
Thank you, Jason, for your detailed comments: I do appreciate them!  Unfortunately, you've caught me at a bad time: I will be overseas and out of e-mail/web contact for almost seven weeks, starting from a few hours' time (I'm about to fly out and, yes, there are still places in the world like that).  I will be responding to your comments in detail once I return.

In the meantime, just a few comments:

* I like the simplified RPM spec file, and given that I already have split off OpenSUSE specifics into another file, it will be easy to do, too.

* I did not realise that bundling gnulib had to be declared.  I'm also happy to work out a way NOT to bundle (or, rather, to use the Fedora gnulib instead of the bundled one)---once I return from overseas.

* gperf is used (or, at least, was used) by gnulib to create iconv_open header files.  The current lib/Makefile.am lists the files iconv_open-aix.h,iconv_open-hpux.h, iconv_open-irix.h, iconv_open-osf.h and iconv_open-solaris.h as requiring gperf; I think this list was larger in older versions of gnulib.

I'd like to work on this now, but I have a flight to catch...
Comment 7 Jason Tibbitts 2012-06-30 18:04:02 EDT
It's no problem.  I'll be out of the country in a couple of weeks as well.  This ticket isn't going anywhere.
Comment 8 John Zaitseff 2012-09-20 05:22:18 EDT
After months of being out of Internet access , I'm back and able to work on Star Traders...

I have simplified the RPM spec file per Jason Tibbitts' suggestions; it is now available at: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec

I found that gperf was NOT needed in the current bundled Gnulib after all, so I've removed that build-time dependency.

Could you please review the new spec file and sponsor the package into Fedora?  I realise that it's probably too late to get the package into F18, though...
Comment 9 John Zaitseff 2012-09-20 05:54:02 EDT
Actually, the dependency on gperf(1) MIGHT be needed, depending on timestamps: if any of the files lib/iconv_open-*.h appear to be older than the corresponding lib/iconv_open-*.gperf, then gperf will be called to recreate the .h files.

Given this, should I put a BuildDepends: gperf back into the RPM spec file?
Comment 10 Jason Tibbitts 2012-10-02 18:03:15 EDT
Sorry for not responding sooner; I must have skipped right over your previous two messages.

If there's any chance that gperf is needed, you should definitely add a build dependency on it.

Can you post a both an updated spec and srpm so that I can do one last build and finally get this approved?
Comment 11 John Zaitseff 2012-10-03 20:18:59 EDT
Given that gperf(1) MIGHT be needed (depending on timestamps), I've included it as a build-time dependency.

I've posted the updated packages at:

Spec URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec
SRPM URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.4-3.fc17.src.rpm
Comment 12 John Zaitseff 2014-06-18 00:28:25 EDT
For some reason, both I and others have let this packaging request remain unresolved.  Could someone please include this simple game as part of Fedora.

The latest version is available at:

Spec URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec
SRPM URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.5-1.fc20.src.rpm

x86_64 binary URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.5-1.fc20.x86_64.rpm

Please let me know if I need to do anything to expedite this package getting into Fedora.  Thanks!
Comment 13 John Zaitseff 2014-08-19 20:01:38 EDT
Unfortunately, I have not heard anything from Jason Tibbitts since 2012.  His comments have certainly been helpful, but I'm wondering whether he is MIA.  Could someone else be reassigned this bug?

I have updated the latest version of Star Traders to 7.6:

Spec URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader.spec
SRPM URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.6-1.fc20.src.rpm

x86_64 binary URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.6-1.fc20.x86_64.rpm

Thanks!
Comment 14 Miroslav Suchý 2015-07-21 09:31:46 EDT
Reassigning back to nobody, so it get more visibility from other sponsors.
Comment 15 Michael Schwendt 2015-12-11 07:57:15 EST
> Reassigning back to nobody, so it get more visibility from other sponsors.

Also the "fedora-review" flag MUST be reset for the ticket to appear on the needsponsor queue tracker: http://fedoraproject.org/PackageReviewStatus/
Comment 16 William Moreno 2016-01-06 15:16:43 EST
I can take your review request to become a sponsor if you agree to do some informal reviews.
Comment 17 William Moreno 2016-01-06 15:19:57 EST
# This file is distributed under the same licence as Star Traders itself:
# the GNU General Public License, version 3 or later.

Not, you can not, specs are software for the FPCA

https://fedoraproject.org/wiki/Legal:Fedora_Project_Contributor_Agreement?rd=Legal:FPCA

"Code" means (i) software code, (ii) any other functional material whose principal purpose is to control or facilitate the building of packages, such as an RPM spec file, (iii) font files, and (iv) other kinds of copyrightable material that the Fedora Council has classified as "code" rather than "content".
Comment 18 William Moreno 2016-01-06 15:22:00 EST
# Author: John Zaitseff <J.Zaitseff@zap.org.au>

There is not need to put you as author for the same reason than be do not use the Packager tag in rpm spes, it is very clear than you are the author of the spec looking at your changelog.
Comment 19 John Zaitseff 2016-01-12 15:54:33 EST
(In reply to William Moreno from comment #16)
> I can take your review request to become a sponsor if you agree to do some
> informal reviews.

Absolutely.  Thank you for taking the time to make your comments.
Comment 20 John Zaitseff 2016-01-12 16:02:10 EST
(In reply to William Moreno from comment #17)
> # This file is distributed under the same licence as Star Traders itself:
> # the GNU General Public License, version 3 or later.
> 
> Not, you can not, specs are software for the FPCA
> 
> https://fedoraproject.org/wiki/Legal:
> Fedora_Project_Contributor_Agreement?rd=Legal:FPCA
> 
> "Code" means (i) software code, (ii) any other functional material whose
> principal purpose is to control or facilitate the building of packages, such
> as an RPM spec file, (iii) font files, and (iv) other kinds of copyrightable
> material that the Fedora Council has classified as "code" rather than
> "content".

I am not quite sure what your point is here.  I am the author of both the software package (Star Traders) and (at least for current versions) the SPEC file as well.  All I aim in making the statement "This file is distributed..." is to make the SPEC file be under the GPL 3+.  It is a declarative statement in English: essentially, "I choose that this file is distributed under...".

My reading of https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing suggests that I can license the SPEC file under an explicit licence, in this case GPL 3+: it is only if I don't that the SPEC file comes under the MIT licence.
Comment 21 John Zaitseff 2016-01-12 16:04:46 EST
(In reply to William Moreno from comment #18)
> # Author: John Zaitseff <J.Zaitseff@zap.org.au>
> 
> There is not need to put you as author for the same reason than be do not
> use the Packager tag in rpm spes, it is very clear than you are the author
> of the spec looking at your changelog.

Granted.  However, the "Author:" line is a standard (and grep-able) line in my header blocks for all source code files in all my projects, so although redundant, I would prefer to keep it in if possible.
Comment 22 William Moreno 2016-01-12 16:16:53 EST
I think you can create a .desktop file with terminal=True

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

You SHOULD provide a .xml.appdata file if you not provide the appdata information users will can not find this game in Gnome Software.

https://fedoraproject.org/wiki/Packaging:AppData

You must use the %license tag for the COPYING file. 

Did you read the FPCA, by default all spec files in Fedora are released under the MIT License

"Current Default License", with respect to a Contribution, means (i)
if the Contribution is Code, the MIT License, and (ii) if the
Contribution is Content, CC-BY-SA supplemented by Moral Rights Clause
Waiver and GPL Relicensing Permission."

Any way:

2.  Licensed Contributions.

If Your Contribution is Licensed, Your Contribution will be governed
by the terms under which it has been licensed.

So just include the spec file in the project source and by this way your code and the spec file will be covered by the same license, any way there is not need to put a license adivisory in the spec so all the spec files are under a very permisive license.

Any way we can ask to legal.

So I will request to Legal to please consider if we can continue with this review with the legal notice in the spec about GPLv3.

Any way you can do some informal reviews by the way.
Comment 23 John Zaitseff 2016-01-12 16:23:55 EST
Adding a .desktop file and a .xml.appdata file are probably good ideas.  I'll try to do so in the near future, although I hope it won't hold up the inclusion of the current package (given these are optional, and I want to see if I can make the two files PO-translatable, which may take some time).

Yes, most definitely I have read the FPCA.  The key words are "by default", that is, unless otherwise explicitly licensed.  In this case, the SPEC file _is_ explicitly licensed to GPL 3+, which happens to match the licence of the rest of the Star Traders package.

If Fedora Legal insists on MIT, I can relicense this one file, but I strongly prefer GPL.  By all means ask Legal.
Comment 24 Tom "spot" Callaway 2016-01-12 16:39:28 EST
Using a comment to declare the license on a spec file to be GPLv3+ is perfectly acceptable. The intent of the FPCA is to provide a blanket default license for all contributed works (including spec files) where the contributor does not specify a license. Since you've specified a license, this does not apply. Your choice of license (GPLv3+) is perfectly acceptable to Fedora.

Lifting FE-Legal.
Comment 25 William Moreno 2016-02-12 14:58:42 EST
Any update?
Comment 26 John Zaitseff 2016-02-12 17:41:30 EST
And I was waiting for you (amongst my other busyness) :-)

Can I defer the .desktop and .xml.appdata files to a later update, as I would like these translated appropriately, and that will take time.