Run get_toolchain_if_necessary.py with depot_tools Python under Cygwin

Run get_toolchain_if_necessary.py with depot_tools Python under Cygwin

get_toolchain_if_necessary.py contains a number of Windows-isms, including
computing hashes on Windows-style path names, so does not work under
Cygwin. This change reruns it under depot_tools' Windows Python if run
from Cygwin Python.

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

[iannucci, 2014-04-11 11:01:33]

lg but I'm punting to scott for the final review, since he's more Attuned to the
Way of Windows :)

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
win_toolchain/get_toolchain_if_necessary.py:203: subprocess_command = [python,
winpath(__file__)]
nit: `cmd` would be a fine name for this variable, IMHO :)

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
win_toolchain/get_toolchain_if_necessary.py:209: sys.exit(p.returncode)
this should be

sys.exit(subprocess.call(subprocess_command))

shell=False is the default I think, and there's no need to consume the output of
the nested command... letting it print directly to stdout/stderr is fine.

[Mike Wittman, 2014-04-11 17:58:04]

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
File win_toolchain/get_toolchain_if_necessary.py (right):

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
win_toolchain/get_toolchain_if_necessary.py:203: subprocess_command = [python,
winpath(__file__)]
On 2014/04/11 11:01:33, iannucci wrote:
> nit: `cmd` would be a fine name for this variable, IMHO :)

Done.

https://codereview.chromium.org/233563003/diff/1/win_toolchain/get_toolchain_...
win_toolchain/get_toolchain_if_necessary.py:209: sys.exit(p.returncode)
On 2014/04/11 11:01:33, iannucci wrote:
> this should be
> 
> sys.exit(subprocess.call(subprocess_command))
> 
> shell=False is the default I think, and there's no need to consume the output
of
> the nested command... letting it print directly to stdout/stderr is fine.

Done.

[scottmg, 2014-04-11 18:11:02]

My If-I-Ever-Get-A-Time-Machine list: cygwin, Hitler.

lgtmeh

[Mike Wittman, 2014-04-11 18:23:09]

The CQ bit was checked by wittman@chromium.org

[commit-bot: I haz the power, 2014-04-11 18:23:18]

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

[commit-bot: I haz the power, 2014-04-11 18:25:34]

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

[commit-bot: I haz the power, 2014-04-11 18:25:35]

Presubmit check for 233563003-20001 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 **
Missing LGTM from an OWNER for these files:
    depot_tools/win_toolchain/get_toolchain_if_necessary.py

Presubmit checks took 134.4s to calculate.

[Mike Wittman, 2014-04-11 18:28:29]

On 2014/04/11 18:25:35, I haz the power (commit-bot) wrote:
> Presubmit check for 233563003-20001 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 **
> Missing LGTM from an OWNER for these files:
>     depot_tools/win_toolchain/get_toolchain_if_necessary.py
> 
> Presubmit checks took 134.4s to calculate.

Robbie, can you formally approve?

[iannucci, 2014-04-11 18:43:59]

I am Robbie, and I LGTM this change.

[Mike Wittman, 2014-04-11 18:54:06]

The CQ bit was checked by wittman@chromium.org

[commit-bot: I haz the power, 2014-04-11 18:54:16]

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

[commit-bot: I haz the power, 2014-04-11 18:57:12]

Change committed as 263301

[Noel Gordon, 2014-04-12 05:17:55]

Thanks Mike.