Initial Gears SConscript file. Doesn't build yet.

Initial Gears SConscript file.  A bit messy.  Only builds chrome+dbg and
IE+dbg configurations.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=2269

[Matt Perry, 2008-09-09 19:03:56]

I've finally got something building.  It builds a debug version gears.dll for
Chrome only.

I peppered lots of TODO comments in there with questions about how to do
something better than the way I'm doing it.  If you know of a way, please
enlighten me :).

[sgk, 2008-09-10 21:44:03]

This is really good stuff; the complexity is restricted to a few key sections,
as it should be, and the SConscript files themselves are nice and clean.

I punted for now on the suffix stuff at the botton of the gears/SConscript file,
in order to get the other comments to you sooner.  There are a couple of
different ways we could make that more readable (separate builders, or
pseudo-builders), but it probably makes more sense to discuss FTF before
deciding on an approach.

http://codereview.chromium.org/232/diff/403/622
File gears/SConscript (right):

http://codereview.chromium.org/232/diff/403/622#newcode37
Line 37: #     on a SConscript.browser?
variant_dir sounds about right.

http://codereview.chromium.org/232/diff/403/622#newcode44
Line 44: )
There are some *Variable() functions that can be used to pull things off the
command line with automatic error checking and help text and the like.  What you
have is fine, but if you want to start doing things in the future like
restricting MODE to a set of known, legal input values, the code already exists
for that and you don't need to invent it.

http://codereview.chromium.org/232/diff/403/622#newcode78
Line 78: # it here.
You can get the string expansion directly using (e.g.):

env.subst('$THIRD_PARTY_DIR')

http://codereview.chromium.org/232/diff/403/622#newcode190
Line 190: #env.Append(SCANNERS = m4_scanner)
Good start; nothing obviously wrong off the top of my head.  Swing by if you
want some additional eyes debugging.

http://codereview.chromium.org/232/diff/403/622#newcode199
Line 199: env['STRIPPED_GEARS_DIR'] = Dir(env.subst('$GEARS_DIR'))
Hmm, did you try:

env.Dir('$GEARS_DIR')

The '#' interpretation gets wonky, but I'd think that should have worked for
you.

http://codereview.chromium.org/232/diff/403/622#newcode212
Line 212: 'cd googleclient/gears/opensource/gears && python tools/bin2cpp.py'
I'd do this by setting

BIN2CPP = File('googleclient/gears/opensource/gears/tools/bin2cpp.py')

and then making the command handle the cd and stuff

BIN2CPPCOM = 'cd ${BIN2CPP.dir}/.. && python tools/${BIN2CPP.file} ...'

Hmm, now that I actually typed it, that doesn't seem as clean, especially since
it really is a pain that you have to strip off this specific directory that's
part-way down the path.  Bleh.  I'd be inclined to see if you can just use
something like ${SOURCE.abspath}, or else modify tools/bin2cpp.py so it's a
little less picky about the directory from, and path names with which, it's
invoked...

http://codereview.chromium.org/232/diff/403/622#newcode247
Line 247: '/BASE:0x65000000',
Should be in the Windows-specific section, I think.

http://codereview.chromium.org/232/diff/403/622#newcode331
Line 331: ['/l', '0x409'],
Windows-specific

http://codereview.chromium.org/232/diff/403/627
File gears/SConscript.libgd (right):

http://codereview.chromium.org/232/diff/403/627#newcode69
Line 69: )
Windows-specific /wd flags.

http://codereview.chromium.org/232/diff/403/629
File gears/SConscript.portaudio (right):

http://codereview.chromium.org/232/diff/403/629#newcode53
Line 53: '/wd4101',
Windows-specific /wd flags.

http://codereview.chromium.org/232/diff/403/628
File gears/SConscript.sqlite (right):

http://codereview.chromium.org/232/diff/403/628#newcode77
Line 77: CCFLAGS = [
Windows-specific CCFLAGS.

[Matt Perry, 2008-09-10 23:35:06]

http://codereview.chromium.org/232/diff/403/622
File gears/SConscript (right):

http://codereview.chromium.org/232/diff/403/622#newcode44
Line 44: )
On 2008/09/10 21:44:04, sgk wrote:
> There are some *Variable() functions that can be used to pull things off the
> command line with automatic error checking and help text and the like.  What
you
> have is fine, but if you want to start doing things in the future like
> restricting MODE to a set of known, legal input values, the code already
exists
> for that and you don't need to invent it.

Thanks for the tip.  I think I'll add that in a future revision so you don't
have to keep reviewing changes.

http://codereview.chromium.org/232/diff/403/622#newcode78
Line 78: # it here.
On 2008/09/10 21:44:04, sgk wrote:
> You can get the string expansion directly using (e.g.):
> 
> env.subst('$THIRD_PARTY_DIR')

Sadly, that expands to #/Hammer/gears/googleclient/third_party.  There's a
number of places where I want to use these variables, but without the
"#/Hammer/gears" prefix.  So far, I've had no luck with experiments:
- env.Dir('$THIRD_PARTY_DIR') is close, but I want an abspath
- env.Dir('$THIRD_PARTY_DIR').abspath gives the abspath to the Hammer/gears/...
dir :(
- env.Dir('$THIRD_PARTY_DIR').srcnode() gives the build dir
- there is no Dir().srcpath or .srcdir or any such thing

This is one of the more common issues I've had with writing scons scripts
(mostly when writing my own builders).  I can't help but feel there must be an
easier way to get the src path from the build output path.

http://codereview.chromium.org/232/diff/403/622#newcode190
Line 190: #env.Append(SCANNERS = m4_scanner)
On 2008/09/10 21:44:04, sgk wrote:
> Good start; nothing obviously wrong off the top of my head.  Swing by if you
> want some additional eyes debugging.

There are a few problems with writing a scanner for this.
1. 'path' passed into m4_scan is empty.  I think I can use path_scanner =
FindPathDirs('M4PATH') passed to Scanner, but I haven't had much luck.
2. More importantly, these .m4 files depend on generated files that may not
exist yet.  For example, permissions_dialog.html_m4 includes
"genfiles/permissions_dialog.js" - but the scanner doesn't know where that's
relative to because the file doesn't exist until it's built for being a
dependency of the .html_m4 file.

http://codereview.chromium.org/232/diff/403/622#newcode199
Line 199: env['STRIPPED_GEARS_DIR'] = Dir(env.subst('$GEARS_DIR'))
On 2008/09/10 21:44:04, sgk wrote:
> Hmm, did you try:
> 
> env.Dir('$GEARS_DIR')
> 
> The '#' interpretation gets wonky, but I'd think that should have worked for
> you.

Thanks, that seems to work.

http://codereview.chromium.org/232/diff/403/622#newcode212
Line 212: 'cd googleclient/gears/opensource/gears && python tools/bin2cpp.py'
On 2008/09/10 21:44:04, sgk wrote:
> I'd do this by setting
> 
> BIN2CPP = File('googleclient/gears/opensource/gears/tools/bin2cpp.py')
> 
> and then making the command handle the cd and stuff
> 
> BIN2CPPCOM = 'cd ${BIN2CPP.dir}/.. && python tools/${BIN2CPP.file} ...'
> 
> Hmm, now that I actually typed it, that doesn't seem as clean, especially
since
> it really is a pain that you have to strip off this specific directory that's
> part-way down the path.  Bleh.  I'd be inclined to see if you can just use
> something like ${SOURCE.abspath}, or else modify tools/bin2cpp.py so it's a
> little less picky about the directory from, and path names with which, it's
> invoked...

Long term I plan on modifying the tool, but I'm approaching this as a third
party right now to see if I can get it building without changing gears at all. 
The problem with feeding it an abspath is that it generates C++ symbols based on
the path you feed it, so it is very particular about being a path relative to
the 'opensource/gears' dir.

http://codereview.chromium.org/232/diff/403/622#newcode247
Line 247: '/BASE:0x65000000',
On 2008/09/10 21:44:04, sgk wrote:
> Should be in the Windows-specific section, I think.

Done.

http://codereview.chromium.org/232/diff/403/622#newcode331
Line 331: ['/l', '0x409'],
On 2008/09/10 21:44:04, sgk wrote:
> Windows-specific
> 
> 

Done.

[Matt Perry, 2008-09-12 23:10:27]

OK, I've made some more changes.  In addition to fixing the issues you raised,
I:
- added some IE inputs sections
- added IDL compiler rules for IE files
- generalized the input_files lists into *src[BROWSER_NAME] dictionaries.  files
to build, for NPAPI for example, are srcs['all'] + srcs['NPAPI'].

Looking forward to getting this checked in.

[sgk, 2008-09-16 18:17:41]

lgtm now