Revert "Make check for dirty index work."

Revert "Make check for dirty index work."

This reverts commit 931b6c6a6c72038b8994565aff5926d5050fb329.

This was causing our try bots to fail on DEPS changes.

After applying the patch successfully, apply_issue.py tries to run gclient sync to get the updated DEPS, resulting in this error:
src (ERROR)
----------------------------------------
[0:00:00] Started.
----------------------------------------
Error: 
____ src at origin/master
	Your index contains uncommitted changes
	Please commit, stash, or reset.

BUG=413326
TESTED=Manually: https://paste.googleplex.com/6375653263802368

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

[smut, 2014-09-11 19:06:26]

smut@google.com changed reviewers:
+ agable@chromium.org, iannucci@chromium.org, mmoss@google.com

[smut, 2014-09-11 19:06:26]

https://code.google.com/p/chromium/issues/detail?id=413326

[agable, 2014-09-11 19:08:01]

On 2014/09/11 19:06:26, smut wrote:
> https://code.google.com/p/chromium/issues/detail?id=413326

Put reason for revert in CL description?

[iannucci1, 2014-09-11 19:08:04]

On 2014/09/11 19:06:26, smut wrote:
> https://code.google.com/p/chromium/issues/detail?id=413326

Wait what.... why? Context?

[Michael Moss, 2014-09-11 19:11:21]

On 2014/09/11 19:08:04, iannucci1 wrote:
> On 2014/09/11 19:06:26, smut wrote:
> > https://code.google.com/p/chromium/issues/detail?id=413326
> 
> Wait what.... why? Context?

According to the bug, they're getting the error:

Error: 
____ src at origin/master
	Your index contains uncommitted changes
	Please commit, stash, or reset.

But that's actually how gclient is _supposed_ to work (it was a possibly quite
destructive bug that it wasn't catching this). If you actually want to trash
local changes, you should be passing --force.

[iannucci, 2014-09-11 19:13:36]

On 2014/09/11 19:11:21, Michael Moss wrote:
> On 2014/09/11 19:08:04, iannucci1 wrote:
> > On 2014/09/11 19:06:26, smut wrote:
> > > https://code.google.com/p/chromium/issues/detail?id=413326
> > 
> > Wait what.... why? Context?
> 
> According to the bug, they're getting the error:
> 
> Error: 
> ____ src at origin/master
> 	Your index contains uncommitted changes
> 	Please commit, stash, or reset.
> 
> But that's actually how gclient is _supposed_ to work (it was a possibly quite
> destructive bug that it wasn't catching this). If you actually want to trash
> local changes, you should be passing --force.

No, it's not how it's supposed to work. You either have a root repo, which is
unmanaged, or you have a gclient managed repo, which should have all changes
committed.

[smut, 2014-09-11 19:14:17]

On 2014/09/11 19:11:21, Michael Moss wrote:
> On 2014/09/11 19:08:04, iannucci1 wrote:
> > On 2014/09/11 19:06:26, smut wrote:
> > > https://code.google.com/p/chromium/issues/detail?id=413326
> > 
> > Wait what.... why? Context?
> 
> According to the bug, they're getting the error:
> 
> Error: 
> ____ src at origin/master
> 	Your index contains uncommitted changes
> 	Please commit, stash, or reset.
> 
> But that's actually how gclient is _supposed_ to work (it was a possibly quite
> destructive bug that it wasn't catching this). If you actually want to trash
> local changes, you should be passing --force.

Context is in the bug, I'm not sure that this is how this is intended to work. I
just checked how apply_issue fares when reverting the change, and it works-- it
applies the patch, and syncs the DEPS even with a detached head. And when it's
done, the patch is still applied. I've put more information on the bug.

[smut, 2014-09-11 19:16:30]

On 2014/09/11 19:14:17, smut wrote:
> On 2014/09/11 19:11:21, Michael Moss wrote:
> > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > On 2014/09/11 19:06:26, smut wrote:
> > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > 
> > > Wait what.... why? Context?
> > 
> > According to the bug, they're getting the error:
> > 
> > Error: 
> > ____ src at origin/master
> > 	Your index contains uncommitted changes
> > 	Please commit, stash, or reset.
> > 
> > But that's actually how gclient is _supposed_ to work (it was a possibly
quite
> > destructive bug that it wasn't catching this). If you actually want to trash
> > local changes, you should be passing --force.
> 
> Context is in the bug, I'm not sure that this is how this is intended to work.
I
> just checked how apply_issue fares when reverting the change, and it works--
it
> applies the patch, and syncs the DEPS even with a detached head. And when it's
> done, the patch is still applied. I've put more information on the bug.

Scratch that, here's a paste of this working after reverting this change:
https://paste.googleplex.com/6375653263802368.

[Michael Moss, 2014-09-11 19:18:18]

On 2014/09/11 19:13:36, iannucci wrote:
> On 2014/09/11 19:11:21, Michael Moss wrote:
> > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > On 2014/09/11 19:06:26, smut wrote:
> > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > 
> > > Wait what.... why? Context?
> > 
> > According to the bug, they're getting the error:
> > 
> > Error: 
> > ____ src at origin/master
> > 	Your index contains uncommitted changes
> > 	Please commit, stash, or reset.
> > 
> > But that's actually how gclient is _supposed_ to work (it was a possibly
quite
> > destructive bug that it wasn't catching this). If you actually want to trash
> > local changes, you should be passing --force.
> 
> No, it's not how it's supposed to work. You either have a root repo, which is
> unmanaged, or you have a gclient managed repo, which should have all changes
> committed.

Huh? You mean it shouldn't be getting that error because gclient shouldn't even
be looking at 'src'?

[smut, 2014-09-11 19:21:07]

On 2014/09/11 19:18:18, Michael Moss wrote:
> On 2014/09/11 19:13:36, iannucci wrote:
> > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > 
> > > > Wait what.... why? Context?
> > > 
> > > According to the bug, they're getting the error:
> > > 
> > > Error: 
> > > ____ src at origin/master
> > > 	Your index contains uncommitted changes
> > > 	Please commit, stash, or reset.
> > > 
> > > But that's actually how gclient is _supposed_ to work (it was a possibly
> quite
> > > destructive bug that it wasn't catching this). If you actually want to
trash
> > > local changes, you should be passing --force.
> > 
> > No, it's not how it's supposed to work. You either have a root repo, which
is
> > unmanaged, or you have a gclient managed repo, which should have all changes
> > committed.
> 
> Huh? You mean it shouldn't be getting that error because gclient shouldn't
even
> be looking at 'src'?

Added a little context in the description, but follow the BUG and TESTED links
for more information, because we're seeing the effects downstream.

[iannucci, 2014-09-11 19:21:12]

On 2014/09/11 19:18:18, Michael Moss wrote:
> On 2014/09/11 19:13:36, iannucci wrote:
> > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > 
> > > > Wait what.... why? Context?
> > > 
> > > According to the bug, they're getting the error:
> > > 
> > > Error: 
> > > ____ src at origin/master
> > > 	Your index contains uncommitted changes
> > > 	Please commit, stash, or reset.
> > > 
> > > But that's actually how gclient is _supposed_ to work (it was a possibly
> quite
> > > destructive bug that it wasn't catching this). If you actually want to
trash
> > > local changes, you should be passing --force.
> > 
> > No, it's not how it's supposed to work. You either have a root repo, which
is
> > unmanaged, or you have a gclient managed repo, which should have all changes
> > committed.
> 
> Huh? You mean it shouldn't be getting that error because gclient shouldn't
even
> be looking at 'src'?

wait, how does bot_update deal with this?

[iannucci, 2014-09-11 19:22:13]

On 2014/09/11 19:21:12, iannucci wrote:
> On 2014/09/11 19:18:18, Michael Moss wrote:
> > On 2014/09/11 19:13:36, iannucci wrote:
> > > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > > 
> > > > > Wait what.... why? Context?
> > > > 
> > > > According to the bug, they're getting the error:
> > > > 
> > > > Error: 
> > > > ____ src at origin/master
> > > > 	Your index contains uncommitted changes
> > > > 	Please commit, stash, or reset.
> > > > 
> > > > But that's actually how gclient is _supposed_ to work (it was a possibly
> > quite
> > > > destructive bug that it wasn't catching this). If you actually want to
> trash
> > > > local changes, you should be passing --force.
> > > 
> > > No, it's not how it's supposed to work. You either have a root repo, which
> is
> > > unmanaged, or you have a gclient managed repo, which should have all
changes
> > > committed.
> > 
> > Huh? You mean it shouldn't be getting that error because gclient shouldn't
> even
> > be looking at 'src'?
> 
> wait, how does bot_update deal with this?

Or, rather: bot_update is dealing with this correctly everywhere else, why isn't
it doing so here?

[smut, 2014-09-11 19:22:36]

On 2014/09/11 19:21:12, iannucci wrote:
> On 2014/09/11 19:18:18, Michael Moss wrote:
> > On 2014/09/11 19:13:36, iannucci wrote:
> > > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > > 
> > > > > Wait what.... why? Context?
> > > > 
> > > > According to the bug, they're getting the error:
> > > > 
> > > > Error: 
> > > > ____ src at origin/master
> > > > 	Your index contains uncommitted changes
> > > > 	Please commit, stash, or reset.
> > > > 
> > > > But that's actually how gclient is _supposed_ to work (it was a possibly
> > quite
> > > > destructive bug that it wasn't catching this). If you actually want to
> trash
> > > > local changes, you should be passing --force.
> > > 
> > > No, it's not how it's supposed to work. You either have a root repo, which
> is
> > > unmanaged, or you have a gclient managed repo, which should have all
changes
> > > committed.
> > 
> > Huh? You mean it shouldn't be getting that error because gclient shouldn't
> even
> > be looking at 'src'?
> 
> wait, how does bot_update deal with this?

Given that this used to be working for us, and as far as I know no one was
complaining about the cases where it supposedly wasn't working, can we go back
to a working state, and then discuss how this should actually be later?

[smut, 2014-09-11 19:23:19]

On 2014/09/11 19:22:13, iannucci wrote:
> On 2014/09/11 19:21:12, iannucci wrote:
> > On 2014/09/11 19:18:18, Michael Moss wrote:
> > > On 2014/09/11 19:13:36, iannucci wrote:
> > > > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > > > 
> > > > > > Wait what.... why? Context?
> > > > > 
> > > > > According to the bug, they're getting the error:
> > > > > 
> > > > > Error: 
> > > > > ____ src at origin/master
> > > > > 	Your index contains uncommitted changes
> > > > > 	Please commit, stash, or reset.
> > > > > 
> > > > > But that's actually how gclient is _supposed_ to work (it was a
possibly
> > > quite
> > > > > destructive bug that it wasn't catching this). If you actually want to
> > trash
> > > > > local changes, you should be passing --force.
> > > > 
> > > > No, it's not how it's supposed to work. You either have a root repo,
which
> > is
> > > > unmanaged, or you have a gclient managed repo, which should have all
> changes
> > > > committed.
> > > 
> > > Huh? You mean it shouldn't be getting that error because gclient shouldn't
> > even
> > > be looking at 'src'?
> > 
> > wait, how does bot_update deal with this?
> 
> Or, rather: bot_update is dealing with this correctly everywhere else, why
isn't
> it doing so here?

I have no idea how bot_update handles this case, but we are not using it.

[iannucci, 2014-09-11 19:24:51]

On 2014/09/11 19:22:36, smut wrote:
> On 2014/09/11 19:21:12, iannucci wrote:
> > On 2014/09/11 19:18:18, Michael Moss wrote:
> > > On 2014/09/11 19:13:36, iannucci wrote:
> > > > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > > > 
> > > > > > Wait what.... why? Context?
> > > > > 
> > > > > According to the bug, they're getting the error:
> > > > > 
> > > > > Error: 
> > > > > ____ src at origin/master
> > > > > 	Your index contains uncommitted changes
> > > > > 	Please commit, stash, or reset.
> > > > > 
> > > > > But that's actually how gclient is _supposed_ to work (it was a
possibly
> > > quite
> > > > > destructive bug that it wasn't catching this). If you actually want to
> > trash
> > > > > local changes, you should be passing --force.
> > > > 
> > > > No, it's not how it's supposed to work. You either have a root repo,
which
> > is
> > > > unmanaged, or you have a gclient managed repo, which should have all
> changes
> > > > committed.
> > > 
> > > Huh? You mean it shouldn't be getting that error because gclient shouldn't
> > even
> > > be looking at 'src'?
> > 
> > wait, how does bot_update deal with this?
> 
> Given that this used to be working for us, and as far as I know no one was
> complaining about the cases where it supposedly wasn't working, can we go back
> to a working state, and then discuss how this should actually be later?

I don't think it's as clear cut as that... as far as you know you'll just break
everything else...

I'm still not sure why bot_update is failing for you but no one else...

[iannucci, 2014-09-11 19:25:26]

On 2014/09/11 19:23:19, smut wrote:
> On 2014/09/11 19:22:13, iannucci wrote:
> > On 2014/09/11 19:21:12, iannucci wrote:
> > > On 2014/09/11 19:18:18, Michael Moss wrote:
> > > > On 2014/09/11 19:13:36, iannucci wrote:
> > > > > On 2014/09/11 19:11:21, Michael Moss wrote:
> > > > > > On 2014/09/11 19:08:04, iannucci1 wrote:
> > > > > > > On 2014/09/11 19:06:26, smut wrote:
> > > > > > > > https://code.google.com/p/chromium/issues/detail?id=413326
> > > > > > > 
> > > > > > > Wait what.... why? Context?
> > > > > > 
> > > > > > According to the bug, they're getting the error:
> > > > > > 
> > > > > > Error: 
> > > > > > ____ src at origin/master
> > > > > > 	Your index contains uncommitted changes
> > > > > > 	Please commit, stash, or reset.
> > > > > > 
> > > > > > But that's actually how gclient is _supposed_ to work (it was a
> possibly
> > > > quite
> > > > > > destructive bug that it wasn't catching this). If you actually want
to
> > > trash
> > > > > > local changes, you should be passing --force.
> > > > > 
> > > > > No, it's not how it's supposed to work. You either have a root repo,
> which
> > > is
> > > > > unmanaged, or you have a gclient managed repo, which should have all
> > changes
> > > > > committed.
> > > > 
> > > > Huh? You mean it shouldn't be getting that error because gclient
shouldn't
> > > even
> > > > be looking at 'src'?
> > > 
> > > wait, how does bot_update deal with this?
> > 
> > Or, rather: bot_update is dealing with this correctly everywhere else, why
> isn't
> > it doing so here?
> 
> I have no idea how bot_update handles this case, but we are not using it.

Oh. That's the bug.

[Michael Moss, 2014-09-11 19:29:02]

> Scratch that, here's a paste of this working after reverting this change:
> https://paste.googleplex.com/6375653263802368.

Does it also work if you make apply_issue.py run gclient with the '--force'
flag?

[Michael Moss, 2014-09-11 19:29:27]

On 2014/09/11 19:29:02, Michael Moss wrote:
> > Scratch that, here's a paste of this working after reverting this change:
> > https://paste.googleplex.com/6375653263802368.
> 
> Does it also work if you make apply_issue.py run gclient with the '--force'
> flag?

(that is, without reverting this change and using --force instead)

[smut, 2014-09-11 19:40:28]

On 2014/09/11 19:29:27, Michael Moss wrote:
> On 2014/09/11 19:29:02, Michael Moss wrote:
> > > Scratch that, here's a paste of this working after reverting this change:
> > > https://paste.googleplex.com/6375653263802368.
> > 
> > Does it also work if you make apply_issue.py run gclient with the '--force'
> > flag?
> 
> (that is, without reverting this change and using --force instead)

No. gclient revert --force reverts the staged/uncommitted local changes (i.e.
the patch).

[Michael Moss, 2014-09-11 19:47:16]

On 2014/09/11 19:40:28, smut wrote:
> On 2014/09/11 19:29:27, Michael Moss wrote:
> > On 2014/09/11 19:29:02, Michael Moss wrote:
> > > > Scratch that, here's a paste of this working after reverting this
change:
> > > > https://paste.googleplex.com/6375653263802368.
> > > 
> > > Does it also work if you make apply_issue.py run gclient with the
'--force'
> > > flag?
> > 
> > (that is, without reverting this change and using --force instead)
> 
> No. gclient revert --force reverts the staged/uncommitted local changes (i.e.
> the patch).

Ugh, of course :(

So as I see, maybe two core bugs:

1) gclient is apparently looking at and caring about what's going on in 'src' in
unmanaged mode
2) you aren't using bot_update, which somehow manages to make things work
despite 1)

I guess LGTM while I look into #1 (probably yet another ancient gclient bug),
and someone should really fix #2 for these bots. I'm not sure how any bots
managed to escape bot_update conversion in the weeks prior to the git migration.

[smut, 2014-09-11 19:50:55]

On 2014/09/11 19:47:16, Michael Moss wrote:
> On 2014/09/11 19:40:28, smut wrote:
> > On 2014/09/11 19:29:27, Michael Moss wrote:
> > > On 2014/09/11 19:29:02, Michael Moss wrote:
> > > > > Scratch that, here's a paste of this working after reverting this
> change:
> > > > > https://paste.googleplex.com/6375653263802368.
> > > > 
> > > > Does it also work if you make apply_issue.py run gclient with the
> '--force'
> > > > flag?
> > > 
> > > (that is, without reverting this change and using --force instead)
> > 
> > No. gclient revert --force reverts the staged/uncommitted local changes
(i.e.
> > the patch).
> 
> Ugh, of course :(
> 
> So as I see, maybe two core bugs:
> 
> 1) gclient is apparently looking at and caring about what's going on in 'src'
in
> unmanaged mode
> 2) you aren't using bot_update, which somehow manages to make things work
> despite 1)
> 
> I guess LGTM while I look into #1 (probably yet another ancient gclient bug),
> and someone should really fix #2 for these bots. I'm not sure how any bots
> managed to escape bot_update conversion in the weeks prior to the git
migration.

I guess you figured out that I meant to say that gclient SYNC --force reverts
the staged/uncommitted local changes. Thanks for looking into this.

[smut, 2014-09-11 19:50:58]

The CQ bit was checked by smut@google.com

[commit-bot: I haz the power, 2014-09-11 19:52:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/563873002/1

[commit-bot: I haz the power, 2014-09-11 19:54:22]

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

[commit-bot: I haz the power, 2014-09-11 19:54:23]

Presubmit check for 563873002-1 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/gclient_scm.py
    depot_tools/tests/gclient_scm_test.py

Presubmit checks took 136.3s to calculate.

[iannucci, 2014-09-11 19:54:54]

On 2014/09/11 19:54:23, I haz the power (commit-bot) wrote:
> Presubmit check for 563873002-1 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/gclient_scm.py
>     depot_tools/tests/gclient_scm_test.py
> 
> Presubmit checks took 136.3s to calculate.

mmoss' lgtm lgtm

[smut, 2014-09-11 19:55:48]

The CQ bit was checked by smut@google.com

[commit-bot: I haz the power, 2014-09-11 19:56:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/563873002/1

[commit-bot: I haz the power, 2014-09-11 19:57:57]

Committed patchset #1 (id:1) as 291932