Repair broken git checkouts caused by aborting git clone.

Repair broken git checkouts caused by aborting git clone.

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

[M-A Ruel, 2013-06-11 19:01:36]

https://codereview.chromium.org/16763004/diff/2001/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/16763004/diff/2001/gclient_scm.py#newcode240
gclient_scm.py:240: def _FetchAndReset(self, revision, file_list, options):
"""Equivalent of git fetch; git reset."""

https://codereview.chromium.org/16763004/diff/2001/gclient_scm.py#newcode364
gclient_scm.py:364: self._Run(['pull', 'origin', 'master'], options)
Why pull when you're about to fetch right after?

https://codereview.chromium.org/16763004/diff/2001/gclient_scm.py#newcode820
gclient_scm.py:820: """Check if git status works in the current repository."""
"""Returns if the directory is a valid git repository.

Checks if git status works.
"""

https://codereview.chromium.org/16763004/diff/2001/gclient_scm.py#newcode828
gclient_scm.py:828: """Check if rev-parse HEAD works in the current
repository."""
"""Returns True if any commit is checked out.

This is done by checking if rev-parse HEAD works in the current repository.
"""

[Mike Stip (use stip instead), 2013-06-11 20:38:54]

updated. ptal!

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py#newc...
gclient_scm.py:240: def _FetchAndReset(self, revision, file_list, options):
On 2013/06/11 19:01:36, Marc-Antoine Ruel wrote:
> """Equivalent of git fetch; git reset."""

Done.

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py#newc...
gclient_scm.py:364: self._Run(['pull', 'origin', 'master'], options)
On 2013/06/11 19:01:36, Marc-Antoine Ruel wrote:
> Why pull when you're about to fetch right after?

Pull sets up the remote tracking branches (including master), while fetch
doesn't.

I don't really care about the second fetch, I care about all the code around it
(reset, capturing changed files, etc).

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py#newc...
gclient_scm.py:820: """Check if git status works in the current repository."""
On 2013/06/11 19:01:36, Marc-Antoine Ruel wrote:
> """Returns if the directory is a valid git repository.
> 
> Checks if git status works.
> """

Done.

[M-A Ruel, 2013-06-11 22:43:50]

There's no test, and in general this is the kind of thing I'd write a test for,
but you're stuck supporting it anyway so it's as you prefer. :)

lgtm

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py#newc...
gclient_scm.py:364: self._Run(['pull', 'origin', 'master'], options)
On 2013/06/11 20:38:54, Mike Stipicevic wrote:
> On 2013/06/11 19:01:36, Marc-Antoine Ruel wrote:
> > Why pull when you're about to fetch right after?
> 
> Pull sets up the remote tracking branches (including master), while fetch
> doesn't.

ORLY? Wow.

[Mike Stip (use stip instead), 2013-06-11 23:35:54]

On 2013/06/11 22:43:50, Marc-Antoine Ruel wrote:
> There's no test, and in general this is the kind of thing I'd write a test
for,
> but you're stuck supporting it anyway so it's as you prefer. :)
> 
> lgtm
> 
> https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py
> File gclient_scm.py (right):
> 
>
https://chromiumcodereview.appspot.com/16763004/diff/2001/gclient_scm.py#newc...
> gclient_scm.py:364: self._Run(['pull', 'origin', 'master'], options)
> On 2013/06/11 20:38:54, Mike Stipicevic wrote:
> > On 2013/06/11 19:01:36, Marc-Antoine Ruel wrote:
> > > Why pull when you're about to fetch right after?
> > 
> > Pull sets up the remote tracking branches (including master), while fetch
> > doesn't.
> 
> ORLY? Wow.

Ah, it *does* set up remote tracking branches, but doesn't check them out for
you and thus doesn't set HEAD if HEAD isn't set:

cd test
git init
git remote add origin http://git.chromium.org/chromium/tools/build.git
git fetch
git rev-parse HEAD   # errors out
git checkout master # already on master, but sets up tracking branch
git rev-parse HEAD  # works

We could add logic to force a checkout after a fetch, but I'm leaving the pull
in as it's a simple and safe operation (that makes the later fetch a no-op).

[commit-bot: I haz the power, 2013-06-11 23:36:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xusydoc@chromium.org/16763004/6001

[commit-bot: I haz the power, 2013-06-11 23:38:13]

Presubmit check for 16763004-6001 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 **
tests/gclient_smoketest.py failed
...................F.F...................
======================================================================
FAIL: testSync (__main__.GClientSmokeGIT)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 858, in testSync
    ['running', 'running', ('running', diffdir), 'deleting'])
  File "tests/gclient_smoketest.py", line 84, in parseGclient
    return self.checkBlock(stdout, items)
  File "tests/gclient_smoketest.py", line 144, in checkBlock
    self.checkString(results[i][0][2], path, (i, results[i][0][2], path))
  File
"/b/commit-queue/workdir/tools/depot_tools/testing_support/fake_repos.py", line
739, in checkString
    self.assertEquals(expected, result, msg)
AssertionError: (0, '/tmp/trialMEQS2N/__main__.GClientSmokeGIT.testSync/src',
'/tmp/trialMEQS2N/__main__.GClientSmokeGIT.testSync')

======================================================================
FAIL: testSyncJobs (__main__.GClientSmokeGIT)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_smoketest.py", line 940, in testSyncJobs
    untangle=True)
  File "tests/gclient_smoketest.py", line 84, in parseGclient
    return self.checkBlock(stdout, items)
  File "tests/gclient_smoketest.py", line 144, in checkBlock
    self.checkString(results[i][0][2], path, (i, results[i][0][2], path))
  File
"/b/commit-queue/workdir/tools/depot_tools/testing_support/fake_repos.py", line
739, in checkString
    self.assertEquals(expected, result, msg)
AssertionError: (0,
'/tmp/trialMEQS2N/__main__.GClientSmokeGIT.testSyncJobs/src',
'/tmp/trialMEQS2N/__main__.GClientSmokeGIT.testSyncJobs')

----------------------------------------------------------------------
Ran 41 tests in 50.999s

FAILED (failures=2)


Presubmit checks took 96.8s to calculate.

[commit-bot: I haz the power, 2013-06-12 00:51:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xusydoc@chromium.org/16763004/13001

[commit-bot: I haz the power, 2013-06-12 00:53:38]

Change committed as 205692

[Isaac (away), 2013-07-16 10:06:20]

https://chromiumcodereview.appspot.com/16763004/diff/13001/gclient_scm.py
File gclient_scm.py (right):

https://chromiumcodereview.appspot.com/16763004/diff/13001/gclient_scm.py#new...
gclient_scm.py:826: self._Capture(['status'])
status is a expensive command on some projects (like blink) -- could we change
this to rev-parse HEAD?