Make gn wrapper use the one in buildtools rather than tools/gn/bin.

Make gn wrapper use the one in buildtools rather than tools/gn/bin.

This updates some infrastructure to make it easy to get at the platform-specific build tools directories.

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

[brettw, 2014-06-18 00:43:19]

Note that this can't be checked in until I enable actually pulling buildtools
binaries in the Chrome tree.

[M-A Ruel, 2014-06-18 00:55:36]

What about backward compatibility? E.g. checking out an older tree.

https://codereview.chromium.org/341533006/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/341533006/diff/1/gclient_utils.py#newcode653
gclient_utils.py:653: def GetBuildtoolsPath():
Why put these functions here and not put it in gn.py?

[brettw, 2014-06-18 05:41:05]

On 2014/06/18 00:55:36, M-A Ruel wrote:
> What about backward compatibility? E.g. checking out an older tree.

Since there are only a few people using this and it's not used for "real work"
yet, I'm not concerned about backwards compat yet. Also, GN itself is generally
backward compatible so if you do that, you'll use the new binary on the old tree
which "should" work in most cases.

[brettw, 2014-06-18 05:41:18]

https://codereview.chromium.org/341533006/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/341533006/diff/1/gclient_utils.py#newcode653
gclient_utils.py:653: def GetBuildtoolsPath():
I think some more stuff will be added to buildtools that will need this kind of
thing.

[brettw, 2014-06-18 17:20:53]

On 2014/06/18 05:41:18, brettw wrote:
> https://codereview.chromium.org/341533006/diff/1/gclient_utils.py
> File gclient_utils.py (right):
> 
> https://codereview.chromium.org/341533006/diff/1/gclient_utils.py#newcode653
> gclient_utils.py:653: def GetBuildtoolsPath():
> I think some more stuff will be added to buildtools that will need this kind
of
> thing.

e.g. clang-format is being planned to move here.

[M-A Ruel, 2014-06-18 17:25:17]

On 2014/06/18 17:20:53, brettw wrote:
> On 2014/06/18 05:41:18, brettw wrote:
> > https://codereview.chromium.org/341533006/diff/1/gclient_utils.py
> > File gclient_utils.py (right):
> > 
> > https://codereview.chromium.org/341533006/diff/1/gclient_utils.py#newcode653
> > gclient_utils.py:653: def GetBuildtoolsPath():
> > I think some more stuff will be added to buildtools that will need this kind
> of
> > thing.
> 
> e.g. clang-format is being planned to move here.

Ok, I personally don't mind but since I'm not officially maintaining depot_tools
myself anymore, I'd prefer someone who will have to handle this in the future to
give his approval. Tagging Robbie.

[brettw, 2014-06-19 22:45:58]

iannucci: ping

[iannucci, 2014-06-20 22:24:22]

lgtm, though I think having mac64 and win64 would be great (and erroring out if
you try to build on a 32 bit platform on either).

https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py#newc...
gclient_utils.py:673: subdir = 'mac'
Can we keep the naming scheme uniform across all platforms? Even if we only have
one binary, I think that uniformity will help in the long run (fewer special
cases, etc.).

[brettw, 2014-06-20 23:43:37]

https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py#newc...
gclient_utils.py:673: subdir = 'mac'
That doesn't really make sense. The idea here is that there is one directory for
all the binaries for your platform. I would like to change the download script
to just download all binaries from the appropriate directory (this involves
encoding the bucket in the .sha1 file somehow).

The Windows and Mac tools in the checkout are 32-bit, even though basically all
development platforms are 64-bit. I might change to 64-bit builds sometime in
the future for GN, but I am certainly not going to make both 32-bit and 64-bit
builds.

Linux is different here because there is typically no capability for running
32-bit binaries on 64-bit systems. We need a 32-bit GN because some of the
Android builds happen on some kind of Google AppEngine thing that's 32-bit.

I'm thinking about it that tools we want regardless of bit-tedness would go into
win/mac/linux, and the bit-specific ones would go into
win32/win64/mac32/mac64/linux32/linux64. We just don't currently have all of
those combinations.

[iannucci, 2014-06-20 23:46:48]

On 2014/06/20 23:43:37, brettw wrote:
> https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py
> File gclient_utils.py (right):
> 
>
https://chromiumcodereview.appspot.com/341533006/diff/1/gclient_utils.py#newc...
> gclient_utils.py:673: subdir = 'mac'
> That doesn't really make sense. The idea here is that there is one directory
for
> all the binaries for your platform. I would like to change the download script
> to just download all binaries from the appropriate directory (this involves
> encoding the bucket in the .sha1 file somehow).
> 
> The Windows and Mac tools in the checkout are 32-bit, even though basically
all
> development platforms are 64-bit. I might change to 64-bit builds sometime in
> the future for GN, but I am certainly not going to make both 32-bit and 64-bit
> builds.
> 
> Linux is different here because there is typically no capability for running
> 32-bit binaries on 64-bit systems. We need a 32-bit GN because some of the
> Android builds happen on some kind of Google AppEngine thing that's 32-bit.
> 
> I'm thinking about it that tools we want regardless of bit-tedness would go
into
> win/mac/linux, and the bit-specific ones would go into
> win32/win64/mac32/mac64/linux32/linux64. We just don't currently have all of
> those combinations.

alright... sounds like you're thinking about it and have a strategy in mind
here, so that seems fine to me. I just wanted to avoid the 'everything is the
same except this one' problem. I guess the algorithm could be generalized to:
  * look in plat
  * otherwise look in plat+bits
  * otherwise fail

And this is just a reduced version of that algorithm.

lgtm

[brettw, 2014-06-23 17:21:52]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-06-23 17:22:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/brettw@chromium.org/341533006/1

[commit-bot: I haz the power, 2014-06-23 17:24:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-23 17:24:43]

Presubmit check for 341533006-1 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
Pylint (97 files) (41.86s) failed
************* Module
/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_utils.py
W0301:668,0: Unnecessary semicolon


Presubmit checks took 120.5s to calculate.

Was the presubmit check useful? If not, run "git cl presubmit -v"
to figure out which PRESUBMIT.py was run, then run git blame
on the file to figure out who to ask for help.

[brettw, 2014-06-23 17:27:44]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-06-23 17:28:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/brettw@chromium.org/341533006/20001

[commit-bot: I haz the power, 2014-06-23 17:30:35]

Change committed as 279132

[kjellander (google.com), 2014-06-26 12:28:53]

On 2014/06/23 17:30:35, I haz the power (commit-bot) wrote:
> Change committed as 279132

This broke GN for WebRTC (as we don't yet pull the buildtools directory in our
DEPS). I'll fix this but please try to keep us informed of changes like this,
even if it's hard to know how changes affect the "client" projects of Chromium.

[brettw, 2014-06-26 17:11:57]

On 2014/06/26 12:28:53, kjellander (DO NOT USE THIS) wrote:
> On 2014/06/23 17:30:35, I haz the power (commit-bot) wrote:
> > Change committed as 279132
> 
> This broke GN for WebRTC (as we don't yet pull the buildtools directory in our
> DEPS). I'll fix this but please try to keep us informed of changes like this,
> even if it's hard to know how changes affect the "client" projects of
Chromium.

Please subscribe to gn-dev for updates. It's low traffic.

[kjellander_chromium, 2014-06-26 19:36:26]

On 2014/06/26 17:11:57, brettw wrote:
> On 2014/06/26 12:28:53, kjellander (DO NOT USE THIS) wrote:
> > On 2014/06/23 17:30:35, I haz the power (commit-bot) wrote:
> > > Change committed as 279132
> > 
> > This broke GN for WebRTC (as we don't yet pull the buildtools directory in
our
> > DEPS). I'll fix this but please try to keep us informed of changes like
this,
> > even if it's hard to know how changes affect the "client" projects of
> Chromium.
> 
> Please subscribe to gn-dev for updates. It's low traffic.

Done, thanks.

This CL breaks also breaks all projects that don't have a top-level folder named
'src' (WebRTC developers usually have 'trunk'). 
Would it be possible to get that back?

[brettw, 2014-06-30 17:43:49]

On 2014/06/26 19:36:26, kjellander wrote:
> On 2014/06/26 17:11:57, brettw wrote:
> > On 2014/06/26 12:28:53, kjellander (DO NOT USE THIS) wrote:
> > > On 2014/06/23 17:30:35, I haz the power (commit-bot) wrote:
> > > > Change committed as 279132
> > > 
> > > This broke GN for WebRTC (as we don't yet pull the buildtools directory in
> our
> > > DEPS). I'll fix this but please try to keep us informed of changes like
> this,
> > > even if it's hard to know how changes affect the "client" projects of
> > Chromium.
> > 
> > Please subscribe to gn-dev for updates. It's low traffic.
> 
> Done, thanks.
> 
> This CL breaks also breaks all projects that don't have a top-level folder
named
> 'src' (WebRTC developers usually have 'trunk'). 
> Would it be possible to get that back?

Hm, how do you think it should work? Should we go back to looking for the .gn
file? That sounds kind of messy to me.

[Dirk Pranke, 2014-06-30 21:27:51]

this is a hassle, but maybe you can parse the .gclient and figure out the dir
name of the first solution, which presumably is either 'src' or 'trunk' ?

[iannucci, 2014-06-30 21:34:15]

On 2014/06/30 21:27:51, Dirk Pranke wrote:
> this is a hassle, but maybe you can parse the .gclient and figure out the dir
> name of the first solution, which presumably is either 'src' or 'trunk' ?

The other thing we should consider is to turn on 'use_relative_paths' in the
chromium DEPS file. There are still a lot of places which assume 'src', but this
is a big one (and in general, I think it's a poor assumption which leads to
problems like this one).

[iannucci, 2014-06-30 21:34:52]

On 2014/06/30 21:34:15, iannucci wrote:
> On 2014/06/30 21:27:51, Dirk Pranke wrote:
> > this is a hassle, but maybe you can parse the .gclient and figure out the
dir
> > name of the first solution, which presumably is either 'src' or 'trunk' ?
> 
> The other thing we should consider is to turn on 'use_relative_paths' in the
> chromium DEPS file. There are still a lot of places which assume 'src', but
this
> is a big one (and in general, I think it's a poor assumption which leads to
> problems like this one).

Oh. Nevermind. That won't help.

We should do that anyway though :)

[cjhopman, 2014-09-19 21:49:51]

On 2014/06/30 21:34:52, iannucci wrote:
> On 2014/06/30 21:34:15, iannucci wrote:
> > On 2014/06/30 21:27:51, Dirk Pranke wrote:
> > > this is a hassle, but maybe you can parse the .gclient and figure out the
> dir
> > > name of the first solution, which presumably is either 'src' or 'trunk' ?
> > 
> > The other thing we should consider is to turn on 'use_relative_paths' in the
> > chromium DEPS file. There are still a lot of places which assume 'src', but
> this
> > is a big one (and in general, I think it's a poor assumption which leads to
> > problems like this one).
> 
> Oh. Nevermind. That won't help.
> 
> We should do that anyway though :)

Ugh, this broke android internal builds too and I'm definitely subscribed to
gn-dev (though I obviously don't sync depot_tools very often).

[cjhopman, 2014-09-19 21:50:06]

On 2014/09/19 21:49:51, cjhopman wrote:
> On 2014/06/30 21:34:52, iannucci wrote:
> > On 2014/06/30 21:34:15, iannucci wrote:
> > > On 2014/06/30 21:27:51, Dirk Pranke wrote:
> > > > this is a hassle, but maybe you can parse the .gclient and figure out
the
> > dir
> > > > name of the first solution, which presumably is either 'src' or 'trunk'
?
> > > 
> > > The other thing we should consider is to turn on 'use_relative_paths' in
the
> > > chromium DEPS file. There are still a lot of places which assume 'src',
but
> > this
> > > is a big one (and in general, I think it's a poor assumption which leads
to
> > > problems like this one).
> > 
> > Oh. Nevermind. That won't help.
> > 
> > We should do that anyway though :)
> 
> Ugh, this broke android internal builds too and I'm definitely subscribed to
> gn-dev (though I obviously don't sync depot_tools very often).

That is, gn builds with android internal checkouts.

[cjhopman, 2014-09-19 21:51:46]

On 2014/09/19 21:50:06, cjhopman wrote:
> On 2014/09/19 21:49:51, cjhopman wrote:
> > On 2014/06/30 21:34:52, iannucci wrote:
> > > On 2014/06/30 21:34:15, iannucci wrote:
> > > > On 2014/06/30 21:27:51, Dirk Pranke wrote:
> > > > > this is a hassle, but maybe you can parse the .gclient and figure out
> the
> > > dir
> > > > > name of the first solution, which presumably is either 'src' or
'trunk'
> ?
> > > > 
> > > > The other thing we should consider is to turn on 'use_relative_paths' in
> the
> > > > chromium DEPS file. There are still a lot of places which assume 'src',
> but
> > > this
> > > > is a big one (and in general, I think it's a poor assumption which leads
> to
> > > > problems like this one).
> > > 
> > > Oh. Nevermind. That won't help.
> > > 
> > > We should do that anyway though :)
> > 
> > Ugh, this broke android internal builds too and I'm definitely subscribed to
> > gn-dev (though I obviously don't sync depot_tools very often).
> 
> That is, gn builds with android internal checkouts.

and it was actually the fix in https://codereview.chromium.org/538393002 that
broke android