Bug 21991 - Add Scons-based build system for Chromium's WebKit build into webkit.org
Summary: Add Scons-based build system for Chromium's WebKit build into webkit.org
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-30 16:30 PDT by Eric Seidel (no email)
Modified: 2008-11-13 20:27 PST (History)
2 users (show)

See Also:


Attachments
First attempt at new scons build system (2.95 KB, patch)
2008-10-30 16:32 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Second attempt, now with jsc (6.53 KB, patch)
2008-10-31 13:16 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Third attempt, now disabling DTRACE support if the Prefix header is missing (7.24 KB, patch)
2008-11-03 13:33 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated patch after sgk (Stephen Knight) review (7.25 KB, patch)
2008-11-03 14:01 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated after Tim Hatcher's comments (7.63 KB, patch)
2008-11-04 15:14 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated after Tim Hatcher's comments (7.17 KB, patch)
2008-11-04 15:17 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated to use 4 spaces instead of 2 (7.52 KB, patch)
2008-11-04 15:19 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Updated to latest JSC source locations (7.64 KB, patch)
2008-11-13 14:09 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Modify Platform.h to fix Chromium JSC/WC build on Mac (839 bytes, patch)
2008-11-13 17:46 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Scons file to build JSC for Chromium-Mac (7.54 KB, patch)
2008-11-13 17:46 PST, Eric Seidel (no email)
aroben: review+
Details | Formatted Diff | Diff
Remove broken default CAIRO definition, we can fix WIN_CAIRO to define this in the build system, or give the port an explicit name (956 bytes, patch)
2008-11-13 17:46 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Add first-stab Chromium-mac WebCore build system (24.66 KB, patch)
2008-11-13 17:46 PST, Eric Seidel (no email)
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-10-30 16:30:03 PDT
Add Scons-based build system for Chromium's WebKit build into webkit.org

I'll start with a build system for JSC.  I'm writing this from scratch because our build system needs some re-working anyway.  The first patch I plan to attach is no where near complete.
Comment 1 Eric Seidel (no email) 2008-10-30 16:32:32 PDT
Created attachment 24786 [details]
First attempt at new scons build system

 JavaScriptCore/SConscript |   93 +++++++++++++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct |    1 +
 2 files changed, 94 insertions(+), 0 deletions(-)
Comment 2 Eric Seidel (no email) 2008-10-30 16:36:20 PDT
Comment on attachment 24786 [details]
First attempt at new scons build system

There are many things wrong with this first patch.

1.  it makes no attempt to be x-platform (which is the whole point of using scons).  I need to find a way to add the various different platform files for the different platforms.  Scons knows how to do this, I just haven't learned enough about scons yet. :)

2.  The resulting file is libJavaScriptCore.dylib.  Scons doesn't know much about frameworks.  There are some 3rd party folks who have done some bundle-related hacks (http://www.scons.org/wiki/MacOSX), but if we want to build frameworks with scons we'll have to write a new scons builder I think.

3.  I build in JavaScriptCore/ directly instead of using WebKitBuild or whatever directory the user has configured.  Our configuration system for Chromium will be different (Chromium ideally will re-use this SConscript file but won't use the SConstruct file.)  Likewise the DerivedSources should be in whatever build directory instead of right in the source directory.

Otherwise SCons wasn't nearly as scary as it could have been. :)
Comment 3 Mark Rowe (bdash) 2008-10-30 20:22:27 PDT
Comment on attachment 24786 [details]
First attempt at new scons build system

> +# Generate Dtrace information for the Leopard build
> +tracing_data="kjs/Tracing.d";
> +tracing_header="DerivedSources/JavaScriptCore/TracingDtrace.h";
> +os.system("dtrace -h -o " + tracing_header + " -s " + tracing_data) == 0 or sys.exit(1)

The dtrace support should not be compiled in on Leopard as it does not behave correctly.
Comment 4 Eric Seidel (no email) 2008-10-31 13:16:45 PDT
Created attachment 24809 [details]
Second attempt, now with jsc

 JavaScriptCore/JavaScriptCore.scons |  204 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    4 +
 JavaScriptCore/jsc.scons            |    7 ++
 3 files changed, 215 insertions(+), 0 deletions(-)
Comment 5 Eric Seidel (no email) 2008-11-03 13:33:05 PST
Created attachment 24863 [details]
Third attempt, now disabling DTRACE support if the Prefix header is missing

 JavaScriptCore/JavaScriptCore.scons |  209 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    4 +
 JavaScriptCore/jsc.scons            |    7 +
 JavaScriptCore/wtf/Platform.h       |    2 +-
 4 files changed, 221 insertions(+), 1 deletions(-)
Comment 6 Eric Seidel (no email) 2008-11-03 13:34:04 PST
I am going to ask Steven Knight (author of scons and member of Chrome team) to look at this before any landing, but I've marked it for review all the same.
Comment 7 Eric Seidel (no email) 2008-11-03 14:01:03 PST
Created attachment 24865 [details]
Updated patch after sgk (Stephen Knight) review

 JavaScriptCore/JavaScriptCore.scons |  210 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    4 +
 JavaScriptCore/jsc.scons            |    7 +
 JavaScriptCore/wtf/Platform.h       |    2 +-
 4 files changed, 222 insertions(+), 1 deletions(-)
Comment 8 Timothy Hatcher 2008-11-04 05:42:11 PST
+  'kjs/debugger.cpp',
+  'kjs/DebuggerCallFrame.cpp',

This is out-of-date with TOT, these files and others have been renamed or moved recently.

Also why do we need this? I thought Chromium didn't use JavaScriptCore?

Why does JavaScriptCore and jsc shell need to be separate? Shouldn't we just make both all the time? Or is that what SConstruct does? Is SConstruct the required name? Because as-is, it is not self-evident.

Is it possible to build jsc.scons without JavaScriptCore.scons? I don't see an indication of dependancy. (I also don't know a bit about Scons.)
Comment 9 Eric Seidel (no email) 2008-11-04 12:33:34 PST
(In reply to comment #8)
> +  'kjs/debugger.cpp',
> +  'kjs/DebuggerCallFrame.cpp',
> 
> This is out-of-date with TOT, these files and others have been renamed or moved
> recently.

Yup, it's hard to keep up!  I'll sync and update the patch shortly.

> Also why do we need this? I thought Chromium didn't use JavaScriptCore?

Well, so this is not the complete solution by far.  Chromium does have a build using JavaScriptCore.  I tried to design this build file so that other platforms/configurations could use it if they so chose, but I assume that only Chromium will use it in the short term.

> Why does JavaScriptCore and jsc shell need to be separate? Shouldn't we just
> make both all the time? Or is that what SConstruct does? Is SConstruct the
> required name? Because as-is, it is not self-evident.

They don't need to be in separate SConscript files.  SConstruct is not the *required* name, but it is the default "scons" will look for in the directory.  SConscript is the conventional name for the supporting script files, however Steven says authors have started to favor "TargetName.scons" instead of the generic "SConscript" name.

SCons files are just python, so they can be named whatever and be located wherever.

> Is it possible to build jsc.scons without JavaScriptCore.scons? I don't see an
> indication of dependancy. (I also don't know a bit about Scons.)

Not currently, no.  Scons files just define the dependency graph, and then the commands like "Program()" and "SharedLibrary()" execute on those dependencies.  I don't even know how you specify dependencies like the jsc "target" depending on JavaScriptCore yet.

Also, note these files do not really build the chromium build yet.  They build the "mac" build of JavaScriptCore.  I'll be fixing them up to work with the Chromium build shortly when we bring our chromium buildbot online (hopefully tomorrow).
Comment 10 Eric Seidel (no email) 2008-11-04 15:14:57 PST
Created attachment 24897 [details]
Updated after Tim Hatcher's comments

 JavaScriptCore/JavaScriptCore.scons |  229 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    1 +
 JavaScriptCore/wtf/Platform.h       |    2 +-
 3 files changed, 231 insertions(+), 1 deletions(-)
Comment 11 Eric Seidel (no email) 2008-11-04 15:17:00 PST
Created attachment 24898 [details]
Updated after Tim Hatcher's comments

 JavaScriptCore/JavaScriptCore.scons |  229 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    1 +
 2 files changed, 230 insertions(+), 0 deletions(-)
Comment 12 Eric Seidel (no email) 2008-11-04 15:19:59 PST
Created attachment 24899 [details]
Updated to use 4 spaces instead of 2

 JavaScriptCore/JavaScriptCore.scons |  229 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    1 +
 2 files changed, 230 insertions(+), 0 deletions(-)
Comment 13 Eric Seidel (no email) 2008-11-13 14:09:41 PST
Created attachment 25135 [details]
Updated to latest JSC source locations

 JavaScriptCore/JavaScriptCore.scons |  231 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    1 +
 2 files changed, 232 insertions(+), 0 deletions(-)
Comment 14 Eric Seidel (no email) 2008-11-13 14:56:00 PST
Comment on attachment 25135 [details]
Updated to latest JSC source locations

I'll upload the WebCore one too in a bit and they can both be reviewed at once.
Comment 15 Eric Seidel (no email) 2008-11-13 17:46:10 PST
Created attachment 25150 [details]
Modify Platform.h to fix Chromium JSC/WC build on Mac

 JavaScriptCore/wtf/Platform.h |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
Comment 16 Eric Seidel (no email) 2008-11-13 17:46:11 PST
Created attachment 25151 [details]
Scons file to build JSC for Chromium-Mac

 JavaScriptCore/JavaScriptCore.scons |  230 +++++++++++++++++++++++++++++++++++
 JavaScriptCore/SConstruct           |    1 +
 2 files changed, 231 insertions(+), 0 deletions(-)
Comment 17 Eric Seidel (no email) 2008-11-13 17:46:13 PST
Created attachment 25152 [details]
Remove broken default CAIRO definition, we can fix WIN_CAIRO to define this in the build system, or give the port an explicit name

 JavaScriptCore/wtf/Platform.h |    6 ------
 WebCore/config.h              |    1 -
 2 files changed, 0 insertions(+), 7 deletions(-)
Comment 18 Eric Seidel (no email) 2008-11-13 17:46:14 PST
Created attachment 25153 [details]
Add first-stab Chromium-mac WebCore build system

 WebCore/SConstruct    |    3 +
 WebCore/WebCore.scons |  787 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 790 insertions(+), 0 deletions(-)
Comment 19 Eric Seidel (no email) 2008-11-13 18:05:31 PST
I will gladly add ChangeLogs to each patch when landing.  The WIN_CAIRO thing will need further work to fix WIN_CAIRO.  I could alternatively just add !PLATFORM(CHROMIUM) there, but it seems like just plastering over brokeness.  Then again, I could do that and just file a bug against the WIN_CAIRO folks to fix it.  I'm comfortable with either solution.
Comment 20 Sam Weinig 2008-11-13 19:30:16 PST
Comment on attachment 25150 [details]
Modify Platform.h to fix Chromium JSC/WC build on Mac

This has already landed.
Comment 21 Eric Seidel (no email) 2008-11-13 19:30:30 PST
Comment on attachment 25150 [details]
Modify Platform.h to fix Chromium JSC/WC build on Mac

Turns out this one was already in ToT.  My checkout was a day behind.
Comment 22 Eric Seidel (no email) 2008-11-13 19:31:50 PST
Comment on attachment 25152 [details]
Remove broken default CAIRO definition, we can fix WIN_CAIRO to define this in the build system, or give the port an explicit name

I'll just add Chromium to the Cairo black list.  It's ugly that we use a blacklist to specify the cairo build.  But at some level, it's not my problem... and trying to fix it will just get my in trouble. ;)  I'll leave it to the Cairo guys to clean up.
Comment 23 Adam Roben (:aroben) 2008-11-13 19:58:28 PST
Comment on attachment 25151 [details]
Scons file to build JSC for Chromium-Mac

r=me
Comment 24 Adam Roben (:aroben) 2008-11-13 19:59:22 PST
Comment on attachment 25153 [details]
Add first-stab Chromium-mac WebCore build system

> +sources['css'] = [
> +'css/CSSBorderImageValue.cpp',
> +'css/CSSCanvasValue.cpp',
> +'css/CSSCharsetRule.cpp',
> +'css/CSSComputedStyleDeclaration.cpp',
> +'css/CSSCursorImageValue.cpp',
> +'css/CSSFontFace.cpp',

Seems like these lines (and many others) should be indented.

r=me
Comment 25 Mark Rowe (bdash) 2008-11-13 20:05:53 PST
Comment on attachment 25153 [details]
Add first-stab Chromium-mac WebCore build system

> diff --git a/WebCore/WebCore.scons b/WebCore/WebCore.scons
> new file mode 100644
> index 0000000..f81d5d1
> --- /dev/null
> +++ b/WebCore/WebCore.scons
> @@ -0,0 +1,787 @@
> +import os
> +import sys
> +from subprocess import Popen, PIPE

These imports seem to be unneeded.

The source file lists should be kept sorted to make keeping them up to date a little easier going forward.
Comment 26 Eric Seidel (no email) 2008-11-13 20:10:49 PST
(In reply to comment #25)
> (From update of attachment 25153 [details] [review])
> > diff --git a/WebCore/WebCore.scons b/WebCore/WebCore.scons
> > new file mode 100644
> > index 0000000..f81d5d1
> > --- /dev/null
> > +++ b/WebCore/WebCore.scons
> > @@ -0,0 +1,787 @@
> > +import os
> > +import sys
> > +from subprocess import Popen, PIPE
> 
> These imports seem to be unneeded.

Ah, good catch.  A copy/paste remnant from the JavaScriptCore file.

> The source file lists should be kept sorted to make keeping them up to date a
> little easier going forward.

Hum... I tried to keep them all sorted, but it's possible I missed some.  I'll look again.

I intentionally tried to factor out the platform-specific files.  Although these files don't yet build on windows, they will soon.  I'll be working on getting them up and going on Windows and Linux as well (or others may in parallel).
Comment 27 Eric Seidel (no email) 2008-11-13 20:11:26 PST
(In reply to comment #24)
> Seems like these lines (and many others) should be indented.

Agreed, I got lazy on my 4th rebuild of that file. ;)  I fixed them all now.
Comment 28 Mark Rowe (bdash) 2008-11-13 20:12:04 PST
Comment on attachment 25151 [details]
Scons file to build JSC for Chromium-Mac

> +# Handle os-version specific build settings
> +if env['PLATFORM'] == 'darwin':
> +    version_pieces = Popen(["sw_vers", "-productVersion"], stdout = PIPE).communicate()[0].split('.')
> +    if int(version_pieces[0]) == 10:
> +        # Dtrace doesn't exist in Tiger, and was broken in Leopard
> +        if int(version_pieces[1]) > 5:
> +            env.Command(DerivedSources('TracingDtrace.h'), 'runtime/Tracing.d', '/usr/sbin/dtrace -h -o $TARGET -s $SOURCE')

The version check may be clearer if written like
if map(int, version_pieces)[:2] > (10, 5):
Comment 29 Eric Seidel (no email) 2008-11-13 20:16:39 PST
(In reply to comment #28)
> (From update of attachment 25151 [details] [review])
> The version check may be clearer if written like
> if map(int, version_pieces)[:2] > (10, 5):

Very cool.  Fixed.

Thank you both very much for your helpful comments and review.
Comment 30 Eric Seidel (no email) 2008-11-13 20:27:12 PST
Landed as:
http://trac.webkit.org/changeset/38383
http://trac.webkit.org/changeset/38384