Bug 27918 - commit-queue mode for bugzilla-tool
Summary: commit-queue mode for bugzilla-tool
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 27970
  Show dependency treegraph
 
Reported: 2009-07-31 23:00 PDT by Adam Barth
Modified: 2009-08-04 01:19 PDT (History)
2 users (show)

See Also:


Attachments
commit-queue script (192 bytes, text/plain)
2009-07-31 23:01 PDT, Adam Barth
no flags Details
Work in progress (6.81 KB, patch)
2009-08-03 18:40 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch v1 (7.53 KB, patch)
2009-08-03 22:56 PDT, Adam Barth
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2009-07-31 23:00:19 PDT
We should be able to run bugzilla-tool in a commit-queue mode that recovers from errors gracefully.  I have a wrapper script that knows how to restart bugzilla-tool when it errors out.
Comment 1 Adam Barth 2009-07-31 23:01:17 PDT
Created attachment 33932 [details]
commit-queue script

Here's the script i'm using.  Could probably use some iteration.
Comment 2 Adam Barth 2009-07-31 23:22:31 PDT
Sometimes patch requires user interaction.  There doesn't to be an obvious flag that does what we want.  --batch is close, but it will assume the patch is reversed if it appears that way, which is kind of like an automated rollout...
Comment 3 Adam Barth 2009-08-01 00:47:11 PDT
Also, failing tests pop up safari, which needs to be closed.  The build bots must have a flag that suppresses this behavior.  More investigation needed.
Comment 4 Adam Barth 2009-08-03 18:40:09 PDT
Created attachment 34032 [details]
Work in progress
Comment 5 Adam Barth 2009-08-03 19:23:00 PDT
Comment on attachment 34032 [details]
Work in progress

Worked at least for a simple test case.
Comment 6 Eric Seidel (no email) 2009-08-03 19:40:29 PDT
Comment on attachment 34032 [details]
Work in progress

We could key this choice off of an option some day:
6         for bug_id in bug_ids:
147              print "%s" % tool.bugs.bug_url_for_bug_id(bug_id)
 147             print "%s" % bug_id

Danger Danger will robinson!
 366         options.force_clean = True

I don't think we should --force-clean automagically.

Seems we should check the value of ['commit-queue'] here:
 239             if 'commit-queue' in attachment and not attachment['is_obsolete']:

attachment.get('commit-queue', False) might do the trick.  I think ['commit-queue'] might throw an exception.

Otherwise looks good.  We need to fix the "destroying your current work state" problem by forcing --force-clean.  We need to require something to be passed before we go destroying data.

You should be aware of the proposed refactoring in:
https://bugs.webkit.org/show_bug.cgi?id=26715

(I really should split out the refactoring bits into a separate patch and get it landed.)
Comment 7 Adam Barth 2009-08-03 22:56:11 PDT
Created attachment 34036 [details]
Patch v1
Comment 8 Adam Barth 2009-08-04 00:04:53 PDT
Comment on attachment 34036 [details]
Patch v1

Oops.  Didn't mean to obsolete this patch.
Comment 9 Eric Seidel (no email) 2009-08-04 00:30:45 PDT
Comment on attachment 34036 [details]
Patch v1

Seems you should combine this with teh one I just reviewed.  No sense in adding the command only to remove it again.

 217                 attachment['commit-queue'] = True # FIXME: Validate that the flag was set by a committer.

validation is possible as soon as bug 27972 is reviewed.

Otherwise this looks great!
Comment 10 Adam Barth 2009-08-04 01:19:34 PDT
Committed revision 46751.