Make apply_issue.py more aggressive when deleting an entry from a DEPS file.

Make apply_issue.py more aggressive when deleting an entry from a DEPS file.

Without this change, if an entry is removed from a DEPS file, it won't be
effectively deleted. Specify --delete_unversioned_trees so the dependency is
properly deleted.

TBR=szager@chromium.org
BUG=

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

[commit-bot: I haz the power, 2013-04-29 18:00:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/maruel@chromium.org/14531006/1

[commit-bot: I haz the power, 2013-04-29 18:01:50]

Change committed as 197081

[Dirk Pranke, 2013-04-29 18:19:50]

This change lgtm -- and I would make it regardless of what I say next -- but
doesn't the new 'gclient revert' step make this moot (at least on the next run)?
Are there cases where leaving the unversioned dir in the tree for a run causes
problems?

[M-A Ruel, 2013-04-29 18:28:14]

On 2013/04/29 18:19:50, Dirk Pranke wrote:
> This change lgtm -- and I would make it regardless of what I say next -- but
> doesn't the new 'gclient revert' step make this moot (at least on the next
run)?
> Are there cases where leaving the unversioned dir in the tree for a run causes
> problems?

gclient revert is run before, not after, applying the diff. This CL is to ensure
that a removed entry if effectively removed. It's important when doing code
deletion.

[Dirk Pranke, 2013-04-29 18:36:12]

On Mon, Apr 29, 2013 at 11:28 AM, <maruel@chromium.org> wrote:

> On 2013/04/29 18:19:50, Dirk Pranke wrote:
>
>> This change lgtm -- and I would make it regardless of what I say next --
>> but
>> doesn't the new 'gclient revert' step make this moot (at least on the next
>>
> run)?
>
>> Are there cases where leaving the unversioned dir in the tree for a run
>> causes
>> problems?
>>
>
> gclient revert is run before, not after, applying the diff.


Right, which is why I said it would get cleaned up on the next run.



> This CL is to ensure
> that a removed entry if effectively removed. It's important when doing code
> deletion.
>

Important how? What are you concerned about breaking on the same run? maybe
the deletion wasn't actually right and we miss compile failures until the
next run?

(Again, I think this is a good change, just wondering what caused you to
notice it and/or what this might be fixing).

-- Dirk

[M-A Ruel, 2013-04-29 18:40:07]

On 2013/04/29 18:36:12, Dirk Pranke wrote:
> On Mon, Apr 29, 2013 at 11:28 AM, <mailto:maruel@chromium.org> wrote:
> > This CL is to ensure
> > that a removed entry if effectively removed. It's important when doing code
> > deletion.
> >
> 
> Important how? What are you concerned about breaking on the same run? maybe
> the deletion wasn't actually right and we miss compile failures until the
> next run?

1. Remove references to a third_party.
2. Remove the third_party from DEPS. This wasn't effective before this CL.
3. Ensure it still compiles. Before this CL, it was not tested at all since the
dependency was left in its old state.
 
> (Again, I think this is a good change, just wondering what caused you to
> notice it and/or what this might be fixing).

Example: https://codereview.chromium.org/13993025/

[M-A Ruel, 2013-04-29 19:13:50]

Note that this CL exposed the underlying bug https://crbug.com/236505 so I
finally understood what was going on. Long story short, ios_* try builders never
worked properly with git cl try.