Fix clobber.py to restore args.gn on failure

Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940
Committed: https://crrev.com/562df4f33e0a29669b83d451d33c81648a9e023e
Cr-Commit-Position: refs/heads/master@{#395177}

[brucedawson, 2016-05-20 00:51:05]

Description was changed from

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

BUG=612940
==========

to

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940
==========

[brucedawson, 2016-05-20 00:51:05]

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

[brucedawson, 2016-05-20 00:51:23]

Code review practice!

[Ilya Kulshin, 2016-05-20 20:04:26]

lgtm

https://codereview.chromium.org/1996833002/diff/1/build/clobber.py
File build/clobber.py (right):

https://codereview.chromium.org/1996833002/diff/1/build/clobber.py#newcode72
build/clobber.py:72: # and we still want to restore args.gn and build.ninja, so
catch the
Also make note of regenerating the .d files

[brucedawson, 2016-05-20 20:15:17]

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

[brucedawson, 2016-05-20 20:15:18]

Comment fixed - stanisc@, can you review and give committer stamp of approval?

[stanisc, 2016-05-20 21:10:14]

lgtm

The only question I have is whether the code that catches exception should use a
specific exception type vs. generic Exception. But that probably doesn't matter
in this case.

[brucedawson, 2016-05-20 21:13:51]

On 2016/05/20 21:10:14, stanisc wrote:
> lgtm
> 
> The only question I have is whether the code that catches exception should use
a
> specific exception type vs. generic Exception. But that probably doesn't
matter
> in this case.

Good point. However the specific type of exception here is Windows specific so
catching it actually violates one of the pylint rules - I hit that in a previous
change where I tried to be more tightly scoped.

Also, since I'm catching and then rethrowing the exception I think it's
reasonable to catch everything. More care is needed when swallowing exceptions.

[brucedawson, 2016-05-20 21:13:56]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-05-20 21:13:56]

The patchset sent to the CQ was uploaded after l-g-t-m from kulshin@chromium.org
Link to the patchset: https://codereview.chromium.org/1996833002/#ps20001
(title: "Tweak comment")

[commit-bot: I haz the power, 2016-05-20 21:14:22]

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

[commit-bot: I haz the power, 2016-05-20 22:00:56]

Description was changed from

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940
==========

to

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940
==========

[commit-bot: I haz the power, 2016-05-20 22:00:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-20 22:02:41]

Description was changed from

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940
==========

to

==========
Fix clobber.py to restore args.gn on failure

If delete_dir or os.mkdir throw an exception then the code to restore
args.gn, build.ninja, and build.ninja.d will not run. This breaks a
design goal of gn which is that the args.gn files should not be
deleted.

The fix is to catch the exception and raise it after the files have
been restored.

To test the code leave chrome.exe or base_unittests.exe running in one
window and run this in another:

    python build\clobber.py d:\src\chromium\src\out

Note that if you do this without the patch applied then all args.gn
files may be deleted, so consider backing them up. Also note that
args.gn files won't be restored unless build.ninja and build.ninja.d
files are also present, so back them up.

Just apply the patch first - it's easier.

BUG=612940

Committed: https://crrev.com/562df4f33e0a29669b83d451d33c81648a9e023e
Cr-Commit-Position: refs/heads/master@{#395177}
==========

[commit-bot: I haz the power, 2016-05-20 22:02:42]

Patchset 2 (id:??) landed as
https://crrev.com/562df4f33e0a29669b83d451d33c81648a9e023e
Cr-Commit-Position: refs/heads/master@{#395177}