Add support for landing to pending ref.

Teach 'git cl land' to land to pending ref.

R=iannucci@chromium.org
BUG=404214
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291302

[Vadim Sh., 2014-08-21 19:31:23]

WIP, haven't tested (or even ran).

Is that what you had in mind?

https://codereview.chromium.org/496073002/diff/1/git_cl.py
File git_cl.py (left):

https://codereview.chromium.org/496073002/diff/1/git_cl.py#oldcode1907
git_cl.py:1907: if cmd == 'dcommit':
Now checks with 'git cl land' too.

https://codereview.chromium.org/496073002/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode1893
git_cl.py:1893: svn_head = RunGit(['log', '--grep=^git-svn-id:', '-1',
This stuff finds svn HEADs in pure git repos... For example in infra/infra
(because we have a chunk of history imported from git-svn repo). I decided it's
safer to have svn_head to be no None only when it's used in the code.

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode1933
git_cl.py:1933: return 1
Now aborts if tree status is unknown, as error message implied.

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2108
git_cl.py:2108: code, out = RunGitWithCode(['cherry-pick', cherry])
cherry-pick supports different merge strategies. Any ideas what's the best one
for this case?

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2110
git_cl.py:2110: RunGitWithCode(['cherry-pick', '--abort'])
Not sure it's needed...

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2114
git_cl.py:2114: code, out = RunGitWithCode(['push', remote, 'HEAD:%s' %
pending_ref])
This can fail in 3 cases:
1. Network\git flake.
2. Non FF push.
3. ACLs.

(1) and (2) should be retried, but not (3). Unfortunately, detecting (3) is not
easy (git push logs all sorts of messages in case ACL\account is misconfigured).
Hardcode these strings here? Or screw it and retry ACL errors too?

[iannucci, 2014-08-21 20:32:20]

lgtm, let's try it on the sandbox

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode460
git_cl.py:460: self.pending_ref_prefix = self.pending_ref_prefix.rstrip('/') +
'/'
why not just take it as-is and then commit the right value?

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode1865
git_cl.py:1865: if not args or cmd == 'land':
good catch

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode1892
git_cl.py:1892: if cmd == 'dcommit' or base_has_submodules:
dcommit?

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode2110
git_cl.py:2110: RunGitWithCode(['cherry-pick', '--abort'])
On 2014/08/21 19:31:22, Vadim Sh. wrote:
> Not sure it's needed...

yep

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode2114
git_cl.py:2114: code, out = RunGitWithCode(['push', remote, 'HEAD:%s' %
pending_ref])
On 2014/08/21 19:31:22, Vadim Sh. wrote:
> This can fail in 3 cases:
> 1. Network\git flake.
> 2. Non FF push.
> 3. ACLs.
> 
> (1) and (2) should be retried, but not (3). Unfortunately, detecting (3) is
not
> easy (git push logs all sorts of messages in case ACL\account is
misconfigured).
> Hardcode these strings here? Or screw it and retry ACL errors too?

I think we should just put 'retry' before 'push'

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode2115
git_cl.py:2115: if not code:
let's do `code == 0`

[iannucci, 2014-08-21 20:37:04]

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/496073002/diff/1/git_cl.py#newcode2107
git_cl.py:2107: RunGitWithCode(['checkout', 'refs/git-cl/pending'])
let's make this

refs/git-cl/pending/heads/master
refs/git-cl/pending/branch-heads/12345

etc.

[iannucci, 2014-08-21 20:39:04]

You'll want to do:

fetchspec = '+refs/pending/heads/master:refs/git-cl/pending/heads/master'
if fetchspec not in`git config --get-all remote.origin.fetch`.splitlines():
  `git config --add remote.origin.fetch {fetchspec}`

[Vadim Sh., 2014-08-21 21:14:35]

will add fetchspec in next CL

bare minimum for now

https://codereview.chromium.org/496073002/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode460
git_cl.py:460: self.pending_ref_prefix = self.pending_ref_prefix.rstrip('/') +
'/'
On 2014/08/21 20:32:20, iannucci wrote:
> why not just take it as-is and then commit the right value?

Okay... just defensive programming.

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2107
git_cl.py:2107: RunGitWithCode(['checkout', 'refs/git-cl/pending'])
On 2014/08/21 20:37:03, iannucci wrote:
> let's make this
> 
> refs/git-cl/pending/heads/master
> refs/git-cl/pending/branch-heads/12345
> 
> etc.

Done.

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2114
git_cl.py:2114: code, out = RunGitWithCode(['push', remote, 'HEAD:%s' %
pending_ref])
On 2014/08/21 20:32:20, iannucci wrote:
> On 2014/08/21 19:31:22, Vadim Sh. wrote:
> > This can fail in 3 cases:
> > 1. Network\git flake.
> > 2. Non FF push.
> > 3. ACLs.
> > 
> > (1) and (2) should be retried, but not (3). Unfortunately, detecting (3) is
> not
> > easy (git push logs all sorts of messages in case ACL\account is
> misconfigured).
> > Hardcode these strings here? Or screw it and retry ACL errors too?
> 
> I think we should just put 'retry' before 'push'

Done.

https://codereview.chromium.org/496073002/diff/1/git_cl.py#newcode2115
git_cl.py:2115: if not code:
On 2014/08/21 20:32:20, iannucci wrote:
> let's do `code == 0`

Done.

[Vadim Sh., 2014-08-22 01:04:24]

Verified it works, added nicer messages. Committing.

[Vadim Sh., 2014-08-22 01:04:32]

The CQ bit was checked by vadimsh@chromium.org

[commit-bot: I haz the power, 2014-08-22 01:05:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vadimsh@chromium.org/496073002/30001

[Vadim Sh., 2014-08-22 01:22:30]

The CQ bit was unchecked by vadimsh@chromium.org

[Vadim Sh., 2014-08-22 01:34:19]

Committed patchset #3 manually as 291302.

[commit-bot: I haz the power, 2014-08-29 19:32:18]

Presubmit check for 496073002-30001 failed and returned exit status -2001.
The presubmit check was hung. It took 671204.6 seconds to execute and the time
limit is 900.0 seconds.

Running presubmit commit checks ...

[Vadim Sh., 2014-09-02 18:27:48]

On 2014/08/29 19:32:18, I haz the power (commit-bot) wrote:
> Presubmit check for 496073002-30001 failed and returned exit status -2001.
> The presubmit check was hung. It took 671204.6 seconds to execute and the time
> limit is 900.0 seconds.
> 
> Running presubmit commit checks ...

wat?