Bug 19975 - [OpenBSD] Patches to enable build of WebKit
Summary: [OpenBSD] Patches to enable build of WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-10 13:37 PDT by Landry Breuil
Modified: 2008-07-18 08:33 PDT (History)
2 users (show)

See Also:


Attachments
patches for OpenBSD (2.32 KB, patch)
2008-07-10 13:38 PDT, Landry Breuil
no flags Details | Formatted Diff | Diff
new patch to correctly include pthread.h (669 bytes, patch)
2008-07-11 02:54 PDT, Landry Breuil
ddkilzer: review-
Details | Formatted Diff | Diff
patch against svn (4.55 KB, patch)
2008-07-17 04:59 PDT, Landry Breuil
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Landry Breuil 2008-07-10 13:37:11 PDT
The following patches enable support for OpenBSD, adapted from the first diff shown here : https://bugs.webkit.org/show_bug.cgi?id=19775
- One patch against JavaScriptCore/wtf/Platform.h to add PLATFORM support, both for OpenBSD and SPARC64.
- One against JavaScriptCore/kjs/config.h to define USE_PTHREAD_NP.
- One against JavaScriptCore/wtf/Threading.h to avoir using functions from libestdc++ 4.2, because otherwise makefiles have to be patched to link agaisnt -lestdc++ instead of -lstdc++ (which is libstdc++ 3.3 on OpenBSD)
- One against JavaScriptCore/kjs/collector.cpp to be sure we include pthread.h besides pthread_np.h, and also an #elif fragment to get the stack base.

There are other patches i'm not submitting for review because they #define isfinite to finite and implement signbit(), because these functions are in the process of beign added to OpenBSD's libm and will be soon here (maybe the incoming 4.4)
With this patches webkit/gtklauncher/midori works fine on i386/amd64/macppc.

Patches from bug #19775 are also needed to try to support sparc64, although i haven't been able to make webkit run without SIGBUS.

Last question, our default gcc/g++ is 3.3.5, are there plans to make webkit build with those compilers or gcc 4.x is _required_ ?
Comment 1 Landry Breuil 2008-07-10 13:38:54 PDT
Created attachment 22203 [details]
patches for OpenBSD

This patches are in the progress of beign tested _again_, but the previous diff linked on #19775 was tested several times.
Comment 2 David Kilzer (:ddkilzer) 2008-07-10 15:06:33 PDT
Comment on attachment 22203 [details]
patches for OpenBSD

Great!  r=me

I forgot to mention in the original review (Bug 19775 Comment #14) that you also need a ChangeLog entry.

http://webkit.org/coding/contributing.html

I'll write one up unless you'd like to do it.
Comment 3 Landry Breuil 2008-07-11 02:54:40 PDT
Created attachment 22246 [details]
new patch to correctly include pthread.h

We don't have thread.h like solaris, so patch it this way. This was my mistake, rebuilding to make sure it's ok now.

For the changelog entry, go ahead :)
Comment 4 David Kilzer (:ddkilzer) 2008-07-11 03:35:00 PDT
Comment on attachment 22203 [details]
patches for OpenBSD

Oops.  Didn't realize the second patch was additive.
Comment 5 David Kilzer (:ddkilzer) 2008-07-11 03:40:10 PDT
Comment on attachment 22246 [details]
new patch to correctly include pthread.h

>--- JavaScriptCore/kjs/collector.cpp.orig	Mon Jun 16 01:40:06 2008
>+++ JavaScriptCore/kjs/collector.cpp	Fri Jul 11 11:46:16 2008
>@@ -61,6 +61,10 @@
> #include <thread.h>
> #endif
> 
>+#if PLATFORM(OPENBSD)
>+#include <pthread.h>
>+#endif
>+
> #if HAVE(PTHREAD_NP_H)
> #include <pthread_np.h>
> #else

Looks good.

>@@ -331,6 +335,11 @@ static inline void* currentThreadStackBase()
>     stack_t s;
>     thr_stksegment(&s);
>     return s.ss_sp;
>+#elif PLATFORM(OPENBSD)
>+    pthread_t thread = pthread_self();
>+    stack_t stack;
>+    pthread_stackseg_np(thread, &stack);
>+    return stack.ss_sp;
> #elif PLATFORM(UNIX)
>     static void* stackBase = 0;
>     static size_t stackSize = 0;

This is already in the first patch.  There's no need to include it here.

The ChangeLog for the first patch is "free".  Please create one using the prepare-ChangeLog script.  Thanks!

r- for above issues.
Comment 6 Landry Breuil 2008-07-17 04:59:29 PDT
Created attachment 22335 [details]
patch against svn

And here's a cleaner, simpler patchset against svn, works fine with midori & webkit-r35203.
ChangeLogs entries are included.
- i finally added a temporary signbit/isfinite implemetation in MathExtras.h, similar to what is done for Solaris. Once we'll have them in libm it shouldn't conflict.
- finally, Thread.h patching is not necessary, we have to link with libstdc++-4.2 anyway (that'll be our only local patch against GNUMakefile.am, because on openbsd libstdc++ in /usr/lib is 3.3 version, libstdc++ 4.2 is installed in /usr/local/lib/libestdc++)
- a non-intrusive patch for WebKitTools/DumpRenderTree/DumpRenderTree.h has been added to conditionally declare the method. I think there's a better way to handle it, but it compiles and runs fine this way. Having a HAVE_WCHAR detection could help maybe.
- while here i kept the PLATFORM(SPARC64) definition, needed for the upcoming fix for alignment problem on sparc64.

I hope this patchset is correct, and am just waiting for a new nightly build including it to import my port of webkit & midori in OpenBSD's ports-tree. The fix for sparc64 will be worked on afterwards.
Comment 7 David Kilzer (:ddkilzer) 2008-07-17 05:09:01 PDT
Comment on attachment 22203 [details]
patches for OpenBSD

Clearing review flag for obsolete patch.
Comment 8 David Kilzer (:ddkilzer) 2008-07-17 05:20:45 PDT
Comment on attachment 22335 [details]
patch against svn

r=me!

Please include reference to the bug in future changelog entries.  Thanks!
Comment 9 David Kilzer (:ddkilzer) 2008-07-18 08:33:26 PDT
Committed r35234

Note that I tweaked the whitespace in MathExtras.h to match the formatting of the PLATFORM(SOLARIS) section above it.

Please file new bugs for additional or follow-up issues, and feel free to copy me on them.  Thanks!