Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1329703003:
V8 Buildbot: Auto-tag recipe. (Closed)
|
Created:
5 years, 3 months ago by Michael Achenbach Modified:
5 years, 3 months ago CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org, stip+watch_chromium.org, luqui, iannucci Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionV8 Buildbot: Auto-tag recipe.
Automatically advances an lkgr ref for release branches.
Initially it always advances to HEAD.
BUG=v8:4408
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=296542
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #
Total comments: 1
Patch Set 5 : Tag on head + test #Patch Set 6 : Missing file #
Total comments: 3
Patch Set 7 : Work only with remote #Patch Set 8 : Create on missing #
Total comments: 2
Patch Set 9 : Review #
Total comments: 2
Patch Set 10 : Review #Patch Set 11 : Whitespace #Patch Set 12 : Prints #Patch Set 13 : Moar #Patch Set 14 : Moar #Patch Set 15 : Well, now it's kinda obvious... #
Messages
Total messages: 46 (17 generated)
machenbach@chromium.org changed reviewers: + hablich@chromium.org
PTAL - initial version without sanity checks
Looks straigh-forward https://codereview.chromium.org/1329703003/diff/60001/scripts/slave/recipes/v... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/60001/scripts/slave/recipes/v... scripts/slave/recipes/v8/auto_tag.py:69: local_lkgr_ref = 'refs/remotes/branch-heads/%s-lkgr' % branch I agree '4.5-lkgr' sounds better than 'lkgr-4.5' and makes it easier to navigate.
PTAL - now with heads
machenbach@chromium.org changed reviewers: + tandrii@chromium.org
+ committer
machenbach@chromium.org changed required reviewers: + tandrii@chromium.org
the tag part is bit confusing, IMO, because tags aren't pushed into typical tags space, but rather it's in heads, AFAIU. code-wise, lgtm. https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:47: 'push', repo, '%s:%s' % (local_ref, remote_ref), ah, so this will just move ref, without actually creating commits. Yeah, ok, gnumbd-safe :)
Yea - this is not yet about tags. Later the script will also set tags on our releases. But the lkgr thing is just a ref. https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:47: 'push', repo, '%s:%s' % (local_ref, remote_ref), On 2015/09/02 14:50:52, tandrii(chromium) wrote: > ah, so this will just move ref, without actually creating commits. Yeah, ok, > gnumbd-safe :) Gnumbd won't look at the remote_ref. I think I actually don't need to update the local ref at all. Can't I just directly push the value to the remote with: api.git('push', repo, '%s:%s' % (hsh, remote_ref), ...
PTAL at patch 8. I radically changed a few things. Now, reading and pushing the ref is done only on the remote. The local checkout is right now not needed but I'm keeping it as we want to manipulate also a file in a follow up CL. The lkgr ref (per branch) will be auto-created if it doesn't exist. ls-remote just returns an empty string in this case.
https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:64: new_lkgr = GetRef(api, repo, branch_ref) GetRef is misleading. How about GetCommitForRef?
https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:64: new_lkgr = GetRef(api, repo, branch_ref) On 2015/09/03 10:46:52, Hablich wrote: > GetRef is misleading. How about GetCommitForRef? There you go!
On 2015/09/03 11:16:08, Michael Achenbach wrote: > https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... > File scripts/slave/recipes/v8/auto_tag.py (right): > > https://codereview.chromium.org/1329703003/diff/140001/scripts/slave/recipes/... > scripts/slave/recipes/v8/auto_tag.py:64: new_lkgr = GetRef(api, repo, > branch_ref) > On 2015/09/03 10:46:52, Hablich wrote: > > GetRef is misleading. How about GetCommitForRef? > > There you go! lgtm
lgtm % nit https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/100001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:47: 'push', repo, '%s:%s' % (local_ref, remote_ref), On 2015/09/03 08:52:54, Michael Achenbach wrote: > On 2015/09/02 14:50:52, tandrii(chromium) wrote: > > ah, so this will just move ref, without actually creating commits. Yeah, ok, > > gnumbd-safe :) > > Gnumbd won't look at the remote_ref. I think I actually don't need to update the > local ref at all. Can't I just directly push the value to the remote with: > api.git('push', repo, '%s:%s' % (hsh, remote_ref), ... Acknowledged. https://codereview.chromium.org/1329703003/diff/160001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/160001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:66: # If the lkgr_ref doesn't exists, it's an empty string. In this case the push nit: exist
https://codereview.chromium.org/1329703003/diff/160001/scripts/slave/recipes/... File scripts/slave/recipes/v8/auto_tag.py (right): https://codereview.chromium.org/1329703003/diff/160001/scripts/slave/recipes/... scripts/slave/recipes/v8/auto_tag.py:66: # If the lkgr_ref doesn't exists, it's an empty string. In this case the push On 2015/09/03 11:37:31, tandrii(chromium) wrote: > nit: exist Done.
The CQ bit was checked by machenbach@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hablich@chromium.org, tandrii@chromium.org Link to the patchset: https://codereview.chromium.org/1329703003/#ps170001 (title: "Review")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/170001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/170001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: build_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/build_presubmit...)
The CQ bit was checked by machenbach@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hablich@chromium.org, tandrii@chromium.org Link to the patchset: https://codereview.chromium.org/1329703003/#ps190001 (title: "Whitespace")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/190001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/190001
hm, what does "Exception: Can't start step Uncaught Exception while in step git ls-remote 3.4-lkgr." imply?
On 2015/09/03 12:19:15, tandrii(chromium) wrote: > hm, what does "Exception: Can't start step Uncaught Exception while in step git > ls-remote 3.4-lkgr." imply? No bloody idea - can't repro that locally...
On 2015/09/03 12:19:56, Michael Achenbach wrote: > On 2015/09/03 12:19:15, tandrii(chromium) wrote: > > hm, what does "Exception: Can't start step Uncaught Exception while in step > git > > ls-remote 3.4-lkgr." imply? > > No bloody idea - can't repro that locally... I mean - I have some idea - it wants to raise an exception. Would be nice if the engine could print its trace anyway...
On 2015/09/03 12:20:48, Michael Achenbach wrote: > On 2015/09/03 12:19:56, Michael Achenbach wrote: > > On 2015/09/03 12:19:15, tandrii(chromium) wrote: > > > hm, what does "Exception: Can't start step Uncaught Exception while in step > > git > > > ls-remote 3.4-lkgr." imply? > > > > No bloody idea - can't repro that locally... > > I mean - I have some idea - it wants to raise an exception. Would be nice if the > engine could print its trace anyway... As it seems to persist, I'll have to figure it out...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: build_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/build_presubmit...)
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/210001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/210001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: build_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/build_presubmit...)
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/230001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/230001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: build_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/build_presubmit...)
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/250001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/250001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: build_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/build_presubmit...)
The CQ bit was checked by machenbach@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hablich@chromium.org, tandrii@chromium.org Link to the patchset: https://codereview.chromium.org/1329703003/#ps270001 (title: "Well, now it's kinda obvious...")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1329703003/270001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1329703003/270001
Message was sent while issue was closed.
Committed patchset #15 (id:270001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=296542
Message was sent while issue was closed.
FYI recipe folks. There was a bug in this CL and the recipe engine didn't do a great job in helping finding it. In patch 10, recipe simulation failed on the bot. Surprisingly, I couldn't repro the same error locally. There all tests passed (can't explain why). The error message from the bot and the stack trace could have been more helpful: Exception: Can't start step Uncaught Exception while in step git ls-remote 3.4-lkgr. I added some prints in the engine until my error got really obvious. Both my error and the fix in patch 15 were trivial. Would be nice if the original exception could be printed by the engine in all cases. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
