Call status before reset in gclient sync.

Call status before reset in gclient sync.

Calling status before reset will ensure files that have no been changed do not
get deleted by reset --hard.

BUG=642711
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/d74a7b4c14512d95cb46204bb7a70ba5666b5eb7

[justincohen, 2016-10-24 18:35:45]

justincohen@chromium.org changed reviewers:
+ sdefresne@chromium.org

[justincohen, 2016-10-24 18:35:45]

Any suggestions for a better CL description, and/or a comment explaining why
this is necessary?

[sdefresne, 2016-10-25 16:44:56]

lgtm

I think the bug linked give enough context on why this is necessary. You'll need
to add someone from Infra as a reviewer to land this CL though.

[justincohen, 2016-10-25 17:43:40]

justincohen@chromium.org changed reviewers:
+ dpranke@google.com, maruel@chromium.org

[justincohen, 2016-10-25 17:43:40]

Over to maruel@ or dpranke@ for OWNERS, PTAL!

[Dirk Pranke, 2016-10-26 00:07:04]

On 2016/10/25 17:43:40, justincohen wrote:
> Over to maruel@ or dpranke@ for OWNERS, PTAL!

Nope, just linking to the bug is not good enough for me :)

It's totally non-obvious why calling `git status` is needed or would have any
effect at all.

So, I'd add a comment along the lines of:

"Builds can create hard links which update source files' ctimes, causing git to
become
confused over what files are out-of-date. Calling `git status` resynchronizes
git and
allows `git reset --hard` to not re-checkout files (and thus forcing unnecessary
rebuilds)".

That is a slightly more verbose version of what the CL description says, so I
think with
the comment the description is fine.

I'm guessing you can't just call `git update-index` here? If you could call that
instead
of `git status`, that would probably be better because it's a little clearer
what you're
trying to do (the fact that `git status` is updating the index is almost a
hidden side effect).

LGTM with the comments addressed.

[justincohen, 2016-10-26 00:42:12]

> I'm guessing you can't just call `git update-index` here? If you could call
that instead
> of `git status`, that would probably be better because it's a little clearer
what you're
> trying to do (the fact that `git status` is updating the index is almost a
hidden side effect).

Correct, calling update-index is not enough, unfortunately.

Added comment

[justincohen, 2016-10-26 00:43:02]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2016-10-26 00:43:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2447813003/#ps20001
(title: "Add comment")

[commit-bot: I haz the power, 2016-10-26 00:43:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 00:46:07]

Description was changed from

==========
Call status before reset in gclient sync.

Calling status before reset will ensure files that have no been changed do not
get deleted by reset --hard.

BUG=642711
==========

to

==========
Call status before reset in gclient sync.

Calling status before reset will ensure files that have no been changed do not
get deleted by reset --hard.

BUG=642711

Committed:
https://chromium.googlesource.com/chromium/tools/depot_tools/+/d74a7b4c14512d...
==========

[commit-bot: I haz the power, 2016-10-26 00:46:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/depot_tools/+/d74a7b4c14512d...

[sdefresne, 2016-10-26 14:25:09]

On 2016/10/26 00:46:08, commit-bot: I haz the power wrote:
> Committed patchset #2 (id:20001) as
>
https://chromium.googlesource.com/chromium/tools/depot_tools/+/d74a7b4c14512d...

Looking at the logs on the bots, we do not see the "git status" invocation, so
we may have put the invocation in the wrong spot (which would also explain why
the "git update-index" was apparently not working). Given that I would recommend
reverting this change and instead we should add an additional step on the bot to
explicitly invoke "git update-index". Justin, WDYT?

[justincohen, 2016-10-26 14:26:18]

When we tested update-index manually it did not work, but status did.  However,
it looks like we need to call status in two different places, both in gclient
revert and gclient sync.

I'm fine reverting this change if we have a better fix elsewhere -- but I'm not
convinced update-index will work either.

[sdefresne, 2016-10-26 15:22:54]

On 2016/10/26 14:26:18, justincohen wrote:
> When we tested update-index manually it did not work, but status did. 
However,
> it looks like we need to call status in two different places, both in gclient
> revert and gclient sync.
> 
> I'm fine reverting this change if we have a better fix elsewhere -- but I'm
not
> convinced update-index will work either.

I've tested it a bit more and it looks like "git update-index --refresh" does
the trick. See my CL https://codereview.chromium.org/2444403003/.

[justincohen, 2016-10-26 20:38:48]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2453083002/ by justincohen@chromium.org.

The reason for reverting is: https://codereview.chromium.org/2444403003/ makes
this pretty redundant.  Reverting this..