Create a temporary dir for patch to do its work.

Create a temporary dir for patch to do its work.

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

[Isaac (away), 2013-07-12 04:03:42]

In general looks good, some nits and I'd also be curious to know *why* we're
having problems :-\.

https://codereview.chromium.org/18664004/diff/1/checkout.py
File checkout.py (right):

https://codereview.chromium.org/18664004/diff/1/checkout.py#newcode624
checkout.py:624: old_tmpdir = os.environ['TMPDIR']
are you sure this environment variable exists?

https://codereview.chromium.org/18664004/diff/1/checkout.py#newcode628
checkout.py:628: shutil.rmtree(new_tmpdir)
this should be a try finally (with rmtree in the finally section)

[groby-ooo-7-16, 2013-07-12 04:16:16]

https://codereview.chromium.org/18664004/diff/1/checkout.py
File checkout.py (right):

https://codereview.chromium.org/18664004/diff/1/checkout.py#newcode624
checkout.py:624: old_tmpdir = os.environ['TMPDIR']
Doesn't matter if it exists - according to the patch sources [1], it's the one
that takes precedence over all other environment variables.

[1]: http://gnuwin32.sourceforge.net/packages/patch.htm

On 2013/07/12 04:03:42, Isaac wrote:
> are you sure this environment variable exists?

https://codereview.chromium.org/18664004/diff/1/checkout.py#newcode628
checkout.py:628: shutil.rmtree(new_tmpdir)
On 2013/07/12 04:03:42, Isaac wrote:
> this should be a try finally (with rmtree in the finally section)

Done.

[Isaac (away), 2013-07-12 05:30:23]

This lgtm.  It's been failing for a while so let's wait for an owners review.

[Isaac (away), 2013-07-12 07:58:01]

[note: this still needs to be tested, please do not land]

[M-A Ruel, 2013-07-12 17:45:12]

On 2013/07/12 07:58:01, Isaac wrote:
> [note: this still needs to be tested, please do not land]

lgtm but I agree with Isaac, it's preferable to test it manually first.

[groby-ooo-7-16, 2013-07-12 18:32:11]

Any advice on how to do that? (Especially given I don't have a windows machine)

[Isaac (away), 2013-07-12 19:29:23]

I tested -- it looks good.  Sending to CQ.

[commit-bot: I haz the power, 2013-07-12 19:29:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/18664004/21001

[commit-bot: I haz the power, 2013-07-12 19:32:35]

Change committed as 211443