Bug 27918 – commit-queue mode for bugzilla-tool

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Adam Barth]

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.

[Adam Barth]

Created attachment 33932 [details]
commit-queue script

Here's the script i'm using.  Could probably use some iteration.

[Adam Barth]

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...

[Adam Barth]

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.

[Adam Barth]

Created attachment 34032 [details]
Work in progress

[Adam Barth]

Comment on attachment 34032 [details]
Work in progress

Worked at least for a simple test case.

[Eric Seidel (no email)]

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.)

[Adam Barth]

Created attachment 34036 [details]
Patch v1

[Adam Barth]

Comment on attachment 34036 [details]
Patch v1

Oops.  Didn't mean to obsolete this patch.

[Eric Seidel (no email)]

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!

[Adam Barth]

Committed revision 46751.