Move control of updating toolchain into src/

Move control of updating toolchain into src/

Moved out of depot_tools here: https://codereview.chromium.org/168603004/

The control is in src/ now, but logic for de-duplication is still in depot_tools.

This changes the default, so that VS2013 will be the default toolchain.

R=iannucci@chromium.org
BUG=323300, 309197
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252733

[scottmg, 2014-02-21 23:52:55]

+maruel, we're thinking to move the execution of update into gyp_chromium, with
the hope of having src/ control asking for a specific hash in the future, but
still having it centralized to one copy in depot_tools if there's multiple
trees. I think this makes more sense.

[iannucci, 2014-02-21 23:56:13]

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium
File build/gyp_chromium (right):

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
build/gyp_chromium:463: get_toolchain_args.extend(GetDesiredVsToolchainHashes())
the + operator works too :P

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
build/gyp_chromium:470: # at a time.
We should probably have the depot tools thing print out the path to the
installed tools, or, better, the path to the batch file / json file with the env
vars.

[scottmg, 2014-02-22 00:03:45]

On 2014/02/21 23:56:13, iannucci wrote:
> https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium
> File build/gyp_chromium (right):
> 
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> build/gyp_chromium:463:
get_toolchain_args.extend(GetDesiredVsToolchainHashes())
> the + operator works too :P
> 
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> build/gyp_chromium:470: # at a time.
> We should probably have the depot tools thing print out the path to the
> installed tools, or, better, the path to the batch file / json file with the
env
> vars.

OK, I'll take a look at what I can have it save out to avoid some of the code in
gyp_chromium.

[scottmg, 2014-02-22 00:10:42]

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium
File build/gyp_chromium (right):

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
build/gyp_chromium:463: get_toolchain_args.extend(GetDesiredVsToolchainHashes())
On 2014/02/21 23:56:14, iannucci wrote:
> the + operator works too :P

maruel always asks for + -> extend/append. Geez, Code Reviews! 9_9

https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
build/gyp_chromium:470: # at a time.
On 2014/02/21 23:56:14, iannucci wrote:
> We should probably have the depot tools thing print out the path to the
> installed tools, or, better, the path to the batch file / json file with the
env
> vars.

This seems better, except it's a somewhat long process, so I'd like to have it
send some output to stdout for the user's benefit. It could write a file ...
somewhere?

[iannucci, 2014-02-22 00:18:54]

On 2014/02/22 00:10:42, scottmg wrote:
> https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium
> File build/gyp_chromium (right):
> 
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> build/gyp_chromium:463:
get_toolchain_args.extend(GetDesiredVsToolchainHashes())
> On 2014/02/21 23:56:14, iannucci wrote:
> > the + operator works too :P
> 
> maruel always asks for + -> extend/append. Geez, Code Reviews! 9_9

Lol, w/e I think nitpicking is dumb anyway :)

> 
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> build/gyp_chromium:470: # at a time.
> On 2014/02/21 23:56:14, iannucci wrote:
> > We should probably have the depot tools thing print out the path to the
> > installed tools, or, better, the path to the batch file / json file with the
> env
> > vars.
> 
> This seems better, except it's a somewhat long process, so I'd like to have it
> send some output to stdout for the user's benefit. It could write a file ...
> somewhere?

Hm. Could usurp stderr, but that feels wrong. In recipes we've been doing a
`--output-json /path/to/file` convention which has worked out pretty well so
far. It also lets you extend the information blob later and you don't neet to do
any custom parsing crap.

[scottmg, 2014-02-22 00:21:16]

On 2014/02/22 00:18:54, iannucci wrote:
> On 2014/02/22 00:10:42, scottmg wrote:
> > https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium
> > File build/gyp_chromium (right):
> > 
> >
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> > build/gyp_chromium:463:
> get_toolchain_args.extend(GetDesiredVsToolchainHashes())
> > On 2014/02/21 23:56:14, iannucci wrote:
> > > the + operator works too :P
> > 
> > maruel always asks for + -> extend/append. Geez, Code Reviews! 9_9
> 
> Lol, w/e I think nitpicking is dumb anyway :)
> 
> > 
> >
>
https://codereview.chromium.org/175573004/diff/90003/build/gyp_chromium#newco...
> > build/gyp_chromium:470: # at a time.
> > On 2014/02/21 23:56:14, iannucci wrote:
> > > We should probably have the depot tools thing print out the path to the
> > > installed tools, or, better, the path to the batch file / json file with
the
> > env
> > > vars.
> > 
> > This seems better, except it's a somewhat long process, so I'd like to have
it
> > send some output to stdout for the user's benefit. It could write a file ...
> > somewhere?
> 
> Hm. Could usurp stderr, but that feels wrong. In recipes we've been doing a
> `--output-json /path/to/file` convention which has worked out pretty well so
> far. It also lets you extend the information blob later and you don't neet to
do
> any custom parsing crap.

Yeah, sg. I'll tempfile a json to pass for it to write to.

[scottmg, 2014-02-22 01:12:06]

PTAL; now gets --output-json that contains paths, etc. to toolchain.

[iannucci, 2014-02-22 01:49:40]

lgtm + thingies

https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium
File build/gyp_chromium (right):

https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium#newc...
build/gyp_chromium:459: with tempfile.NamedTemporaryFile(delete=False,
suffix='.json') as tempf:
doesn't this close the tempfile? I guess it doesn't auto-unlink on windows... I
would put this -> json.load all inside the with statement and use tempf
directly. I think there's an unlink option you can pass to NamedTemporaryFile
too...

Oh, or are you trying to avoid having it open by 2 procs at once?

If so, I would use mkstemp
(http://docs.python.org/2/library/tempfile.html#tempfile.mkstemp) directly as
the intent is clearer.

https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium#newc...
build/gyp_chromium:495: os.environ['PATH'] = runtime_path + ';' +
os.environ['PATH']
kinda feel like this env var update should just be
os.environ.update(toolchain_data['env']), but we can do that later.

[scottmg, 2014-02-22 01:58:52]

On 2014/02/22 01:49:40, iannucci wrote:
> lgtm + thingies

Thanks

> 
> https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium
> File build/gyp_chromium (right):
> 
>
https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium#newc...
> build/gyp_chromium:459: with tempfile.NamedTemporaryFile(delete=False,
> suffix='.json') as tempf:
> doesn't this close the tempfile? I guess it doesn't auto-unlink on windows...
I
> would put this -> json.load all inside the with statement and use tempf
> directly. I think there's an unlink option you can pass to NamedTemporaryFile
> too...

Yeah, delete=False is passed there. I was following the example of
http://docs.python.org/2/library/tempfile.html#tempfile.mktemp

> 
> Oh, or are you trying to avoid having it open by 2 procs at once?

Right, gyp_chromium is creating the name, but depot_tools is writing the data
there, so it needs to be closed (on Windows) for the other one to write it.

> 
> If so, I would use mkstemp
> (http://docs.python.org/2/library/tempfile.html#tempfile.mkstemp) directly as
> the intent is clearer.

Ah, I thought it cleaned up automatically. Switched.

Wacky, it returns (fileno, path), not (file object, path).

> 
>
https://codereview.chromium.org/175573004/diff/150001/build/gyp_chromium#newc...
> build/gyp_chromium:495: os.environ['PATH'] = runtime_path + ';' +
> os.environ['PATH']
> kinda feel like this env var update should just be
> os.environ.update(toolchain_data['env']), but we can do that later.

Yeah, that'd be ideal, but it's a bit tricky with the GYP_DEFINES merging
windows_sdk_path business. Hopefully we've got enough room/rope to extend/hang
ourselves in the future here.

[scottmg, 2014-02-22 02:00:15]

(Also, just to be clear, this CL will switch the default toolchain to VS2013)

[scottmg, 2014-02-22 02:41:44]

Committed patchset #7 manually as r252733 (tree was closed).