Fix git branch parsing.

Fix git branch parsing.

In git version 2.4 the git branch command prints "* (HEAD detached at"
or "* (HEAD detached from" instead of "* (detached from". Adjust the parsing to make our tests
still work with git 2.4.

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

[Adrian Kuegel, 2015-06-08 13:39:24]

Patchset #1 (id:1) has been deleted

[Adrian Kuegel, 2015-06-08 13:44:10]

Patchset #1 (id:20001) has been deleted

[Adrian Kuegel, 2015-06-08 13:46:23]

akuegel@chromium.org changed reviewers:
+ iannucci@chromium.org

[Adrian Kuegel, 2015-06-08 13:46:24]

Robbie, can you please review this CL? I did the change you suggested, and it
seems to work fine (I tested it on the CQ host which has git 2.4).

[iannucci, 2015-06-08 16:01:49]

Try just "* (detached"?

On Mon, Jun 8, 2015, 06:46  <akuegel@chromium.org> wrote:

> Reviewers: iannucci,
>
> Message:
> Robbie, can you please review this CL? I did the change you suggested, and
> it
> seems to work fine (I tested it on the CQ host which has git 2.4).
>
> Description:
> Fix git branch parsing.
>
> In git version 2.4 the git branch command may print "detached at"
> instead of "detached from". Adjust the parsing to make our tests
> still work with git 2.4.
>
> BUG=487172
>
> Please review this at https://codereview.chromium.org/1162763003/
>
> Base URL: svn://svn.chromium.org/chrome/trunk/tools/depot_tools
>
> Affected files (+3, -9 lines):
>    M git_common.py
>    M tests/git_common_test.py
>    M tests/git_rebase_update_test.py
>
>
> Index: git_common.py
> diff --git a/git_common.py b/git_common.py
> index
>
>
6b21050b22b87575ca192d55ddb117fcdb3710c6..73d4f387114f60ec3189c22ed3ea08c0bea48d39
> 100644
> --- a/git_common.py
> +++ b/git_common.py
> @@ -293,7 +293,7 @@ def branch_config_map(option):
>
>
>   def branches(*args):
> -  NO_BRANCH = ('* (no branch', '* (detached from ')
> +  NO_BRANCH = ('* (no branch', '* (detached from ', '* (detached at ')
>
>     key = 'depot-tools.branch-limit'
>     limit = 20
> Index: tests/git_common_test.py
> diff --git a/tests/git_common_test.py b/tests/git_common_test.py
> index
>
>
5ba00b25791c52246f78fdf0751ffcf2c3c1c1d7..5c6febe0d02b897adbd06e2b898bc17587225661
> 100755
> --- a/tests/git_common_test.py
> +++ b/tests/git_common_test.py
> @@ -230,7 +230,6 @@ class
> GitReadOnlyFunctionsTest(git_test_utils.GitRepoReadOnlyTestBase,
>       self.repo.git('checkout', 'branch_D')
>       self.assertEqual(self.repo.run(self.gc.current_branch), 'branch_D')
>
> -  @unittest.skip('broken by git 2.4')
>     def testBranches(self):
>       # This check fails with git 2.4 (see crbug.com/487172)
>       self.assertEqual(self.repo.run(set, self.gc.branches()),
> @@ -448,7 +447,6 @@ class
> GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
>       self.repo.git('branch', '--set-upstream-to', 'root_A', 'branch_G')
>       self.repo.git('branch', '--set-upstream-to', 'root_X', 'root_A')
>
> -  @unittest.skip('broken by git 2.4')
>     def testTooManyBranches(self):
>       for i in xrange(30):
>         self.repo.git('branch', 'a'*i)
> @@ -535,7 +533,6 @@ class
> GitMutableStructuredTest(git_test_utils.GitRepoReadWriteTestBase,
>       self.assertIsNone(
>         self.repo.run(self.gc.get_or_create_merge_base, 'branch_DOG'))
>
> -  @unittest.skip('broken by git 2.4')
>     def testGetBranchTree(self):
>       skipped, tree = self.repo.run(self.gc.get_branch_tree)
>       # This check fails with git 2.4 (see crbug.com/487172)
> @@ -731,6 +728,4 @@ class
> GitFreezeThaw(git_test_utils.GitRepoReadWriteTestBase):
>
>   if __name__ == '__main__':
>     sys.exit(coverage_utils.covered_main(
> -    os.path.join(DEPOT_TOOLS_ROOT, 'git_common.py'),
> -    required_percentage=88.0
> -  ))
> +    os.path.join(DEPOT_TOOLS_ROOT, 'git_common.py')))
> Index: tests/git_rebase_update_test.py
> diff --git a/tests/git_rebase_update_test.py
> b/tests/git_rebase_update_test.py
> index
>
>
f374948c6463f64453b8f94166b9a105da04a89a..933ef352ab9f4c1a6f31f8e31408d21995aed14b
> 100755
> --- a/tests/git_rebase_update_test.py
> +++ b/tests/git_rebase_update_test.py
> @@ -69,7 +69,6 @@ class
> GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase):
>       self.origin.nuke()
>       super(GitRebaseUpdateTest, self).tearDown()
>
> -  @unittest.skip('broken by git 2.4')
>     def testRebaseUpdate(self):
>       self.repo.git('checkout', 'branch_K')
>
> @@ -342,4 +341,4 @@ if __name__ == '__main__':
>       os.path.join(DEPOT_TOOLS_ROOT, 'git_new_branch.py'),
>       os.path.join(DEPOT_TOOLS_ROOT, 'git_reparent_branch.py'),
>       os.path.join(DEPOT_TOOLS_ROOT, 'git_rename_branch.py')
> -  ), required_percentage=85.0))
> +  )))
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Adrian Kuegel, 2015-06-08 16:05:20]

That also works, of course. It just makes it less obvious that both cases can
occur, but I guess we don't care.

PTAL

[iannucci, 2015-06-08 16:09:36]

Lgtm thanks :)

[Adrian Kuegel, 2015-06-08 16:10:04]

The CQ bit was checked by akuegel@chromium.org

[commit-bot: I haz the power, 2015-06-08 16:10:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1162763003/60001

[commit-bot: I haz the power, 2015-06-08 16:16:15]

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

[commit-bot: I haz the power, 2015-06-08 16:16:17]

Try jobs failed on following builders:
  depot_tools_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/depot_tools_pre...)

[Adrian Kuegel, 2015-06-08 16:16:37]

Hmm, I didn't realize the presubmit tests running on upload don't include those
tests. So apparently the simplification doesn't work (for some reason).

[Adrian Kuegel, 2015-06-08 16:21:08]

On 2015/06/08 16:16:37, Adrian Kuegel wrote:
> Hmm, I didn't realize the presubmit tests running on upload don't include
those
> tests. So apparently the simplification doesn't work (for some reason).

And actually it is running fine locally. Not sure why it doesn't work on the
bot.

[iannucci, 2015-06-08 16:26:45]

Oh weird :(

On Mon, Jun 8, 2015, 09:21  <akuegel@chromium.org> wrote:

> On 2015/06/08 16:16:37, Adrian Kuegel wrote:
> > Hmm, I didn't realize the presubmit tests running on upload don't include
> those
> > tests. So apparently the simplification doesn't work (for some reason).
>
> And actually it is running fine locally. Not sure why it doesn't work on
> the
> bot.
>
> https://codereview.chromium.org/1162763003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Adrian Kuegel, 2015-06-08 16:30:55]

On 2015/06/08 16:26:45, iannucci wrote:
> Oh weird :(
> 
> On Mon, Jun 8, 2015, 09:21  <mailto:akuegel@chromium.org> wrote:
> 
> > On 2015/06/08 16:16:37, Adrian Kuegel wrote:
> > > Hmm, I didn't realize the presubmit tests running on upload don't include
> > those
> > > tests. So apparently the simplification doesn't work (for some reason).
> >
> > And actually it is running fine locally. Not sure why it doesn't work on
> > the
> > bot.
> >
> > https://codereview.chromium.org/1162763003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Ok, the other thing also doesn't work. I thought I tested it on the CQ host, but
maybe I did something wrong when testing it and it didn't in fact run this test?
Need to investigate this tomorrow.
Anyway, it turns out the trybot now also has git 2.4, so I can also just trigger
presubmit tryjobs with my patchsets :)

[iannucci, 2015-06-08 16:36:25]

:)

Maybe add a bunch of debug logs in one patch?

On Mon, Jun 8, 2015, 09:30  <akuegel@chromium.org> wrote:

> On 2015/06/08 16:26:45, iannucci wrote:
> > Oh weird :(
>
> > On Mon, Jun 8, 2015, 09:21  <mailto:akuegel@chromium.org> wrote:
>
> > > On 2015/06/08 16:16:37, Adrian Kuegel wrote:
> > > > Hmm, I didn't realize the presubmit tests running on upload don't
> > include
> > > those
> > > > tests. So apparently the simplification doesn't work (for some
> > reason).
> > >
> > > And actually it is running fine locally. Not sure why it doesn't work
> on
> > > the
> > > bot.
> > >
> > > https://codereview.chromium.org/1162763003/
> > >
>
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> Ok, the other thing also doesn't work. I thought I tested it on the CQ
> host, but
> maybe I did something wrong when testing it and it didn't in fact run this
> test?
> Need to investigate this tomorrow.
> Anyway, it turns out the trybot now also has git 2.4, so I can also just
> trigger
> presubmit tryjobs with my patchsets :)
>
> https://codereview.chromium.org/1162763003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Adrian Kuegel, 2015-06-09 15:17:48]

Patchset #4 (id:100001) has been deleted

[Adrian Kuegel, 2015-06-09 15:23:17]

And it works now. Apparently in addition to printing "detached at", git 2.4 also
adds "HEAD " in front.

[Adrian Kuegel, 2015-06-09 15:23:24]

The CQ bit was checked by akuegel@chromium.org

[Adrian Kuegel, 2015-06-09 15:23:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/1162763003/#ps120001
(title: "Fix it for real.")

[commit-bot: I haz the power, 2015-06-09 15:23:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1162763003/120001

[commit-bot: I haz the power, 2015-06-09 15:26:41]

Committed patchset #4 (id:120001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=295578