587369 - [OS/2] let YARR build on OS/2

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Walter Meinl]

User-Agent:       Mozilla/5.0 (OS/2; Warp 4.5; rv:2.0b4pre) Gecko/20100814 Minefield/4.0b4pre
Build Identifier: 

YARR was ported from webkit to tracemonkey in bug564953. Since OS/2 is not a webkit supported platform - building mozjs.lib breaks during linking due to unresolved symbols. We need an OS/2 specific ExecutableAllocator file as well as some changes to the Platform header.

Reproducible: Always

Comment 1

[Walter Meinl]

Attachment: v1 first try

the patch adds ExecutableAllocatorOS2.cpp and change Platform.h to recognize OS/2. I've mostly tried to convert the lines we ported to dlmalloc in libffi. At least the browser builds and runs...

Comment 2

[Dave Yeo]

(In reply to comment #0)

> YARR was ported from webkit to tracemonkey in bug564953. Since OS/2 is not a
> webkit supported platform - building mozjs.lib breaks during linking due to
> unresolved symbols. We need an OS/2 specific ExecutableAllocator file as well
> as some changes to the Platform header.

Wonder if it would be worth looking at the OS/2 port of QT4 at netlabs and their webkit. I have a browser here, Arora 0.10.2, which says it's WebKit version: 532.4.
http://www.ecomstation.it/ecsoft2/prog.php?progid=1608&name=Arora&sys=os2+ecomstation

Comment 3

[Walter Meinl]

(In reply to comment #2)
> 
> Wonder if it would be worth looking at the OS/2 port of QT4 at netlabs and
> their webkit. I have a browser here, Arora 0.10.2, which says it's WebKit
> version: 532.4.
Looking at the QTwebkit port, the patch appears to be quite reasonable.
One may add to the function ExecutablePool::Allocation ExecutablePool::systemAlloc(size_t n)

        if (allocation != NO_ERROR) 
 	CRASH();
as Dmik did in the respective qtwebkit file. BTW, the implementation of the os2 changes happens in the main allocator file, and just a couple of days ago the platform allocator files were retracted from webkit trunk and all the platform specific allocation functions were munged into the main allocator file.

Comment 4

[Dave Yeo]

Wonder if this should also be reviewed by one of the JavaScript people?

Comment 5

[Walter Meinl]

Comment on attachment 466038 [details] [diff] [review]
v1 first try

Andreas, the OS/2 bits are in principle ok. Anything else you want to mention on this patch, as it touches a few js files as well? I guess we don't need an upstreamable version of this patch, since it's only a small part of webkit that was ported to the needs of mozilla (there's no OS/2 port of webkit available).

Comment 6

[Walter Meinl]

Attachment: v1 qrefreshed for bitrot

Comment 7

[Andreas Gal :gal]

Comment on attachment 477248 [details] [diff] [review]
v1 qrefreshed for bitrot

Thanks!

Comment 8

[Walter Meinl]

Comment on attachment 477248 [details] [diff] [review]
v1 qrefreshed for bitrot

pretty low risk for tier_1 platforms all ifdef sections

Comment 9

[Walter Meinl]

(In reply to comment #7)
> Comment on attachment 477248 [details] [diff] [review]
> v1 qrefreshed for bitrot
> 
> Thanks!

gal, just for clarification as I was told so in another bug related to tracemonkey. Do tracemonkey patches have to be approved for mozilla-2.0? Asking because this waits for approval now for several weeks. Before, it was sitting with the checkin-needed flag ("tracemonkey" in the whiteboard) as well for several weeks (maybe noone was really sure, if the patch needs approval).
Thanks

Comment 10

[Brendan Eich [:brendan]]

Walter: no one was cc'ed who might actually check the patch in.

Porting fixes can go in any time if they affect only the non-tier-1 platform.

/be

Comment 11

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/tracemonkey/rev/8003e273cd7e

Comment 12

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/8003e273cd7e