Bug 30634

Summary: commit-queue will get stuck on patches if land-patches terminates unexpectedly
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: Tools / TestsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on: 30683    
Bug Blocks: 30084    
Attachments:
Description Flags
post unknown errors to the bug instead of spinning
none
post unknown errors to the bug instead of spinning abarth: review+, commit-queue: commit-queue-

Description Eric Seidel (no email) 2009-10-21 10:48:26 PDT
commit-queue will get stuck on patches if land-patches terminates unexpectedly

If "bugzilla-tool land-patches" exit's early, we just spin.  This is bad.  We really should record that failure on the bug and mark the patch cq- if land-patches exits unexpectedly.
Comment 1 Eric Seidel (no email) 2009-10-21 11:22:48 PDT
Created attachment 41586 [details]
post unknown errors to the bug instead of spinning
Comment 2 Eric Seidel (no email) 2009-10-21 11:40:48 PDT
Created attachment 41593 [details]
post unknown errors to the bug instead of spinning
Comment 3 Eric Seidel (no email) 2009-10-21 11:48:30 PDT
bug 30084's patch is based on the same branch as this one, so currently this one needs to be landed first.  the commit-queue doesn't know how to deal with dependencies, so I guess I'll remove the cq? from the other one.
Comment 4 Adam Barth 2009-10-21 23:04:32 PDT
Comment on attachment 41593 [details]
post unknown errors to the bug instead of spinning

I don't really like the exit code 2 business, but I don't see a better design.
Comment 5 Eric Seidel (no email) 2009-10-21 23:50:56 PDT
Agreed.  It gets even more skanky with the whole re-exec nonsense.  But this seemed the cleanest way to indicate to the calling process that it should keep going.  I could have it exit(0) in --commit-queue mode instead, but that seemed more confusing.
Comment 6 Eric Seidel (no email) 2009-10-21 23:51:21 PDT
Comment on attachment 41593 [details]
post unknown errors to the bug instead of spinning

I'm certainly open to other opinions.
Comment 7 WebKit Commit Bot 2009-10-22 00:03:25 PDT
Comment on attachment 41593 [details]
post unknown errors to the bug instead of spinning

Rejecting patch 41593 from commit-queue.

Failed to run "['git', 'svn', 'dcommit']" exit_code: 1
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/bugzilla-tool
A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output:
svnlook: Can't write to stream: Broken pipe

    The following ChangeLog files contain OOPS:

        trunk/WebKitTools/ChangeLog

    Please don't ever say "OOPS" in a ChangeLog file.
 at /usr/local/libexec/git-core//git-svn line 469
Comment 8 Eric Seidel (no email) 2009-10-22 00:04:39 PDT
Sigh.  Clearly I need to build a smarter commit queue.
Comment 9 Eric Seidel (no email) 2009-10-22 10:57:30 PDT
Going to see if I can't fix svn-apply (And thus the commit-queue) first.
Comment 10 Eric Seidel (no email) 2009-10-26 15:31:13 PDT
I've fixed svn-apply, but it is faster to just land this by hand.
Comment 11 Eric Seidel (no email) 2009-10-26 15:32:10 PDT
Committed r50104: <http://trac.webkit.org/changeset/50104>