Bug 812758

Sorry, I forgot to mention that I need a sponsor, as this will be my first Fedora package.

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. :-)

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...

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.

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

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...

It's no problem.  I'll be out of the country in a couple of weeks as well.  This ticket isn't going anywhere.

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...

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?

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?

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

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!

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!

Reassigning back to nobody, so it get more visibility from other sponsors.

> 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/

I can take your review request to become a sponsor if you agree to do some informal reviews.

# 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".

# Author: John Zaitseff <J.Zaitseff.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.

(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.

(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.

(In reply to William Moreno from comment #18)
> # Author: John Zaitseff <J.Zaitseff.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.

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.

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.

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.

Any update?

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.

Are you still interested in this review?

For the record, including a .desktop file is a MUST item on the review guidelines (https://fedoraproject.org/wiki/Packaging:ReviewGuidelines), so I think you should include one even if it's not yet translated.

Including AppData is a SHOULD according to the packaging guidelines (https://fedoraproject.org/wiki/Packaging:AppData). Personally I would include one even if it's not yet translated and then add the translations in a later update.

Anyway, I'd be happy to finish this up: I cannot sponsor you, but I can at least approve this package, at which point you can apply for a sponsor via https://fedorahosted.org/packager-sponsors/.

I am not sure why I never received comment 27.  I apologise: life has been extremely busy the past year, but I am able to get back to this package.

To answer concerns in comment 27 and 22, I have added a .desktop file to the latest release of Star Traders, as well as matching icons.  I have not, at this stage, added an .xml.appdata file.  I have also used the %license macro in the spec file:

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.10-1.fc25.src.rpm

x86_64 binary URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.10-1.fc25.x86_64.rpm
i686 binary URL: ftp://ftp.zap.org.au/pub/trader/unix/binary/fedora/trader-7.10-1.fc25.i686.rpm

Could someone please review (in light of previous comments) and, hopefully, approve.  Thanks!

Five year old review ticket still alive? Yeah, I'll take this one for the karma. :)

John, please tell me what your Fedora Account System username is so I can sponsor you.

== Review ==
Good:

- rpmlint checks return nothing
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv3+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (b769379a3bd03f3c742e80aacc4160bd60935c84882f0b1b841096acd88df114)
- package compiles on F26 (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- .desktop file okay

APPROVED.

You are now here in this process:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join#Get_Sponsored

I will sponsor you once I know your FAS username.
Questions, comments, help needed? Let me know.

Thank you very much, Tom!  At last we make progress :-)

My FAS username is "zaitseff".

(In reply to John Zaitseff from comment #30)
> Thank you very much, Tom!  At last we make progress :-)
> 
> My FAS username is "zaitseff".

You are now sponsored. :)

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/trader

I've imported the sources and done a "fedpkg build" as per the documentation.  Is there anything more I need to do for this review request?

The documentation at https://fedoraproject.org/wiki/Join_the_package_collection_maintainers doesn't mention anything more to do, at least to my reading: it seems to imply the rest is automatic--but I don't want to assume anything!

(In reply to John Zaitseff from comment #33)
> I've imported the sources and done a "fedpkg build" as per the
> documentation.  Is there anything more I need to do for this review request?

A few things:

You're at this step now:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Import.2C_commit.2C_and_build_your_package

I can see that you have a successful build in rawhide (f27), but not for any released branches. You probably want to do builds for the maintained versions of Fedora (f24, f25, f26). To do that, you'll need to request those branches in pkgdb: https://admin.fedoraproject.org/pkgdb/package/rpms/trader/

Once those branches are created, go to the next step:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Update_Your_Branches_.28if_desired.29

Then continue on and make updates for those branches in bodhi (no update is required for rawhide). You can attach this bug to those updates so that it will close when the updates land. Remember, updates go first to "updates-testing", then "updates" when the karma count hits (or when it hits the testing timeout). You'll get email notification when either case happens, so you can go back and push the update to stable.

Hopefully that makes sense. :)

(In reply to Tom "spot" Callaway from comment #34)
> Hopefully that makes sense. :)

It does, thanks!  The linked instructions said "Update Your Branches (if desired)" (section title); your instructions make it clear that such a thing is indeed desirable.

trader-7.10-2.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-59d8832418

trader-7.10-2.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-697e3c9cea

trader-7.10-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-2d2cc6027d

trader-7.10-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-2d2cc6027d

trader-7.10-2.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-697e3c9cea

trader-7.10-2.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-59d8832418

trader-7.10-2.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-132cea1f3a

trader-7.10-2.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-1e4cce6a22

trader-7.10-2.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-1e4cce6a22

trader-7.10-2.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-132cea1f3a

trader-7.10-2.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

trader-7.11-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-afdec15340

trader-7.11-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-65aeafd42a

trader-7.11-1.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-5b2b1cf40a

trader-7.11-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-7577d9937c

You don't need to CC this ticket on updates made to your package after the initial release.

(In reply to Jason Tibbitts from comment #51)
> You don't need to CC this ticket on updates made to your package after the
> initial release.

True, but this was done automatically: the initial package had not made it through to the stable repository before I uploaded the bug-fix version.  The act of uploading version 7.11-1 made the original packages obsolete, and the new package inherited this ticket.

trader-7.11-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

trader-7.11-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

trader-7.11-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.

trader-7.11-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.