Add tag write access to merge script.

Add tag write access to merge script.

The script will poll the remote branch until the git updater
has replicated the commit to tag. The tag will then be
directly pushed to git.

BUG=chromium:410721
LOG=n
TEST=script_test.py
R=agable@chromium.org, tandrii@chromium.org
TBR=tandrii@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=24431

[Michael Achenbach, 2014-09-26 14:10:02]

machenbach@chromium.org changed reviewers:
+ agable@chromium.org, tandrii@chromium.org

[Michael Achenbach, 2014-09-26 14:11:52]

PTAL

https://codereview.chromium.org/607893004/diff/1/tools/push-to-trunk/test_scr...
File tools/push-to-trunk/test_scripts.py (right):

https://codereview.chromium.org/607893004/diff/1/tools/push-to-trunk/test_scr...
tools/push-to-trunk/test_scripts.py:1276: Cmd("git push origin 3.22.5.1", ""),
Pushing the tag might take over the git updater from the dcommit above by a few
seconds. Is that a problem?

[Michael Achenbach, 2014-09-26 14:51:51]

ok - I'll work on a busy waiting solution...

[Michael Achenbach, 2014-09-29 08:49:12]

PTAL - now with busy fetch waiting.

[agable, 2014-09-29 09:49:12]

basically lgtm

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
File tools/push-to-trunk/common_includes.py (right):

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:393: hsh += "^"
Cute. Might be better to actually query Git for the parent of the commit and
directly reference that instead. Clearer logging too.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:401: commit = self.step.GitLog(n=1,
format="%H", grep=title, branch=branch)
This n=1 is dangerous -- what if two commits get replicated in rapid succession?

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:404: print("The commit is not replicated
on git. Waiting for %s seconds." %
nit: "...has not replicated to..."

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:410: "git updater is lacking behind?")
nit: indentation

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:410: "git updater is lacking behind?")
nit: "lagging"

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:412: self.step.Git("tag %s %s" % (tag,
commit))
The fact that this takes args as a single string instead of a list of tokens
makes me very sad.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
File tools/push-to-trunk/git_recipes.py (right):

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
tools/push-to-trunk/git_recipes.py:105: def GitRemoteContains(self, hsh,
**kwargs):
I'd be scared that someone might use this who doesn't fully understand it -- it
only looks at local remote-tracking branches, not actually at what is in the
remote server. So it's useless without a fetch first.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
tools/push-to-trunk/git_recipes.py:108: Returns an empty string in case the
commit doesn't exist on any remote
None is a better sentinel value than "".

[Michael Achenbach, 2014-09-29 10:46:25]

Done. PTAL

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
File tools/push-to-trunk/common_includes.py (right):

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:393: hsh += "^"
On 2014/09/29 09:49:11, agable wrote:
> Cute. Might be better to actually query Git for the parent of the commit and
> directly reference that instead. Clearer logging too.

Done. Changed the semantics slightly as HEAD always refers to a local commit
only.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:401: commit = self.step.GitLog(n=1,
format="%H", grep=title, branch=branch)
On 2014/09/29 09:49:11, agable wrote:
> This n=1 is dangerous -- what if two commits get replicated in rapid
succession?

But shouldn't the grep on the unique title take care of this? Is there a better
command then git log for achieving the same thing?

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:404: print("The commit is not replicated
on git. Waiting for %s seconds." %
On 2014/09/29 09:49:11, agable wrote:
> nit: "...has not replicated to..."

Done.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:410: "git updater is lacking behind?")
On 2014/09/29 09:49:11, agable wrote:
> nit: "lagging"

Done.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:410: "git updater is lacking behind?")
On 2014/09/29 09:49:11, agable wrote:
> nit: indentation

Done.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/comm...
tools/push-to-trunk/common_includes.py:412: self.step.Git("tag %s %s" % (tag,
commit))
On 2014/09/29 09:49:11, agable wrote:
> The fact that this takes args as a single string instead of a list of tokens
> makes me very sad.

I'm sad too. But I'll address this in a separate refactoring CL.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
File tools/push-to-trunk/git_recipes.py (right):

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
tools/push-to-trunk/git_recipes.py:105: def GitRemoteContains(self, hsh,
**kwargs):
On 2014/09/29 09:49:12, agable wrote:
> I'd be scared that someone might use this who doesn't fully understand it --
it
> only looks at local remote-tracking branches, not actually at what is in the
> remote server. So it's useless without a fetch first.

I'll rename the function as it should be just used to determine the remote
branch's name, not for actually verifying a commit on a remote server.

https://codereview.chromium.org/607893004/diff/40001/tools/push-to-trunk/git_...
tools/push-to-trunk/git_recipes.py:108: Returns an empty string in case the
commit doesn't exist on any remote
On 2014/09/29 09:49:11, agable wrote:
> None is a better sentinel value than "".

Done. Was because of the @Strip wrapper. But I make that more robust.

[Michael Achenbach, 2014-09-30 13:28:07]

PTAL at patch 6. This reuses the remote branch info that we know from earlier in
the script. So, no need to add logic for retrieving this. The svn tag workflow
also needed this.

[tandrii(chromium), 2014-10-02 17:36:06]

lgtm

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
File tools/push-to-trunk/common_includes.py (right):

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:393: for wait_interval in [3, 5, 10, 30]:
not sure 48 seconds is enough, you probably know better anyways.

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:396: if commit:
Naturally, i'm not sure about assumption above the loop. So, I'd check the title
for equality here anyways.

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:402: if not commit:
or just

else:

[Michael Achenbach, 2014-10-07 10:10:50]

Done

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
File tools/push-to-trunk/common_includes.py (right):

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:393: for wait_interval in [3, 5, 10, 30]:
On 2014/10/02 17:36:06, tandrii wrote:
> not sure 48 seconds is enough, you probably know better anyways.

Bumped it a little.

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:396: if commit:
On 2014/10/02 17:36:06, tandrii wrote:
> Naturally, i'm not sure about assumption above the loop. So, I'd check the
title
> for equality here anyways.

I don't need the assumption if I add the commit message/title to the api (which
I did now).

https://codereview.chromium.org/607893004/diff/100001/tools/push-to-trunk/com...
tools/push-to-trunk/common_includes.py:402: if not commit:
On 2014/10/02 17:36:06, tandrii wrote:
> or just
> 
> else:

Odd construct - but I love shorter code :)

[Michael Achenbach, 2014-10-07 10:46:17]

Committed patchset #7 (id:120001) manually as 24431 (presubmit successful).