Bug 508836

Since you want a legal review, I'll block FE-Legal so they can take a look.

I did a trademark search for the mark "Titan" with "game" anywhere in Goods &  Services and found nothing applicable, which is surprising.

I can find no reason for this package to be arch-specific.  It's just java, isn't it?  Maybe if you were doing AOT compilation, but there are no gcj calls in the spec and nothing arch-specific that I can find in the final package.

The javadoc package is also completely empty, save for one empty directory.

If I make the package noarch, do I need to take steps to block it from being compiled by gcj?

I am not sure about javadoc. I don't believe the game is meant to export an interfaces to other stuff. There are important internal interfaces as it's client server and development is being done on a game lobby. But I don't know if that should show up in javadoc or not.

When I watch the javadoc build I do get some warnings about references to stuff from jdom. I don't know if that's really an issue or not.

"Titan" is currently being sold. It's a small operation, so that may not have paid to register the trademark. Also they are using new artwork. The guy who did the original artwork is kind of reclusive and they were going to want a more modern look. The original game had one color sheets.

Just to track this here, I noticed that the icon cache gets updated after javadoc is installed. If I end up needing to keep javadoc, I'll want to figureout how to attach the post and postun scripts just to the base package.

Further note, the scripts aren't actually in the javadoc package, the output from yum just made it look that way.

(In reply to comment #2)
> If I make the package noarch, do I need to take steps to block it from being
> compiled by gcj?

I'm not sure I understand the question.  If you don't call the compiler in your %build section, it doesn't get called.  However, if you want to look into it, the guidelines are at http://fedoraproject.org/wiki/Packaging/GCJGuidelines.  They say that you should (but aren't required to) call aot-compile-rpm.  In that case your package does need to be arch-specific.

What I am not sure of is if someone does a build with the gcj stuff installed, but not openjdk stuff, can they get arch dependent output? If that can't happen without doing something specific to make it happen, then I definitely should build noarch.
I'll plan on switching to noarch and doing some more reading.

I'll also do some more reading about javadoc. I thought this was pretty much required, but it really doesn't make much sense for stand alone programs.

(In reply to comment #6)
> What I am not sure of is if someone does a build with the gcj stuff installed,
> but not openjdk stuff, can they get arch dependent output?

No.  Your package would have to call aot-compile-rpm.

I asked the java folks and it seems that the current state of affairs is that if you don't do the aot stuff as outlined in the GCJGuidelines document, at least ARM and PPC architectures will run the code very slowly.

> I'll also do some more reading about javadoc. I thought this was pretty much
> required, but it really doesn't make much sense for stand alone programs.  

It only makes sense if the upstream code actually has javadoc content; yours doesn't seem to.  (Or at least it isn't generated by the build process.)  And I agree, I don't see what point javadoc would have for an standalone application.  End-user documentation would be better, but probably wouldn't be packaged separately unless it was very large.

Speed isn't that big of a deal for this app, so I think being arch independent is the way to go.

There is some documentation included, and I made that part of the main package. It doesn't include the rules, but that would require someone writing a free set. A draft set was provided on BGG to get proofed before the rerelease, but I don't think it is free and it has some errors. I maintain some errata and clarifications, but they don't replace the base rules.

Anyway I'll take out the javadoc and put together another source rpm to look at tonight. I'll also resync to the latest SVN. I don't plan on tracking SVN releases continuously once the game is really packaged. The colossus projects do public builds of snapshots on an irregular basis when they think things look good, especially before getting ready to break things for new features. They'll probably be once this summer and how things go with Fedora might affect that.

Thanks for clarifying things. I kind of started out with a moderately hard (java + legal) package for my first one, but I have a lot of interest in this particular one.

The last time I looked at this I don't think I thought of redefining a repo in the live base ks to point a repository that didn't match its name. Since it only defines one repo, I can just override it and that will solve my problem. In the past I tried some things to undefine a repo but never figured out how. Just for completeness is there some way to do that, or is redefine the only solution?

Skip that last comment, it was meant for another bug.

I double checked the javadoc rpm and there is stuff in it. rpm -ql colossus-javadoc lists 400 files. So for now I'll just switch the arch to noarch.

Looking through some archives on the devel-java lists suggests that gcj builds are preferred so I should at least try to get one to work. If I can, I'll do that, otherwise the fallback is noarch.

I updated the spec file and put up an updated source rpm at:
http://wolff.to/bruno/colossus-20090701-1.4427svn.fc11.src.rpm

I have included the aot stuff. I did a build and it seemed to handle the aot stuff and I was able to run the program OK.

I am not sure why your javadoc file didn't have anything in it. Could I be missing a buildrequires for something?

I no longer get the warning about no binary, but do get one about libdir in a noarch package. I suspect this is a false positive where rpmlint doesn't know for sure how the conditional macros interact.

I found a redundant requires for java and java >= 1.5 . I'll be updating things on my web server in a bit, but I want to look to see if I am missing anything obvious for javadoc first.

The artwork here is certainly similar, however, it is not an exact copy. I'd prefer a little more variance, but IMHO, there is enough to minimize legal risk here.

In addition, while the "Titan" trademark is in play for a board game, I cannot find any reference to it being trademarked in the software space.

Lifting FE-Legal, as everything seems okay.

Thanks for the legal review.

If circumstances change later, upstream has offered to help with any new artwork. They have already made extra legion markers for the variants and those are where things are the closest.

The upstream project is known as Colossus, so excising "Titan" won't be a huge burden if that becomes necessary.

I'll go ahead and take this for review, and eventually sponsor you when we get to that point.  I'll wait until you have that new package up before doing a full review, but I can make a few comments:

The package in comment 13 does build OK; the javadoc package still comes out empty but I suppose you're still working on that.  The AOT bits compile OK (with proper debuginfo generated) and everything looks to be in the proper place.  I'll install and test this when I get home.

The package versioning isn't proper, however.  First, is the version really "20090701" or is that just a snapshot date?  I don't see that any version 20090701 was ever released, so it seems that you've just made that version up.  Has upstream ever made an actual versioned release?  Do you expect that in the future they will release version 1.0?

I suggest you peruse the Naming Guidelines, specifically http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages

My guess is that upstream hasn't actually released any versioned tarball, and so you'd just use a version of 0, making the proper NVR 
  colossus-0-0.1.20090701svn4427
(that's a pre-release snapshot).

If the last downloadable file is really versioned 20081029 and you know that they intend to use dates for versioning in the future:
  colossus-20080129-1.20090701svn4427
(that's a post-release snapshot).

I am still getting stuff in my javadoc. So there is probably something different about my build environment. Are you building if F11? Do you have the openjdk stuff installed? Any other ideas on likely things to be different?

On the versioning, I have been consulting with upstream. They have been using date based releases so far. There was some thoughts that they might switch, but they haven't made a decision to switch yet.

What I have is closer to a prerelease version. I have asked them for guidance here. One possibility is just putting the package in rawhide until their next release. They are thinking about doing one soon. Once that is done I would expect to back port occasional fixes not follow svn. They haven't done much work in released branches in the past. I used a date in the prerelease because they currently use date based versions. Though because I don't know the date of the next release I can't use that.

What they put up, they call public builds. They do this when there is enough new stuff that it seems worth doing. They pick what seems to be a solid snapshot and use that. When it turns out the snapshot wasn't that solid they have done another public build quickly, but normally there are long gaps between releases.

There has been a lot of refactoring going on lately. So the current version is a lot different than what was released in 2008. Also some of the dependencies are different. For example they replaced a dependency on an options parsing package in order to make it easier to get Colossus into Fedora. That dependency wasn't in Fedora and didn't have an active upstream making not very desirable to get into Fedora.

Thank you for helping by doing this review.

I got feedback from one of the developers that might give some guidance.
My request is at:
http://sourceforge.net/mailarchive/forum.php?thread_name=20090706170444.GA13211%40bruno.wolff.to&forum_name=colossus-developers
A reply is at:
http://sourceforge.net/mailarchive/forum.php?thread_name=20090707173746.155260%40gmx.net&forum_name=colossus-developers

I am doing rawhide builds in mock on x86_64.  I did an F11 build in mock, there was no change.  I did a mock scratch build (on dist-f12) and all of the javadoc packages for all architectures are empty:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=1460300

If you are doing builds with rpmbuild instead of doing mock or koji scratch builds, you aren't testing things like your BuildReqires because you almost certainly have many packages installed which won't be installed on the builders.

koji is open to anyone who has a Fedora account, so you can do all of the scratch builds you like.

Yeah, I was using rpmbuild. Removing all of the development packages was going to make that system pretty much unusable for other stuff.
I'll take a look at the koji stuff tonight to see if that gives me a clue as to what is missing from my build requires.
I didn't know that I could using koji for building before I was in the packaging group. That gives me a direction to go in that should be useful. (Both for learning about using Koji and for figuring out the javadoc problem.)

You should really read through
  http://fedoraproject.org/wiki/PackageMaintainers/Join
if you haven't already.

I started looking through the build log. I see a couple of syntax errors while reporting on the javadoc build. In my case I saw lots of warnings, but didn't notice syntax errors. Maybe these aborted the javadoc creation. I have an idea how to test that. If that's the cause, it may be a compiler difference, rather the a build requires difference. I want to test that theory as well.
I saw a couple of other minor things with the path names used, and will go back to the spec file to check for oddities.

And on the versioning topic, below is a link to a thread where I was discussing version naming with the developers:
http://sourceforge.net/mailarchive/forum.php?thread_name=20090627222801.GA26934%40wolff.to&forum_name=colossus-developers

I now think the javadoc issue is caused by different failure modes of which compiler was used to build the javadocs. The way I was running it it wasn't finding jdom for that part of the run. In my case I was getting lots of warnings, but I still got output. However I suspect that on koji this caused issues with lists being used in for statements that resulted in the process failing. Next is trying to get my updated version run in koji to test this theory.

It looks like some 1.6ism's crept into part of the sources. Probably for something I am not building (possibly the test suites) since the program builds.
The project has intended to have 1.5 as the minimum version and I'll get back to them about this.
In the meantime having the spec file require java 1.6 produces a reasonable x86_64 (I figured I only needed one arch for testing.) build. See:
https://koji.fedoraproject.org/koji/taskinfo?taskID=1460797

The feedback from the developers is that this is the standard enhanced for loop introduced in 1.5. This code should be used with the normal build, so the problem is probably a javadoc bug. These are the only two places in the code where final is used with an enhanced loop.
For now, as a workaround, I'll require java 1.6. But I'll also file a bug to see if we can get this fixed.

Bug 510243 has been filed against gjdoc regarding the javadoc issue.

I did a test build against all of the primary architectures and didn't see anything that looked like a problem in the log files. All the arches produced rpm files as expected. I don't have a way to all of the arch's rpms, but I did test the i586 rpm and it looked OK.
The build is:
https://koji.fedoraproject.org/koji/taskinfo?taskID=1461513

I think that leaves pretty much just the version name as a blocking issue. I didn't see guidance on prerelease names for date based version names. For now I only want to build in rawhide as the project would like me to wait for the next public build before putting in released versions of Fedora.

I asked for some more feedback from the packaging list and so far the best suggestion seems to match your treat it as version 0 suggestion. I think with the possibility that the project will start doing versioned releases in the future and the issue with doing prerelease naming with date based version it's better to go that route. I can still get their date / SVN snapshot number in the Fedora release (once it happens) to make it clear which public build the Fedora release corresponds to.
Barring some new information that suggests doing something differently, I'll get another test build up tonight with the above version-release naming scheme as well as using the latest snapshot from the project.

I changed the version to version 0, got the latest snapshot and tested building on f10, f11 and f12. The builds all succeeded, though I haven't looked through all of the build logs. I also tested the i586 rpms build on f11 and things looked OK.
If there is anything else I need to do yet, please let me know.
The scratch builds are:
https://koji.fedoraproject.org/koji/taskinfo?taskID=1466728
https://koji.fedoraproject.org/koji/taskinfo?taskID=1466734
https://koji.fedoraproject.org/koji/taskinfo?taskID=1466733

You should post links to the new spec and src.rpm so that I can take a look.

http://wolff.to/bruno/colossus-0-0.1.20090710svn4432.fc11.src.rpm
http://wolff.to/bruno/colossus.spec

I had mistakenly assumed you would be pulling those from the scratch builds once I started doing those.

Well, not only am I lazy, but:
  I have over 40 other reviews in progress.
  I didn't know which of your scratch builds you wanted me to actually review.
Now I know.

rpmlint says:
  colossus.src: W: strange-permission colossus-gen-tarball.sh 0755
I've never understood why rpmlint cares about this.

  colossus.src:172: W: libdir-macro-in-noarch-package (main package) 
   %attr(-,root,root) %{_libdir}/gcj/%{name}
rpmlint doesn't understand BuildArch being conditional.

So both of those are OK.

You should use %{version} in your BuildRoot: somewhere.  I'm not sure why you've used %{revdate}.  It shouldn't really matter, but you've obviously started with one of the recommended values so I don't quite understand why you'd change it.  This is the only thing I see that needs fixing.  It's so trivial that I'll go ahead and approve this now and you can fix it when you check in.

The only other thing I can say is that most packages you'll see have the scriptlets down before the %files list.  Not sure why, but it seems strange to see them near the front.

* source files match upstream (manually compared).
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.                                                              
* description is OK.                                                          
* dist tag is present.                                                        
X build root.
* license field matches the actual license.                                   
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  colossus-0-0.1.20090710svn4432.fc12.x86_64.rpm
   colossus.jar.so()(64bit)
   colossus = 0-0.1.20090710svn4432.fc12
   colossus(x86-64) = 0-0.1.20090710svn4432.fc12
  =
   /bin/sh
   coreutils
   java >= 1.6
   java-gcj-compat >= 1.0.31
   jdom
   jpackage-utils
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libgcj_bc.so.1()(64bit)
   libz.so.1()(64bit)

  colossus-javadoc-0-0.1.20090710svn4432.fc12.x86_64.rpm
   colossus-javadoc = 0-0.1.20090710svn4432.fc12
   colossus-javadoc(x86-64) = 0-0.1.20090710svn4432.fc12
  =
   colossus = 0-0.1.20090710svn4432.fc12
   jpackage-utils

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (gcj and icon-cache).
* code, not content.
* %docs are not necessary for the proper functioning of the package.

Java-specific bits:
* no pre-built jars
* single jar, named after the package
* jarfiles are under _javadir.
* javadocs are under _javadocdir.
* ant called properly.
* wrapper script provided.
* gcj called properly.

APPROVED

I'll look at the version issue before finishing up. I am pretty sure this is an artifact of switching version to revdate in many places, but not leaving it in someplace I should have. revdate is supposed to get used as part of the source package name and as part of the release name. Anything that is really version should be using version.

I'll move the scriptlets to be more conventional.

Thanks for doing the review! I saw you were approving others earlier today and was hoping you'd find time to get to mine today. (Because I am anxious, not because it really needed to get done soon.)

I had the most to check with this ticket, so I left it for last.

You should make your CVS request now; if I'm still up, I'll take care of it tonight.

I made the changes. I moved the scriptlets down and changed the incorrect replacement of version to revdate back to version. When I do the check in, those changes will be made. (I reran rpmlint again and didn't appear to make any typos.)
The next comment will be the cvs module creation request.
The only thing odd is that I don't want to accidentally push a copy to F10 or F11 by mistake. It looks like if I don't do the import -b until I am ready for that I should be OK?

New Package CVS Request
=======================
Package Name: Colossus
Short Description: Allows people to play Titan against each other or AIs
Owners: bruno
Branches: F-10 F-11 F-12
InitialCC: tibbs

Please use a local case 'c' in colossus. I capitalized the name out of habit; the package actually is setup as all lower case.

I've set up the package with a lowercase 'c'.  Note that it will still be a while before we can do F-12 branches, and that I prefer not to be CC'd on packages that I don't own.

Otherwise, CVS done.

You won't push anything to F-11 or F-10 unless you check a package in on one of those branches, then do a build, then issue an update to push the package to updates-testing, and then push the package to updates.  That's quite a bit to do accidentally.

Yeah I read further and it looks like it is pretty safe to create the branches.
I thought sponsors might need to watch for a while. I'll ask questions if I get stuck.
Since this should be in the games group I have checked out comps. I was planning to update the F12 one as soon as I have the package in devel. Should I wait in F10 and F11 until I have made my bodhi push or is some other timing preferred?

I have imported colossus into F10, F11 and devel. A build in devel is running now. I won't be running builds in F10 and F11 until the next public release. I have added colossus to the games group in the F12 comps file. I added colossus to the games spin kickstart file. I have updated the games spin page to list colossus.

Since colossus is in rawhide now, I think it is safe to close this bugzilla entry.

colossus-0.9.0-1.20090810svn4482.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/colossus-0.9.0-1.20090810svn4482.fc11

colossus-0.9.0-1.20090810svn4482.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/colossus-0.9.0-1.20090810svn4482.fc10

colossus-0.9.0-1.20090810svn4482.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8464

colossus-0.9.0-1.20090810svn4482.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8486

colossus-0.9.0-2.20090810svn4482.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/colossus-0.9.0-2.20090810svn4482.fc11

colossus-0.9.0-2.20090810svn4482.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/colossus-0.9.0-2.20090810svn4482.fc10

colossus-0.9.0-2.20090810svn4482.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8676

colossus-0.9.0-2.20090810svn4482.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8690

colossus-0.9.1-1.20090817svn4489.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/colossus-0.9.1-1.20090817svn4489.fc11

colossus-0.9.1-1.20090817svn4489.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/colossus-0.9.1-1.20090817svn4489.fc10

colossus-0.9.1-1.20090817svn4489.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8712

colossus-0.9.1-1.20090817svn4489.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8722

colossus-0.9.1-2.20090817svn4489.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/colossus-0.9.1-2.20090817svn4489.fc11

colossus-0.9.1-2.20090817svn4489.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/colossus-0.9.1-2.20090817svn4489.fc10

colossus-0.9.1-2.20090817svn4489.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8782

colossus-0.9.1-2.20090817svn4489.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update colossus'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8783

colossus-0.9.1-2.20090817svn4489.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

colossus-0.9.1-2.20090817svn4489.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.