Bug 234490 - Review Request: VDrift - VDrift is a cross-platform, open source driving/drift racing simulation
Review Request: VDrift - VDrift is a cross-platform, open source driving/drif...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-29 12:41 EDT by Gwyn Ciesla
Modified: 2008-02-21 09:02 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-21 09:02:49 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
fix for vdrift scons files (449 bytes, patch)
2007-04-11 18:05 EDT, Rudolf Kastl
no flags Details | Diff
PATCH: fixing vdrift compile with gcc-4.3 (1.48 KB, patch)
2008-02-14 17:10 EST, Hans de Goede
no flags Details | Diff

  None (edit)
Description Gwyn Ciesla 2007-03-29 12:41:22 EDT
Spec URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift.spec
SRPM URL: http://zanoni.jcomserv.net/extras/vdrift/vdrift-20070323-1.src.rpm
Description: VDrift is a cross-platform, open source driving simulation made with drift
racing in mind. It's powered by the excellent Vamos physics engine. It is
released under the GNU General Public License (GPL) v2. It is currently
available for Linux, FreeBSD, Mac OS X and Windows (Cygwin).

This package includes the minimal data set, not packaged serarately as vdrift will not function without it and it is updated with the appliaction.

I will submit a separate package review request for the full data set at a later date.
Comment 1 Hans de Goede 2007-03-29 13:43:03 EDT
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.
Comment 2 Gwyn Ciesla 2007-03-29 13:48:21 EDT
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.
Comment 3 Hans de Goede 2007-03-29 14:15:07 EDT
(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?



Comment 4 Gwyn Ciesla 2007-03-29 14:32:26 EDT
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. :)
Comment 5 Gwyn Ciesla 2007-03-30 09:05:05 EDT
Oh, and I was just looking,and that's the bz2 compressed size.  Full is actually
339MB. :)
Comment 6 Hans de Goede 2007-03-30 09:10:37 EDT
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?
Comment 7 Gwyn Ciesla 2007-03-30 09:22:50 EDT
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?
Comment 8 Hans de Goede 2007-03-30 09:43:27 EDT
(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.
Comment 9 Gwyn Ciesla 2007-03-30 13:34:47 EDT
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?
Comment 10 Hans de Goede 2007-03-30 14:01:22 EDT
(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!

Comment 11 Gwyn Ciesla 2007-04-04 14:03:37 EDT
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@gmail.com, 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.
Comment 12 Rudolf Kastl 2007-04-11 18:05:23 EDT
Created attachment 152325 [details]
fix for vdrift scons files
Comment 13 Rudolf Kastl 2007-04-11 18:23:32 EDT
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}
Comment 14 Gwyn Ciesla 2007-04-12 08:27:31 EDT
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.
Comment 15 Chris Guirl 2007-04-14 04:12:29 EDT
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?
Comment 16 Chris Guirl 2007-04-14 04:46:07 EDT
(In reply to comment #12)
> Created an attachment (id=152325) [edit]
> fix for vdrift scons files
> 

I've checked this into VDrift SVN...
Comment 17 Chris Guirl 2007-04-14 05:17:20 EDT
(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.
Comment 18 Chris Guirl 2007-04-14 05:29:09 EDT
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...
Comment 19 Rudolf Kastl 2007-04-14 08:17:26 EDT
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.
Comment 20 Chris Guirl 2007-04-14 15:44:04 EDT
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? 
Comment 21 Chris Guirl 2007-04-14 16:27:40 EDT
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
Comment 22 Rudolf Kastl 2007-04-14 17:55:18 EDT
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.
Comment 23 Chris Guirl 2007-04-14 18:25:27 EDT
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
Comment 24 Gwyn Ciesla 2007-04-16 11:26:14 EDT
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.
Comment 25 Chris Guirl 2007-04-16 13:53:47 EDT
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
Comment 26 Gwyn Ciesla 2007-04-16 14:17:47 EDT
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. . .?
Comment 27 Chris Guirl 2007-04-16 14:27:42 EDT
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.
Comment 28 Chris Guirl 2007-04-16 15:01:51 EDT
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. :)
Comment 29 Gwyn Ciesla 2007-04-16 15:24:22 EDT
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
Comment 30 Chris Guirl 2007-04-18 14:35:30 EDT
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?
Comment 32 Hans de Goede 2007-04-19 00:50:22 EDT
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

Comment 33 Gwyn Ciesla 2007-04-19 09:55:37 EDT
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?
Comment 34 Rudolf Kastl 2007-04-19 12:21:24 EDT
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.
Comment 35 Gwyn Ciesla 2007-04-19 12:36:32 EDT
Release or Version?  Would not 0.0.0.20070323-3 be better than 20070323-0.0.0.3?
Comment 36 Rudolf Kastl 2007-04-19 20:13:19 EDT
yeah exactly. to prefix means to put it beforehand :)

so i completly agree with you.
Comment 37 Hans de Goede 2007-04-20 07:53:30 EDT
(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.



Comment 38 Remi Collet 2007-04-26 13:42:55 EDT
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.
Comment 39 Gwyn Ciesla 2007-04-26 13:57:31 EDT
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?
Comment 40 Hans de Goede 2007-04-26 14:51:29 EDT
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 .


Comment 41 Gwyn Ciesla 2007-04-27 07:43:25 EDT
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. :)
Comment 42 Hans de Goede 2007-04-27 08:33:50 EDT
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
Comment 43 Gwyn Ciesla 2007-04-27 09:37:20 EDT
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. . .
Comment 44 Gwyn Ciesla 2007-04-27 09:47:39 EDT
Can't find specific contact info:

vdrift forum post:
http://vdrift.net/Forum/viewtopic.php?t=665&start=0&postdays=0&postorder=asc&highlight=
Comment 45 Hans de Goede 2007-04-27 10:43:14 EDT
(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.
Comment 46 Gwyn Ciesla 2007-04-27 10:48:09 EDT
Duh. :)  I knew that.  I'm thinking about doing the same thing with Roundcube if
upstream doesn't re-release soon.
Comment 47 Gwyn Ciesla 2007-04-30 09:43:34 EDT
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. . .
Comment 48 Hans de Goede 2007-05-20 09:21:14 EDT
Any news on the next all clean data release?
Comment 49 Gwyn Ciesla 2007-05-21 14:22:29 EDT
Pinged upstream, it's coming, I'm waiting on an ETA.
Comment 50 Hans de Goede 2007-08-27 16:21:12 EDT
Jon, any news on this?
Comment 51 Gwyn Ciesla 2007-08-29 12:53:48 EDT
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.
Comment 52 Hans de Goede 2007-08-29 13:01:56 EDT
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?
Comment 53 Gwyn Ciesla 2007-08-29 13:09:25 EDT
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.
Comment 54 Gwyn Ciesla 2007-08-29 15:45:57 EDT
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.
-----------
Comment 55 Jason Tibbitts 2008-01-27 15:20:21 EST
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.
Comment 56 Gwyn Ciesla 2008-02-12 12:09:42 EST
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.
Comment 57 Hans de Goede 2008-02-12 15:07:10 EST
(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.
Comment 58 Gwyn Ciesla 2008-02-12 15:23:03 EST
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.
Comment 59 Chris Guirl 2008-02-12 15:48:02 EST
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.
Comment 60 Gwyn Ciesla 2008-02-12 15:59:41 EST
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?
Comment 61 Hans de Goede 2008-02-13 03:18:01 EST
(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?
Comment 62 Gwyn Ciesla 2008-02-13 12:40:16 EST
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. :)
Comment 63 Gwyn Ciesla 2008-02-14 12:03:49 EST
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.
Comment 64 Hans de Goede 2008-02-14 17:10:25 EST
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 ;
Comment 66 Hans de Goede 2008-02-19 07:19:10 EST
(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!
Comment 67 Gwyn Ciesla 2008-02-19 07:35:24 EST
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
Comment 68 Hans de Goede 2008-02-19 07:42:22 EST
(In reply to comment #67)
> Thanks! 
> 
> Want to do the -data too? :)
> 

Already on it, almost finished :)
Comment 69 Kevin Fenzi 2008-02-19 12:21:38 EST
cvs done.
Comment 70 Gwyn Ciesla 2008-02-20 07:38:09 EST
Imported, fixed several BR issues, and now building it fails on ppc64:

http://koji.fedoraproject.org/koji/getfile?taskID=449200&name=build.log
Comment 71 Hans de Goede 2008-02-20 10:20:23 EST
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)
Comment 72 Gwyn Ciesla 2008-02-20 14:46:37 EST
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.
Comment 73 Hans de Goede 2008-02-20 15:17:45 EST
(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
Comment 74 Gwyn Ciesla 2008-02-21 07:32:43 EST
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? :)
Comment 75 Gwyn Ciesla 2008-02-21 09:02:49 EST
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!

Note You need to log in before you can comment on or make changes to this bug.