Bug 30634 - commit-queue will get stuck on patches if land-patches terminates unexpectedly
Summary: commit-queue will get stuck on patches if land-patches terminates unexpectedly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on: 30683
Blocks: 30084
  Show dependency tree / graph
 
Reported: 2009-10-21 10:48 PDT by Eric Seidel (no email)
Modified: 2009-10-26 15:32 PDT (History)
2 users (show)

See Also:

Attachments
post unknown errors to the bug instead of spinning (3.92 KB, patch)
2009-10-21 11:22 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
post unknown errors to the bug instead of spinning (3.94 KB, patch)
2009-10-21 11:40 PDT, Eric Seidel (no email) 		abarth: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>