Bug 459211

Summary: Review Request: oolite - A space sim game, inspired by Elite
Product: [Fedora] Fedora Reporter: Michel Alexandre Salim <michel>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: all_killem, bugzilla, christof, fedora-package-review, fschwarz, gilboad, lemenkov, musuruan, notting, redhat-bugzilla.ja, redhat-micha, richmattes, rjones, rransom.8774, wolfy
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-03-25 12:00:09 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On: 475852    
Bug Blocks: 658489, 459212    
Attachments:
Description Flags
Missing BR. none

Description Michel Alexandre Salim 2008-08-14 21:25:56 EDT
Spec URL: http://salimma.fedorapeople.org/for_review/gnustep/oolite.spec
SRPM URL: http://salimma.fedorapeople.org/for_review/gnustep/oolite-1.65-2.fc10.src.rpm
Description:
Oolite is a space sim game, inspired by Elite, powered by Objective-C
and OpenGL, and designed as a small game that is easy for users to
pick up, modify and expand upon. Almost every aspect of the game can
be changed by using simple, free graphics packages and text editors.
Comment 1 Sébastien Scour 2008-11-16 07:21:54 EST
The link for the RPM is broken. :-(
Comment 2 manuel wolfshant 2008-12-05 19:00:40 EST
The correct link seems to be http://salimma.fedorapeople.org/for_review/gnustep/oolite-1.65-1.fc10.src.rpm

Anyway, the package does not build because it build requires gnustep-base-devel which does not seem to exist in Fedora
Comment 3 manuel wolfshant 2008-12-05 19:05:04 EST
I mean "does not exist yet"...
Comment 4 Michel Alexandre Salim 2009-03-02 21:26:31 EST
Updating the blocker on the active gnustep-base review. Sorry for the delay, I was waiting for gnustep filesystem issues to be hashed out, hoping we could get the native GNUstep filesystem in, but it looks like it's flattened FHS-only for Fedora.
Comment 5 manuel wolfshant 2009-03-03 17:25:32 EST
eh, my interest in gnustep is null, what I care for is to revive my Elite memories since the C64 era :)
Comment 6 Michel Alexandre Salim 2009-03-03 18:16:09 EST
I spent too many hours last summer on Oolite -- but GNUstep itself is a rather neat platform, especially with new work now on Etoile (looking forward to when it works on stock GNUstep).

I'll stop chattering on this bug now, until gnustep-base passes review.
Comment 7 Michel Alexandre Salim 2009-09-15 04:25:07 EDT
Status update: gnustep-base is *almost* done. We might push oolite 1.65 as that is the easier job, and would serve as a nice sanity test for our GNUstep set-up, but what would really be interesting is packaging oolite 1.73.3 (the current test release; a lot of the new add-ons require >= 1.71).

Thing is, the 1.7x series all bundle Mozilla's SpiderMonkey JS engine -- an older copy, circa v1.70. This is rather verboten according to the packaging guidelines (see fedora-devel for the current rsync vs zsync and bundled zlib mess). I'm asking upstream whether oolite could actually use the system-provided JS library, and we'll decide from there.
Comment 8 Michel Alexandre Salim 2009-09-24 14:13:54 EDT
Manuel: gnustep-base has just been approved; could you continue with the review? Thanks!
Comment 9 Peter Lemenkov 2009-09-24 14:26:02 EDT
404 while downloading files. The same is for bz #459212.
Comment 10 Michel Alexandre Salim 2009-09-24 21:30:34 EDT
Ah yes, I reorganized my review queue.

Spec URL: http://salimma.fedorapeople.org/specs/etoile/oolite.spec
SRPM URL:
http://salimma.fedorapeople.org/specs/etoile/oolite-1.65-1.fc10.src.rpm
Comment 11 manuel wolfshant 2009-09-24 23:00:32 EDT
The requested URL /specs/etoile/oolite-1.65-1.fc10.src.rpm was not found on this server.



please be as kind as to provide src.rpm for both oolite and oolite-data. And before doing that, please make sure that your spec does not create unowned directories, because at the first glance over oolite-data.spec, oolite.app/Contents/Resources/ and oolite.app/Contents/ (the directories, not the their content ) seem to be in this situation.
Comment 12 Michel Alexandre Salim 2009-09-25 03:41:58 EDT
Sorry; should have posted a note that the URL above is broken. I've decided to update to oolite 1.73.4 instead (the latest "development" release, but the stable release is really unsupported now).

Spec URL: http://salimma.fedorapeople.org/specs/etoile/oolite.spec
SRPM URL:
http://salimma.fedorapeople.org/specs/etoile/oolite-1.73.4-1.fc12.src.rpm

There is no more separate oolite-data.
Comment 13 Jens Ayton 2009-09-25 06:53:26 EDT
A comment on the SpiderMonkey issue: we intend to be up-to-date in the next full (i.e., feature-stable) release of Oolite, and hopefully for the 1.74-test release. There are API and build issues with upgrading (across all platforms); not huge, but not trivial.

However, Oolite depends on SpiderMonkey being built with the JS_C_STRINGS_ARE_UTF8 feature macro enabled, while Firefox currently depends on it not being enabled. (There’s a runtime check in Oolite for this.) Switching to JS_C_STRINGS_ARE_UTF8 is a to-do for Mozilla 2.0, as attempting to do it for 1.9 broke stuff. (Specifically, bits of Mozilla incorrectly use strings as a binary blob, and fixing this requires API changes.) This is Mozilla bug 315288, https://bugzilla.mozilla.org/show_bug.cgi?id=315288

Until Mozilla 2.0 and whatever the relevant versions of Firefox etc. will be, Oolite won't be able to use system-provided SpiderMonkey. If we reverted to stone-age string handling in Oolite to work around this, we'd be incompatible when the Mozilla issue is fixed.
Comment 14 manuel wolfshant 2009-09-25 07:03:58 EDT
In view of #13, I guess it's high time to officially ask for approval for shipping an internal copy
Comment 15 Michel Alexandre Salim 2009-09-28 16:22:27 EDT
(In reply to comment #13)
> However, Oolite depends on SpiderMonkey being built with the
> JS_C_STRINGS_ARE_UTF8 feature macro enabled, while Firefox currently depends on
> it not being enabled. (There’s a runtime check in Oolite for this.) Switching
> to JS_C_STRINGS_ARE_UTF8 is a to-do for Mozilla 2.0, as attempting to do it for
> 1.9 broke stuff. (Specifically, bits of Mozilla incorrectly use strings as a
> binary blob, and fixing this requires API changes.) This is Mozilla bug 315288,
> https://bugzilla.mozilla.org/show_bug.cgi?id=315288
> 
Jens, thanks for the official comment! I've linked to the Mozilla tracker. I agree with Manuel -- we'd need the packaging committee's approval to get an exemption for this.

What would be really helpful is if we get official word from Oolite (you, basically) clarifying how libjs is used, and thus what the security risk in using a local copy (that is not patched automatically) would be.
Comment 16 Jens Ayton 2009-09-28 17:05:12 EDT
Libjs is used to run local user-installed scripts (as parts of expansion packs) only. There is, by design, no support for loading scripts (or other expansion pack data) over the network.

In debug and testing builds – that is, ones built without the OO_EXCLUDE_DEBUG_SUPPORT macro predefined – one designated script is able to communicate with an external program over the network using a host-defined protocol. (This is used to implement a debug console facility; it is usually only used with localhost, but this can be overridden in a configuration file, which can be part of an expansion pack together with the console script.) In principle, an expansion pack could be made which masquerades as the debug facility and uses JavaScript to send information to an arbitrary server implementing the protocol. Since we only produce “test” versions at the moment, the standard makefile has no option to define OO_EXCLUDE_DEBUG_SUPPORT.

The use of this debug facility also enables scripts to call a semi-arbitrary set of Objective-C methods on classes which are bridged to JavaScript. (Specifically, methods which take zero or one parameter, which must be an object, and return void or an object.) I am not aware of any way in which this could be used to do anything more nefarious than cause the game to access invalid memory and crash, but since it’s an open-ended set of methods I can’t rule anything out.

It should also be noted that in versions of Oolite prior to test release 1.73, a similarly open set of methods could be called by the “legacy scripting system” (which essentially consists of lists of methods to call under various circumstances). This is true of every version of Oolite prior to 1.73 on all platforms. Version 1.73 added method whitelists to address this problem. To my knowledge, nobody has successfully attacked this mechanism, but it was obviously insecure in principle.

It is quite likely that there are still bugs in Oolite’s use of libjs. In the past, such bugs have manifested as immediate crashes as a result of heap corruption or assertion failures. These are broadly the sort of bugs you would expect in C programs that handle diverse and complex user data - inherently a risk, unlikely to be an attractive target.

In short, I judge that the risk in building your own copy of the game is small. However, while this is not what Oolite fans want to hear, it would be a reasonable precaution to hold on including it in a distribution until we catch up with the current version of libjs (hopefully this autumn) or wait until the we’ll-get-there-eventually next “full” release (optimistically, this winter), and then build with OO_EXCLUDE_DEBUG_SUPPORT defined by default.

Due to the questionable design of the legacy scripting mechanism, I can’t recommend the previous “stable” version (1.65) either.
Comment 17 Michel Alexandre Salim 2009-10-02 21:10:11 EDT
Status update: Fedora has a test libjs that is built with JS_C_STRINGS_ARE_UTF8, but when building oolite against it, it crashes right after the splash screen is displayed. The background music never got played.

Is there any patches that Oolite apply to the stock libjs, that is needed for Oolite to run?

$ gdb ./oolite
GNU gdb (GDB) Fedora (6.8.91.20090930-2.fc12)
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/lib64/GNUstep/Applications/oolite.app/oolite...Reading symbols from /usr/lib/debug/usr/lib64/GNUstep/Applications/oolite.app/oolite.debug...done.
done.
(gdb) run
Starting program: /usr/lib64/GNUstep/Applications/oolite.app/oolite 
[Thread debugging using libthread_db enabled]
warning: "/usr/lib/debug/usr/lib64/gconv/libJIS.so.debug": The separate debug info file has no debug info
warning: "/usr/lib/debug/usr/lib64/gconv/libJIS.so.debug": The separate debug info file has no debug info
warning: "/usr/lib/debug/usr/lib64/gconv/libGB.so.debug": The separate debug info file has no debug info
warning: "/usr/lib/debug/usr/lib64/gconv/libKSC.so.debug": The separate debug info file has no debug info
warning: "/usr/lib/debug/usr/lib64/gconv/libGB.so.debug": The separate debug info file has no debug info
warning: "/usr/lib/debug/usr/lib64/gconv/libKSC.so.debug": The separate debug info file has no debug info
[New Thread 0x7fffedaf3710 (LWP 16882)]
[New Thread 0x7fffe3de4710 (LWP 16886)]
[Thread 0x7fffe3de4710 (LWP 16886) exited]
[New Thread 0x7fffe3de4710 (LWP 16887)]
[New Thread 0x7fffe1fc3710 (LWP 16893)]
[New Thread 0x7fffe15c2710 (LWP 16894)]
[New Thread 0x7fffe0bc1710 (LWP 16895)]
[New Thread 0x7fffd3fff710 (LWP 16896)]
[Thread 0x7fffedaf3710 (LWP 16882) exited]
[Thread 0x7fffd3fff710 (LWP 16896) exited]
[Thread 0x7fffe0bc1710 (LWP 16895) exited]
[Thread 0x7fffe15c2710 (LWP 16894) exited]
[Thread 0x7fffe1fc3710 (LWP 16893) exited]
[Thread 0x7fffe3de4710 (LWP 16887) exited]

Program exited with code 01.
(gdb) bt
No stack.
Comment 18 Jens Ayton 2009-10-03 05:05:51 EDT
I don’t believe there are any patches, but there may be incompatibilities and usage errors. Since I haven’t yet managed to build an up-to-date libjs for Mac OS X, I have not been able to test it.

However, that looks like a clean exit, in which case there should be something logged in ~/.Oolite/Logs/Latest.log.
Comment 19 Rich Mattes 2009-10-07 22:51:42 EDT
I downloaded and built the latest Oolite SRPM on F11 x86 (1.73.4-1) for an informal review.  Here are some of my observations:

1. No errors in rpmlint.
2. gcc-objc not specified in BuildRequires.  My machine tried to build it without that package, and obviously failed.
3. Launcher in Applications/Games folder doesn't work for me, I have to launch it from the command line.
4. Program runs fine when launched from command line.

Hope this helps!
Comment 20 Gilboa Davara 2010-06-12 03:03:11 EDT
Created attachment 423449 [details]
Missing BR.
Comment 21 Gilboa Davara 2010-06-12 03:38:53 EDT
- Missing BR (patch above).
- Build just fine on F13/x86_64.
- Game doesn't start from menu.
- Starting the game from command line crashes.
- Adding --debug shows the following callstack:

(gdb) bt
#0  0x0000003f910329a5 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x0000003f91034185 in abort () at abort.c:92
#2  0x0000003f9102b925 in __assert_fail (assertion=0x673aba "surface != ((void *)0)", 
    file=<value optimized out>, line=216, function=<value optimized out>) at assert.c:81
#3  0x000000000056e0b4 in -[MyOpenGLView init] ()
#4  0x0000000000569a14 in -[GameController beginSplashScreen] ()
#5  0x000000000056a89c in -[GameController applicationDidFinishLaunching:] ()
#6  0x000000000056c5b7 in main ()


    if(!gameView){
        gameView = [MyOpenGLView alloc];
-->     [gameView init];
        [gameView setGameController:self];
        [gameView initSplashScreen];
    }

(I tried drilling down into MyOpenGLView, but I couldn't find the init function, and sadly enough, objc is too far from normal C to let me figure out what's broken and why assert was raised)

P.S. Review stalled?
Comment 22 Gilboa Davara 2010-06-12 03:51:57 EDT
OK, found the actual problem, somehow SDL refuses to initialize my display [1]. Anyone else seeing it? Localized OpenGL problem?

- Gilboa
[1] $ debugapp oolite
...
(gdb) r --nosplash
Starting program: /usr/lib64/GNUstep/Applications/oolite.app/oolite --nosplash
...
Program received signal SIGSEGV, Segmentation fault.
0x000000000056e852 in -[MyOpenGLView initialiseGLWithSize:useVideoMode:] ()
Missing separate debuginfos, use: debuginfo-install oolite-1.73.4-1.fc13.x86_64
(gdb) bt
#0  0x000000000056e852 in -[MyOpenGLView initialiseGLWithSize:useVideoMode:] ()
#1  0x000000000056deab in -[MyOpenGLView init] ()
#2  0x0000000000569a14 in -[GameController beginSplashScreen] ()
#3  0x000000000056a89c in -[GameController applicationDidFinishLaunching:] ()
#4  0x000000000056c5b7 in main ()

Starting the application without the --nosplash, sends the assert message listed above.

- Gilboa
Comment 23 Gilboa Davara 2010-06-12 04:04:22 EDT
FYI, switching to --nosplash seems to trigger a crash instead of an assert.
$ debugapp oolite
...
(gdb) r --nosplash
Starting program: /usr/lib64/GNUstep/Applications/oolite.app/oolite --nosplash
...
(gdb) bt
#0  0x000000000056e852 in -[MyOpenGLView initialiseGLWithSize:useVideoMode:] ()
#1  0x000000000056deab in -[MyOpenGLView init] ()
#2  0x0000000000569a14 in -[GameController beginSplashScreen] ()
#3  0x000000000056a89c in -[GameController applicationDidFinishLaunching:] ()
#4  0x000000000056c5b7 in main ()
(gdb) 

Looking at the code (I -really- need to get the debuginfo package built) doesn't give any clue as for the reason of the crash.

- Gilboa
Comment 24 Jens Ayton 2010-06-12 05:24:17 EDT
FYI, we’re expecting to release Oolite 1.74 tomorrow. The Linux built setup has changed significantly, but I’m not familiar with the details. I’ll ask our Linux maintainers to pop in.

Regarding the security statement above: all script access to native Objective-C methods is now whitelisted, except through the debug console mechanism. (The whitelisting in 1.73 was partial.) The updated Linux build system should be able to produce a deployment build with debug console support disabled. We’re still stuck on an old version of libjs, though. :-/
Comment 25 Jens Ayton 2010-06-12 07:10:09 EDT
Follow-up:

“Just to clarify. The current build options, which do not have the "deps-" prefix, operate as always did.

The restructuring of the Linux-deps folders, combined with the deps-* build options, 
is just the first step towards building an oolite development environment (as in Windows) which:
1. will not impact on the O/S's libraries
2. will give the developers the opportunity to build oolite with a steady dev-environment without depending on any unstable library each distro decides to go with.
Currently this approach is used only for autopackaging and requires the Linux-deps/x86*/lib/ libraries to also be installed on the system.

Concluding, and if Micha has nothing more to add on the Makefile, GNUmakefile improvements he applied,
the answer to your last post in that thread (i.e. Jens Ayton 2010-06-12 05:24:17 EDT) is that the Linux built setup is performed the same way since 1.73.4.”
Comment 26 Michael Werle 2010-06-12 11:56:56 EDT
The core build (GNUmakefile) still works as always, however we've added a
'Makefile' to simplify the process, especially as concerns building the
included Spidermonkey dependency.

Assuming all required dependencies are installed on the build machine, "make -f
Makefile release" will build the Spidermonkey dependency and a standard Oolite
release executable into "./oolite.app".  To run this, type
"./oolite.app/oolite".

The Makefile also includes targets to simplify building of snapshot and release
packages, however we don't have an RPM target. However it may assist in
generating version numbers.

Please also note that we have identified serious issues in gnustep-base v1.19.x
(x > 1), and later.  The last known good version of GNUstep for Oolite is
gnustep-base v1.18.

I have a patch for gnustep-base v1.20 (which has been submitted upstream and
will be rolled into the official gnustep-base 1.20.1 release) and indications
are that this is also fine, although this has not been exhaustively tested.
Comment 27 Jason Tibbitts 2010-10-27 13:32:34 EDT
Anything happening here?
Comment 28 Jason Tibbitts 2010-11-03 09:39:36 EDT
I also point out that the most recently posted package (from comment 12) does not build.  Failing scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2573882

Please clear the whiteboard if providing a package which builds.
Comment 29 Michel Alexandre Salim 2010-11-09 20:29:23 EST
Here's the updated SRPM:
http://salimma.fedorapeople.org/specs/etoile/oolite-1.74.2-1.fc14.src.rpm

and Koji scratch build for F-14:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2591972

It still uses the bundled SpiderMonkey for now; we have two possible solutions:
- ask for permission to bundle, or
- package a SpiderMonkey compiled with UTF-8 support, and make sure it's parallel-installable
Comment 30 Michel Alexandre Salim 2010-11-09 20:33:09 EST
Looks like Fedora 14's js actually is compiled with UTF-8 strings now, so I'll update the package soon.
Comment 31 Michel Alexandre Salim 2010-11-09 20:59:23 EST
(In reply to comment #30)
> Looks like Fedora 14's js actually is compiled with UTF-8 strings now, so I'll
> update the package soon.

Spoke too soon; while we do have UTF-8 support, we also compile with JS_THREADSAFE, which triggers this error:

src/Core/Scripting/OOJavaScriptEngine.m:78:2: error: #error Oolite and libjs must be built with JS_THREADSAFE undefined.
Comment 32 Michel Alexandre Salim 2010-11-10 05:29:29 EST
note: spec recently updated to add a virtual Provide: to declare that we ship a bundled copy of js.

see https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Requirement_if_you_bundle

Not redoing the Koji build for now as the result will be identical (apart from the additional Provide: entry) and it takes a while to upload the huge package!
Comment 33 Jason Tibbitts 2010-11-10 09:21:31 EST
Has FESCo given it's assent to this shipping a bundled library here?

Honestly this whole thing is pretty full of failure.  Why would this program possibly require that the library _not_ be thread safe?
Comment 34 Jens Ayton 2010-11-10 19:10:21 EST
The thread-safe mode significantly changes the API, and is slower. No-one’s got around to pessimizing the game yet.
Comment 35 Michel Alexandre Salim 2010-11-10 19:50:33 EST
(In reply to comment #33)
> Has FESCo given it's assent to this shipping a bundled library here?

Not yet. I've just submitted the request:
https://fedorahosted.org/fesco/ticket/491
Comment 36 Jason Tibbitts 2010-11-11 11:58:24 EST
I can see an argument for shipping a non-thread-safe version of the library, if indeed someone can quantify the speed issue, but I still see no argument for bundling.
Comment 37 Jens Ayton 2010-12-19 11:23:04 EST
FYI, I’m currently working on updating Oolite to use trunk SpiderMonkey. The next release (1.75) will target whatever version of SM is stable at the time, and support building with thread-safe mode enabled. Realistically, 1.75 should be ready some time in February.

The JS_C_STRINGS_ARE_UTF8 should no longer be an issue, as it’s now a runtime setting in SpiderMonkey.
Comment 38 Michel Alexandre Salim 2010-12-19 12:11:45 EST
Wonderful news, thanks. This bug will just have to stay open until then.
Comment 39 Michel Alexandre Salim 2011-03-25 12:00:09 EDT
I regrettably don't have time to fix the issues right now, so I'm closing this request. Feel free to take my spec and base a new request on it.

Thanks,

-- 
Michel
Comment 40 Richard W.M. Jones 2014-06-15 17:10:17 EDT
I got some way through compiling 1.80.

http://oirase.annexia.org/reviews/oolite/

Unfortunately it fails with the Fedora javascript library:

./obj.spk/oolite.obj/OOJSEngineTimeManagement.m.o: In function `ContextCallback':
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSEngineTimeManagement.m:247: undefined reference to `JS_SetFunctionCallback'
./obj.spk/oolite.obj/OOJSScript.m.o: In function `-[OOJSScript initWithPath:properties:]':
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSScript.m:227: undefined reference to `JS_DestroyScript'
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSScript.m:227: undefined reference to `JS_DestroyScript'
./obj.spk/oolite.obj/OOJSScript.m.o: In function `ScriptWithCompiledData':
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSScript.m:770: undefined reference to `JS_XDRScript'
./obj.spk/oolite.obj/OOJSScript.m.o: In function `LoadScriptWithName':
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSScript.m:712: undefined reference to `JS_NewScriptObject'
./obj.spk/oolite.obj/OOJSScript.m.o: In function `CompiledScriptData':
/home/rjones/rpmbuild/BUILD/oolite-master/src/Core/Scripting/OOJSScript.m:741: undefined reference to `JS_XDRScript'
collect2: error: ld returned 1 exit status
make[3]: *** [obj.spk/oolite] Error 1
make[2]: *** [internal-objc_program-all_] Error 2
make[1]: *** [oolite.all.objc-program.variables] Error 2
make: *** [internal-all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.VbfWoe (%build)