Adding visual studio solution files....

Adding visual studio solution files.
Sandbox is currently having a problem with this so it is disabled.
Fixed a few stray items.


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

[bradn, 2008-11-03 18:59:55]

Ok this generates the source solution for everything except sandbox which is
still having a problem down in the tests.

[sgk, 2008-11-03 19:16:14]

http://codereview.chromium.org/9259/diff/1/4
File build/SConscript.main (right):

http://codereview.chromium.org/9259/diff/1/4#newcode652
Line 652: # This must be last!
A little more explanation in the comment about *why* it must be last (so someone
in the future who thinks their new code is the one that really should go last
has a chance of deciding correctly).

http://codereview.chromium.org/9259/diff/1/4#newcode659
Line 659: solution_env.Alias('solution', solution)
Don't need this Alias since we're going to use the env.Bit('solution') to
control generation of the solution files.

http://codereview.chromium.org/9259/diff/1/4#newcode660
Line 660: Default(None)
Comment about why we need Default(None) here.  Otherwise it's pretty obscure to
people reading the code.

http://codereview.chromium.org/9259/diff/1/4#newcode661
Line 661: Default(Alias('solution'))
Ditto here, should be able to pass in the proper Node.

http://codereview.chromium.org/9259/diff/1/3
File build/SConscript.v8 (left):

http://codereview.chromium.org/9259/diff/1/3#oldcode79
Line 79: env.ChromeStaticLibrary('$V8_DIR/v8_snapshot',
???  Why removing this?  In my last rounds of work, it was necessary to make
sure the v8_snapshot.lib file was generated in the output directory (Hammer/v8),
not in the build/ source directory.

http://codereview.chromium.org/9259/diff/1/8
File chrome/SConscript (right):

http://codereview.chromium.org/9259/diff/1/8#newcode44
Line 44: env_res.Requires(chrome_dll_res,
'installer/util/installer_util_strings.rc')
Is Requires() correct here?  If installer_util_string.rc is used in the
generation of chrome_dll.res, then usually it would need a Depends() so that the
target is re-generated if the dependency changes.  (Needing
installer_util_strings.rc to be present but not caring about whether its
contents change would be a little unusually, but might be correct...)

http://codereview.chromium.org/9259/diff/1/8#newcode246
Line 246: '$VERSION_BAT $SOURCE $CHROME_SRC_DIR/chrome/ $PWD $TARGET',
Ouch--the trailing / is necessary here?  If so, a comment would be helpful.

http://codereview.chromium.org/9259/diff/1/6
File chrome/test/automation/SConscript (right):

http://codereview.chromium.org/9259/diff/1/6#newcode32
Line 32: #i = env.Install('$TARGET_ROOT', lib)
Right, we no longer need this because ChromeStaticLibary() puts it in
$TARGET_ROOT for us.  If so, just delete this and the next line.

[bradn, 2008-11-03 21:53:04]

http://codereview.chromium.org/9259/diff/1/4
File build/SConscript.main (right):

http://codereview.chromium.org/9259/diff/1/4#newcode652
Line 652: # This must be last!
On 2008/11/03 19:16:14, sgk wrote:
> A little more explanation in the comment about *why* it must be last (so
someone
> in the future who thinks their new code is the one that really should go last
> has a chance of deciding correctly).

Done.

http://codereview.chromium.org/9259/diff/1/4#newcode659
Line 659: solution_env.Alias('solution', solution)
On 2008/11/03 19:16:14, sgk wrote:
> Don't need this Alias since we're going to use the env.Bit('solution') to
> control generation of the solution files.

Done.

http://codereview.chromium.org/9259/diff/1/4#newcode660
Line 660: Default(None)
On 2008/11/03 19:16:14, sgk wrote:
> Comment about why we need Default(None) here.  Otherwise it's pretty obscure
to
> people reading the code.

Done.

http://codereview.chromium.org/9259/diff/1/4#newcode661
Line 661: Default(Alias('solution'))
On 2008/11/03 19:16:14, sgk wrote:
> Ditto here, should be able to pass in the proper Node.

Done.

http://codereview.chromium.org/9259/diff/1/3
File build/SConscript.v8 (left):

http://codereview.chromium.org/9259/diff/1/3#oldcode79
Line 79: env.ChromeStaticLibrary('$V8_DIR/v8_snapshot',
Parts of hammer assume libraries are build into the current directory. I've
adjusted things to do this a different way.

On 2008/11/03 19:16:14, sgk wrote:
> ???  Why removing this?  In my last rounds of work, it was necessary to make
> sure the v8_snapshot.lib file was generated in the output directory
(Hammer/v8),
> not in the build/ source directory.

http://codereview.chromium.org/9259/diff/1/8
File chrome/SConscript (right):

http://codereview.chromium.org/9259/diff/1/8#newcode44
Line 44: env_res.Requires(chrome_dll_res,
'installer/util/installer_util_strings.rc')
On 2008/11/03 19:16:14, sgk wrote:
> Is Requires() correct here?  If installer_util_string.rc is used in the
> generation of chrome_dll.res, then usually it would need a Depends() so that
the
> target is re-generated if the dependency changes.  (Needing
> installer_util_strings.rc to be present but not caring about whether its
> contents change would be a little unusually, but might be correct...)

Done.

http://codereview.chromium.org/9259/diff/1/8#newcode246
Line 246: '$VERSION_BAT $SOURCE $CHROME_SRC_DIR/chrome/ $PWD $TARGET',
On 2008/11/03 19:16:14, sgk wrote:
> Ouch--the trailing / is necessary here?  If so, a comment would be helpful.

Done.

http://codereview.chromium.org/9259/diff/1/6
File chrome/test/automation/SConscript (right):

http://codereview.chromium.org/9259/diff/1/6#newcode32
Line 32: #i = env.Install('$TARGET_ROOT', lib)
On 2008/11/03 19:16:14, sgk wrote:
> Right, we no longer need this because ChromeStaticLibary() puts it in
> $TARGET_ROOT for us.  If so, just delete this and the next line.

Done.

[sgk, 2008-11-04 00:29:03]

lgtm; one nit

http://codereview.chromium.org/9259/diff/230/53
File site_scons/site_tools/visual_studio_solution.py (right):

http://codereview.chromium.org/9259/diff/230/53#newcode92
Line 92: 'hammer.bat MODE=all',
Make part of string on previous line.