Fixing up hammer script to work in the nacl and o3d trees....

Fixing up hammer script to work in the nacl and o3d trees.

BUG=None
TEST=None


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

[M-A Ruel, 2009-08-06 22:27:42]

rubberstamp lgtm. Can you add the copyright header?

[bradn, 2009-08-06 22:30:22]

Added the header.

On Thu, Aug 6, 2009 at 3:27 PM, <maruel@chromium.org> wrote:

> rubberstamp lgtm. Can you add the copyright header?
>
>
> http://codereview.chromium.org/164099
>

[sgk, 2009-08-07 16:18:23]

http://codereview.chromium.org/164099/diff/4/5
File hammer (right):

http://codereview.chromium.org/164099/diff/4/5#newcode3
Line 3: # Copyright 2008 Google Inc.  All Rights Reserved.
I think this should be the "Chromium Authors" copyright, not the Google
copyright.

http://codereview.chromium.org/164099/diff/4/5#newcode24
Line 24: while [[ ! -d "${SRC_DIR}/site_scons" ||
I'm uncomfortable assuming every potential outside developer will have a /bin/sh
that's actually bash (even though that's pretty common).  The test should really
be done with more portable, old school use of "test" so that it works even if
the version of shell being used doesn't understand the [[ expression evaluation:

while `test ! -d "${SRC_DIR}/site_scons" -o -e "${SRC_DIR}/..."`; do

http://codereview.chromium.org/164099/diff/4/5#newcode27
Line 27: if [[ "${SRC_DIR}" == "${PARENT_DIR}" ]]; then
if `test "${SRC_DIR}" = "${PARENT_DIR}"`; then

[bradn, 2009-08-10 17:55:35]

Ok, have another look.
Does this mechanism even make sense if you end up factoring out the stuff in
site_init?

http://codereview.chromium.org/164099/diff/4/5
File hammer (right):

http://codereview.chromium.org/164099/diff/4/5#newcode3
Line 3: # Copyright 2008 Google Inc.  All Rights Reserved.
On 2009/08/07 16:18:23, sgk wrote:
> I think this should be the "Chromium Authors" copyright, not the Google
> copyright.

Done.

http://codereview.chromium.org/164099/diff/4/5#newcode24
Line 24: while [[ ! -d "${SRC_DIR}/site_scons" ||
On 2009/08/07 16:18:23, sgk wrote:
> I'm uncomfortable assuming every potential outside developer will have a
/bin/sh
> that's actually bash (even though that's pretty common).  The test should
really
> be done with more portable, old school use of "test" so that it works even if
> the version of shell being used doesn't understand the [[ expression
evaluation:
> 
> while `test ! -d "${SRC_DIR}/site_scons" -o -e "${SRC_DIR}/..."`; do
> 

Done.

http://codereview.chromium.org/164099/diff/4/5#newcode27
Line 27: if [[ "${SRC_DIR}" == "${PARENT_DIR}" ]]; then
On 2009/08/07 16:18:23, sgk wrote:
> if `test "${SRC_DIR}" = "${PARENT_DIR}"`; then
> 

Done.

[sgk, 2009-08-10 18:07:08]

lgtm modulo one readability nit

Yes, it still makes sense to do this. 
site_scons/site_tools/chromium_builders.py isn't going away entirely, just
having most of it ripped out.  We'll still need it to pull in the GRIT and
data_pack tool modules.

http://codereview.chromium.org/164099/diff/8/9
File hammer (right):

http://codereview.chromium.org/164099/diff/8/9#newcode14
Line 14: while `test ! -d "${SRC_DIR}/site_scons" -o -e \
Nit:  put the -e down on the same line with the directory, so it's easier to
read that that's what's being tested for existence.

[bradn, 2009-08-11 00:46:29]

Done, uploaded and commited...

On Mon, Aug 10, 2009 at 11:07 AM, <sgk@chromium.org> wrote:

> lgtm modulo one readability nit
>
> Yes, it still makes sense to do this.
> site_scons/site_tools/chromium_builders.py isn't going away entirely,
> just having most of it ripped out.  We'll still need it to pull in the
> GRIT and data_pack tool modules.
>
>
> http://codereview.chromium.org/164099/diff/8/9
> File hammer (right):
>
> http://codereview.chromium.org/164099/diff/8/9#newcode14
> Line 14: while `test ! -d "${SRC_DIR}/site_scons" -o -e \
> Nit:  put the -e down on the same line with the directory, so it's
> easier to read that that's what's being tested for existence.
>
>
> http://codereview.chromium.org/164099
>