Use rmdir /s/q instead of flaky shutil.rmtree on Windows

Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.

Committed: https://crrev.com/b0e768eae38f683886600ba869e31054273ebeab
Cr-Commit-Position: refs/heads/master@{#382140}

[brucedawson, 2016-03-16 23:32:17]

brucedawson@chromium.org changed reviewers:
+ scottmg@chromium.org

[brucedawson, 2016-03-16 23:32:18]

I am super unhappy about this change, but I'm also super unhappy about
continuously hitting this problem.

Any thoughts? Is anybody else hitting rmtree failures?

[scottmg, 2016-03-16 23:39:41]

I did hit this, yes. shutil.rmtree is junk on Windows.

Normally, we do `cmd /c rmdir /s/q path` on Windows instead as a workaround.
Maybe one of these sundry implementations is reachable from here?
https://code.google.com/p/chromium/codesearch#search/&q=rmtree%20file:.py&sq=...

[brucedawson, 2016-03-18 00:28:03]

Well it's *less* gross now. And it does the job of nuking the out directory
while preserving args.gn and build.ninja

And now I now that I should prefer "python build\clobber.py out" over "rmdir
/s/q out" when I need to force a full rebuild.

Good enough?

[brucedawson, 2016-03-18 00:29:23]

Description was changed from

==========
Retry shutil.rmtree on failure 'cause it's flaky

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This hacks around
the problem.
==========

to

==========
Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.
==========

[brucedawson, 2016-03-18 00:30:13]

I just updated the title and description also.

[scottmg, 2016-03-18 00:40:37]

lgtm

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py
File build/clobber.py (right):

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py#newcode43
build/clobber.py:43: if platform.system() == 'Windows':
I think "if sys.platform.startswith('win')" is more common. But if you have some
reason to do this, then that's fine. (If you switch, remove the import too.)

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py#newcode44
build/clobber.py:44: subprocess.call(['rmdir', '/s', '/q', build_dir],
shell=True)
.call -> .check_call

[brucedawson, 2016-03-18 20:55:50]

Fixes made as suggested. Will commit in a bit.

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py
File build/clobber.py (right):

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py#newcode43
build/clobber.py:43: if platform.system() == 'Windows':
On 2016/03/18 00:40:37, scottmg wrote:
> I think "if sys.platform.startswith('win')" is more common. But if you have
some
> reason to do this, then that's fine. (If you switch, remove the import too.)

Ah - yes. That's what I was looking for. Done (including removing the import).

https://codereview.chromium.org/1812673003/diff/40001/build/clobber.py#newcode44
build/clobber.py:44: subprocess.call(['rmdir', '/s', '/q', build_dir],
shell=True)
On 2016/03/18 00:40:37, scottmg wrote:
> .call -> .check_call

Done.

[brucedawson, 2016-03-18 21:38:18]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-03-18 21:38:19]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1812673003/#ps60001
(title: "Code review tweaks")

[commit-bot: I haz the power, 2016-03-18 21:38:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1812673003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1812673003/60001

[commit-bot: I haz the power, 2016-03-19 00:30:58]

Description was changed from

==========
Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.
==========

to

==========
Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.
==========

[commit-bot: I haz the power, 2016-03-19 00:31:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-19 00:33:19]

Description was changed from

==========
Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.
==========

to

==========
Use rmdir /s/q instead of flaky shutil.rmtree on Windows

For unknown reasons (Windows!) shutil.rmtree has been failing frequently
when the landmines change due to a toolchain change. This avoids the
the problem. This is necessary because manually deleting the 'out'
directory causes args.gn to get lost, which is a hassle.

This was noticed because the VS 2015 toolchain upgrades kept failing
to clobber the out directory on multiple machines.

Committed: https://crrev.com/b0e768eae38f683886600ba869e31054273ebeab
Cr-Commit-Position: refs/heads/master@{#382140}
==========

[commit-bot: I haz the power, 2016-03-19 00:33:20]

Patchset 4 (id:??) landed as
https://crrev.com/b0e768eae38f683886600ba869e31054273ebeab
Cr-Commit-Position: refs/heads/master@{#382140}