Please package the minimal data set seperately. It may get updated by upstream
together with the engine, but we might want todo local / Fedora bugfix releases,
in which case most of the time we usually only need to rebuild the engine, thus
packaging the data seperately leads to significant bandwith savings.

Did I mention it will build but will not install properly using scons without
one or the other data sets in place?  I'd prefer to package separately as well,
but I just don't see how.  I suppose I could manually install, but that seems ugly.

(In reply to comment #2)
> Did I mention it will build but will not install properly using scons without
> one or the other data sets in place?

No you didn't that might be an argument, how will this interact with the full
data package? Will the minimal data be in a subpackage and the full data package
and that subpackage conflict each others, or ... ?

Also how big is the minimal data? And how hard is it todo a manual install?

It looks like Full is comprehensive and so would conflict with Minimal, unless I
tweaked the spec for Full to only include the unique data, which would be wise
IMHO.  Minimal = 12MB, Full = 220MB.  I'll not be packaging Full until after
4/13, when my bandwidth improves. :)

Oh, and I was just looking,and that's the bz2 compressed size.  Full is actually
339MB. :)

Repeating myself (info needed to decie wether to seperate the minimal data into
a seperate package or not):
And how hard is it todo a manual install?

Oops.  Sorry.  So far as I can tell, it would just be a matter of copying to the
right place.  So we could build the engine with scons, and do a manual install.
 I think.  I'd have to test.  Would this method be your preference?

On another note, you'll see that this package installs things in /share/games,
which totally breaks from other games packages.  Will this be a problem?  I
experimented with moving the binary and the data and editing the SConstruct
files and using patches, but nothing worked.  Have you any experience with scons?

(In reply to comment #7)
> Oops.  Sorry.  So far as I can tell, it would just be a matter of copying to the
> right place.  So we could build the engine with scons, and do a manual install.
>  I think.  I'd have to test.  Would this method be your preference?
> 
Yes, since we can then have 2 seperate packages, so that people do not have to
download 12Mb each time we fix a bug in the engine. (I know they still need it
for new upstream releases)

> On another note, you'll see that this package installs things in /share/games,
> which totally breaks from other games packages.  Will this be a problem?  I
> experimented with moving the binary and the data and editing the SConstruct
> files and using patches, but nothing worked.  Have you any experience with scons?

Only very limited experience with scons, I'm sure this can be fixed, and IMHO it
should be fixed. You could try contacting che from newrpms, he has a wip package
of vdrift too, he send it to me but I lost it :( . I'm afraid I'm currently
rather busy with all kinda stuff so I cannot be of much help here.

I've started a dialog with che.  Should I go ahead and file the review for
vdrift-data-minimal, or can it be handled in this bug?

(In reply to comment #9)
> I've started a dialog with che.  Should I go ahead and file the review for
> vdrift-data-minimal

Yes please.

> or can it be handled in this bug?

No seperate bug please, Thanks!

Spec URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift-20070323-2.src.rpm

Split out vdrift-data-minimal.  Review BZ here:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=235249

Adding che666, still needs work on path correction/scons.

Working on full data set RPM.  Will submit for review once my bandwidth improves
on 4/13/07.

Created attachment 152325 [details]
fix for vdrift scons files

build:

CC="gcc" CXX="g++" CXXFLAGS="%{optflags}" scons destdir=$RPM_BUILD_ROOT bin=/bin
datadir=/share/%{name}/data prefix=/usr use_binreloc=0


install:

install -d %{buildroot}%{_bindir}
CC="gcc" CXX="g++" CXXFLAGS="%{optflags}" scons install destdir=%{buildroot}
bin=/bin datadir=/share/%{name}/data prefix=/usr use_binreloc=0
mv %{buildroot}/usr/share/games/vdrift/bin/vdrift %{buildroot}%{_bindir}

Rudolf, these are very helpful, but the scons install fails due to lack of data,
as that's being package separately.  We either need to manually install the
files created with scons in the build step, or somehow trick scons into not
checking for data.  I'd prefer the latter, but I can do the former if it's
non-trivial.

To make scons install work without data, simply comment out the line

SConscript('data/SConscript')

in the file SConstruct. Should I add a SCons option to install no data to make
packaging easier?

(In reply to comment #12)
> Created an attachment (id=152325) [edit]
> fix for vdrift scons files
> 

I've checked this into VDrift SVN...

(In reply to comment #15)
> To make scons install work without data, simply comment out the line
> 
> SConscript('data/SConscript')

To clarify, you should comment out lines 355 and 356 in SConstruct.


By the way, reading through the delimma about packaging above, it seems it would
be handy to have a way for SCons to only install the data unique to the full
data package.

Also, it might make more sense to call the RPMs "vdrift",
"vdrift-required-data", and "vdrift-extra-data", or something along those lines,
instead of just full or minimal.

One more thing...in at least the build section you should probably add
"release=1" as another option to scons.

Jon, this might give you a tiny bit more info on our SCons system:
http://wiki.vdrift.net/index.php/Using_SCons

If you have any questions about the page let me know, the info that's there is
very basic but should give you a starting point...

if you want lower case names theelusive then i do suggest to also name the
tarballs lower case in the next iteration. it will make things somewhat more
trivial. otherwise you will end up with the upper/lower case mix like above.

I'm not so concerned about case, but I thought that most package were named
after the "short name" of the application anyway. Also our tarballs are named in
lowercase, only the .package files have uppercase names. Is this what you're
talking about? 

Also, can't believe I forgot about this. There's no need at all for that patch
to import CC/CXX/CXXFLAGS. I added options to do just this long ago, just for
this purpose. Just add these options to scons during build:

scons os_cc=1 os_cxx=1 os_cxxflags=1

last time i looked though those paths were hardcoded and that means that ccache
/ distcc wont be used :)

which cc
/usr/lib/ccache/cc

sorry if it just looks in PATH now, could be.

There are no hard-coded paths. You will still have to put "CC=gcc CXX=g++
CXXFLAGS=..." before running scons for them to apply the desired effect. These
options do the same thing your patch did:

#----------------------#
# OS compiler settings #
#----------------------#
if env['os_cc']:
    env.Replace(CC = os.environ['CC'])

if env['os_cxx']:
    env.Replace(CXX = os.environ['CXX'])

if env['os_cxxflags']:
    env.Append(CXXFLAGS = os.environ['CXXFLAGS'])


http://svn.vdrift.net/viewvc.py/trunk/SConstruct?view=markup

I applied changes from #17, #18 and #21.  Build breaks. . .
Checking for C header file libintl.h... yes
KeyError: 'CC':
  File "SConstruct", line 291:
    env.Replace(CC = os.environ['CC'])
  File "/usr/lib/python2.4/UserDict.py", line 17:
    def __getitem__(self, key): return self.data[key]
error: Bad exit status from /var/tmp/rpm-tmp.67321 (%build)

Do I need latest SVN for this?  I'm not sure I've got the full gist of what
changes I need to make beyond these.

Did you remember to run scons with the environment variables before it, as che
suggested? Like:

CC="gcc" CXX="g++" CXXFLAGS="%{optflags}" scons os_cc=1 os_cxx=1 os_cxxflags=1
destdir=$RPM_BUILD_ROOT bin=/bin datadir=/share/%{name}/data prefix=/usr
use_binreloc=0

I misunderstood, I thought os_cc=1. . .was to replace CC="gcc". . ., not
complement.  That corrected, the build gets a lot farther. :)

Then dies here:

scons: done reading SConscript files.
scons: Building targets ...
guilists_to_cppfile_builder(["po/guilists.cpp"], [])
[<SCons.Node.FS.File instance at 0xb7cc5ccc>]
scons: *** Source `data/settings/options.config' not found, needed by target
`po/optionslist.cpp'.  Stop.
scons: building terminated because of errors.

Patch to comment out part of po/optionslist.cpp, or. . .?

Since our translations are extremely limited and not being included in the
package anyway, just turn them off at build time, by adding the "NLS=0" option
to the scons argument list. This should probably cover it.

Er, well I guess they *were* being included before, but they are very limited as
I said. We only have a Belgian translation and it hasn't been updated in a while
so it's incomplete anyway...just turn it off. :)

Ah.  That worked much better:

Spec URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift-20070323-3.src.rpm

One possible suggestion: mouse driving was broken in the Linux source release.
We've since fixed it, it's very simple, just replace mouse.cpp with the one in
SVN here:
http://svn.vdrift.net/viewvc.py/trunk/src/mouse.cpp?revision=1638

Besides that, what's the status on this package?

Spec URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift-20070323-4.src.rpm

Patched.

Jon, maybe we can exchange reviews? I myself also have several game packages
awaiting review:

* asc-music - Background music for the game asc -
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233256 - requires asc
* vegastrike - 3D OpenGL spaceflight simulator -
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233782
* vegastrike-data - Data files for Vega Strike -
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233783
* ballz - Platform game with some puzzle elements -
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=236798

That's great, but it looks like they all have reviewers assigned, though it
looks like Andrea and Peter haven't done much in awhile.  Have you looked at
http://fedoraproject.org/wiki/Extras/Policy/StalledReviews?

it would probably make sense to prefix the release tag with 0.0.0. just incase
at some point theres a switch to a regular versioning scheme.

Release or Version?  Would not 0.0.0.20070323-3 be better than 20070323-0.0.0.3?

yeah exactly. to prefix means to put it beforehand :)

so i completly agree with you.

(In reply to comment #33)
> That's great, but it looks like they all have reviewers assigned, though it
> looks like Andrea and Peter haven't done much in awhile.

Jon, I've added a comment to bug 233782, asking Peter if he has time for this
and if not, if he would be okay with someone else reviewing it.

I'm currently a bit too busy todo 100% volunteer reviews I'm afraid, otherwise I
would love todo this as I think vdrift is a great game to get into Fedora.

Do you think vdrift license is OK for Extras ?

Please read : http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=420965

Some content seems to be EA games property.
Debian have choose to remove it from their repository.

Regards.

Hmm.  A quick look at the link from the bug shows no content from the "not ok"
list in the -minimal package.  If we can verify the content from -minimal, we
could push that and just nix the -full package.

How do I verify the status of the material in question?  Do I ask upstream to do it?

You could ask upstream, and/or you could go mailing authors of car models/
tracks yourself. For the EA tracks the solution is simple: don't distribute them.

For now I would just rm -r all the dirs that contain content marked unsorted and
not ok (so that the entire car / track gets removed), and then recreate the
tarbal and use the _new_ tarbal as Source0 .

Well, crud.  That wipes out the whole data-minimal package.  I suppose I should
close both reviews?  I don't see much point in pursuing this with upstream with
so many sources.  I guess I could do a release with just the approved material,
but two cars and no tracks is not a recipe for a good race. :)

Either close both reviews, or you could try to contact some of the non EA track
authors. If we can get a few decent tracks then we can still ship vdrift IMHO. @
cars is not much, but I concider it enough.

Also you could try asking upstream, maybe they can help to clearify some of the
content marked unsorted by the Debian review

I think I'll just contact upstream.  They'd probably appreciate the heads-up
froma legal front.  The only thing that bothers me is that the potentially OK
tracks are in the Full package.  Imagine having a +200MB SRPM for 2 cars and a
handful of tracks. . .

Can't find specific contact info:

vdrift forum post:
http://vdrift.net/Forum/viewtopic.php?t=665&start=0&postdays=0&postorder=asc&highlight=

(In reply to comment #43)
> I think I'll just contact upstream.  They'd probably appreciate the heads-up
> froma legal front.  The only thing that bothers me is that the potentially OK
> tracks are in the Full package.  Imagine having a +200MB SRPM for 2 cars and a
> handful of tracks. . .

That (200MB SRPMs) won't happen, as we cannot ship any of the tainted stuff in
any way, iow you must create your own tarbals for inclusion into the srpm, which
will only include the allowed stuff and thus be small.

Then in the spec file you put a little howto above Source0 as comment which
explains how to recreate the tarbal from the official tarbal, or you add a
script to the source rpm as SourceX, which can be used by people who want to
recreate the shipped / srpm tarbal from upstreams original tarbal.

Duh. :)  I knew that.  I'm thinking about doing the same thing with Roundcube if
upstream doesn't re-release soon.

Discussion in VDrift forum indicates that currently, 4 cars are known clean and
2 tracks are of dubious legality.  The next release will have all clean data. 
Getting an ETA. . .

Any news on the next all clean data release?

Pinged upstream, it's coming, I'm waiting on an ETA.

Jon, any news on this?

Nothing on the forum, and no new releases.  Last I checked, they had some cars
ready but no real progress, at least that I could see, on tracks.

Bummer. Have you tried pinging them?

Can you get those cars from CVS? If we were to use whats available now, how many
tracks would we have?

I'll ping, and ask the cvs question.  I'd do it now, but my forum account got
locked out due to a fat-fingered password.

From upstream:
-----------
No new release yet, and no new GPL cars/tracks yet. We're working on
something though. It will probably be a while yet before another
release.
-----------

I guess it's time to ping this ticket again.  It would be a shame after all of
this review work to not end up with an actual package in Fedora.

There's a new release out, with new data, which seems to be making an effort at
legality. :)

Spec URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift-20071226-1.src.rpm

Still needs a lot of work, but I've made some basic corrections.  I've updated
the vdrift-data-minimal bug as well, renaming it just vdrift-data, since it's
all one set now.

It now needs and bundles the bullet library, which might be wise to split out
and package seperately.  If all concur, I will do that.

(In reply to comment #56)
> There's a new release out, with new data, which seems to be making an effort at
> legality. :)
> 

Why do you say "an effort" they seem to have done a full audit, atelast judging
from:
http://vdrift.net/Forum/viewtopic.php?p=6896

They seem. Sure they may have missed something, is there anything troublesome
you know about still in there?

> It now needs and bundles the bullet library, which might be wise to split out
> and package seperately.  If all concur, I will do that.

Yes package bullet seperate please, using pristine sources from upstream:
http://www.bulletphysics.com/

I will happy to review all 3 packages.

Sorry, I tend to understate.  A habit I picked up from a college history
textbook, entitled "Problems in Western Civilisation", with a picture of Hitler
on the cover. :)

I applaud the "effort", it must have been a great deal of work.

I'll get bullet packaged and submitted, then post it here.

Hi guys, it's me again, from the VDrift team. Don't bother building a Bullet
package from pristine upstream sources, this will not work for VDrift. There's a
reason why we include the Bullet source with the VDrift source, we are using a
development version with some patches we've applied. I think there were some
performance issues with the ray-casting in the Bullet release and we had to go
to the developers to get this fixed.

Also, there wasn't a great deal of work involved in the tracks. The tracks that
made it into the last release were just community-created tracks for other
games, which we found the authors of and obtained permission to use them. We
have started work on some original tracks, but these are not yet finished, so
they have not been included yet. These include a hillclimb style track ("Indian
Hill" in our data repository), an off-road rally track, a new version of
Zandvoort, and Carolina Motorsports Park. These last three are not yet available
for testing, and Indian Hill needs further work to increase drivability, and add
landscape/scenery objects.

Also, we plan to eventually create an application to go with VDrift to allow the
user to select and download data on their own, so that little or no data needs
to be distributed with the game. This will fix the package license issue once
and for all, but work on this has stalled for the time being.

Feel free to email me or post on our forums if you have any questions. I'm
monitoring these bug reports so I will watch for updates. Thanks for your effort
in packaging VDrift for Fedora.

Thanks for the information, Chris, and thanks to you and your team for the
effort between 3-23 and 12-26.  

Fedora folken, should we bundle bullet into vdrift, or possibly package a
pristine bullet with a bullet-vdrift subpackage with the vdrift patches applied,
so that vdrift can BT bullet-vdrift-devel and other packages could BR bullet-devel?

Am I crazy, or is this a good idea?

(In reply to comment #60)
> Thanks for the information, Chris, and thanks to you and your team for the
> effort between 3-23 and 12-26.  
> 
> Fedora folken, should we bundle bullet into vdrift, or possibly package a
> pristine bullet with a bullet-vdrift subpackage with the vdrift patches applied,
> so that vdrift can BT bullet-vdrift-devel and other packages could BR
bullet-devel?
> 
> Am I crazy, or is this a good idea?

It all depends. Do the patches change ABI / API, are they heading upstream, etc.
I've gone this way before, that is I've packed a bundled patched library
separately before including the patches in the separate build, there this could
be done because the patches we're only bugfixes, so no ABI / API changes (note
that additions are fine, changes are the problem). However since that lib's
upstream was dead, the app including the library was effectively maintaining it
for their own uses and each new release of the application I had todo a new
release of the lib, so I eventually stopped doing this.

So I say since its modified (for good reasons), just use the included version.

Chris (Guirl) have your bullet patches been submitted upstream? Are there any
plans to get them upstream and eventually switch to using a pristine bullet again?

FYI, I'm working on correcting file placement, and generally cleaning up the
packages.  I have a tad more experience under my belt now than I did when I
first created these packages. :)

Spec URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift-20071226-2.src.rpm

Greatly cleaned up, bullet issue still unaddressed.

Created attachment 294952 [details]
PATCH: fixing vdrift compile with gcc-4.3

Full review done ..

MUST FIX
--------

- Doesn't compile with gcc-4.3, patch fixing this attached
- It isn't necessary to pass --libdir=%{_libdir} to %configure
- %doc docs/* is no good, this will include the following unwanted files:
INSTALL HOW_TO_COMPILE.txt SConscript
- remove -32x32 from icon name
- remove obsolte Version and Encoding entries from .desktop file
- Contains some non free files in the sourcetarbal, these must be removed
  and then we (Fedora) must ship our own sourcetarbal without these, all
  troublesome files are under the bullet-2.64, so I'm omitting that from their
  path:
glew32.dll: non free SGI licensed version of glew
GLUT32.DLL: glut is not free software (unlike freeglut)
Glut <directory> contains more non free glew and glut stuff
Extras <direcytory>: atleast the COLLADA_DOM is a problem as that contains
files
 under the SCEA Shared Source License 1.0, which seems fine, but hasn't been 
 audited yet, so its easier to just remove these files
Demos <direcytory>: atleast the BulletDinoDemo is not properly licensed, I
stopped auditing this dir after that, just blow it away

You will also need to remove the following 2 lines from Jamfile.in (again in 
bullet-2.64), to make jam no longer look for the removed dirs:
SubInclude TOP Extras ;
SubInclude TOP Demos ;

Spec URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift-20071226-3.src.rpm

All addressed.

(In reply to comment #65)
> Spec URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift.spec
> SRPM URL: http://zanoni.jcomserv.net/fedora/vdrift/vdrift-20071226-3.src.rpm
> 
> All addressed.

Approved!

Thanks! 

Want to do the -data too? :)

https://bugzilla.redhat.com/show_bug.cgi?id=235249


New Package CVS Request
=======================
Package Name: vdrift
Short Description: VDrift is a cross-platform, open source driving/drift racing
simulation
Owners: limb
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

(In reply to comment #67)
> Thanks! 
> 
> Want to do the -data too? :)
> 

Already on it, almost finished :)

cvs done.

Imported, fixed several BR issues, and now building it fails on ppc64:

http://koji.fedoraproject.org/koji/getfile?taskID=449200&name=build.log

The problem is that the link command for vdrift gets passed:
-Lbullet-2.64/out/linuxx86/optimize/libs
instead of:
-Lbullet-2.64/out/linuxppc/optimize/libs

Various solutions:
-patch configure (learn it about ppc64) (I guess)
-sed the Makefile after %configure (on ppc64 only)
-create a symlink (on ppc64 only)

I've attempted #1 and #3, to no avail.  I can't find where in the Makefile to
patch for #2.

Any suggestions?  If not, i can just ExcludeArch, but I'd rather not.

(In reply to comment #72)
> I've attempted #1 and #3, to no avail.  I can't find where in the Makefile to
> patch for #2.
> 
> Any suggestions?  If not, i can just ExcludeArch, but I'd rather not.

Sorry, my bad, I forgot that vdrift is usin sconstruct not autconf/make.

Adding this to %prep should do the trick:
%ifarch ppc ppc64
sed -i 's/linuxx86/linuxppc/' src/SConscript
%endif

I put it in %setup, and it worked beautifully.  Thanks!

What's that, like, 2 or 3 beers I owe you, next time you're near Chicago? :)

Imported and built for all branches, including F-7, which was rawhide when this
review was submitted. :)

Thanks to everyone involved in making this possible!