RESOLVED FIXED Bug 26715
bugzilla-tool should automate rollouts
https://bugs.webkit.org/show_bug.cgi?id=26715
Summary bugzilla-tool should automate rollouts
Eric Seidel (no email)
Reported 2009-06-25 01:22:15 PDT
bugzilla-tool should automate rollouts 1. Should take an svn revision and know how to search through git logs as necessary. (I think git svn has a command for this.) 2. Do the revert. 3. Update the ChangeLog to note that no review is needed. 4. Test that things build correctly, etc. 5. Re-open the bug with a rollout message.
Attachments
Work in progress (14.94 KB, patch)
2009-07-02 02:24 PDT, Eric Seidel (no email)
no flags
Working, but not ready for primetime (16.48 KB, patch)
2009-07-02 02:59 PDT, Eric Seidel (no email)
no flags
Updated patch for rollout support (16.90 KB, patch)
2009-07-17 15:04 PDT, Eric Seidel (no email)
no flags
Begone ye tabs! (16.92 KB, patch)
2009-07-17 15:06 PDT, Eric Seidel (no email)
no flags
Updated, just needs testing with more rollouts (16.72 KB, patch)
2009-07-31 22:32 PDT, Eric Seidel (no email)
no flags
Updated to ToT (9.34 KB, patch)
2009-08-11 23:16 PDT, Eric Seidel (no email)
no flags
Update rollout support (24.72 KB, patch)
2009-08-19 14:55 PDT, Eric Seidel (no email)
no flags
Patch v1 (9.13 KB, patch)
2009-09-10 14:12 PDT, Eric Seidel (no email)
no flags
Works well enough for others to start to use (9.20 KB, patch)
2009-09-10 14:16 PDT, Eric Seidel (no email)
no flags
Make ChangeLog less pessimistic (9.12 KB, patch)
2009-09-10 14:18 PDT, Eric Seidel (no email)
no flags
Add a bunch of testing code for abarth (12.14 KB, patch)
2009-09-10 17:29 PDT, Eric Seidel (no email)
no flags
Final patch with a bunch of extra testing (18.86 KB, patch)
2009-09-10 17:30 PDT, Eric Seidel (no email)
no flags
Document all the extra testing in the ChangeLog (20.08 KB, patch)
2009-09-10 17:40 PDT, Eric Seidel (no email)
no flags
Eric Seidel (no email)
Comment 1 2009-06-25 01:31:13 PDT
Once we have a way to do automated rollbacks, then we could eventually consider adding a web interface to do so. :) Never have a red tree again! :)
Eric Seidel (no email)
Comment 2 2009-07-02 02:24:24 PDT
Created attachment 32173 [details] Work in progress
Eric Seidel (no email)
Comment 3 2009-07-02 02:59:40 PDT
Created attachment 32174 [details] Working, but not ready for primetime
Eric Seidel (no email)
Comment 4 2009-07-17 15:04:40 PDT
Created attachment 32974 [details] Updated patch for rollout support --- 4 files changed, 189 insertions(+), 51 deletions(-)
Eric Seidel (no email)
Comment 5 2009-07-17 15:06:24 PDT
Created attachment 32976 [details] Begone ye tabs! --- 4 files changed, 189 insertions(+), 51 deletions(-)
David Levin
Comment 6 2009-07-17 19:42:25 PDT
Comment on attachment 32976 [details] Begone ye tabs! Just a few things to address. I'm most concerned about the issue I mentioned in update_changelog_for_revert. Two general notes: 1. It was pointed out to me that python style is to have two blank lines between top level functions: "Separate top-level function and class definitions with two blank lines." 2. Many comments don't end with periods. Please add them. > diff --git a/WebKitTools/Scripts/bugzilla-tool b/WebKitTools/Scripts/bugzilla-tool > +def update_changelog_for_revert(changelog_path, revision): > + removing_boilerplate = False > + # inplace=1 creates a backup file and re-directs stdout to the file > + for line in fileinput.FileInput(changelog_path, inplace=1): > + if re.search('Reviewed by', line): I'm confused. How does this stop after this boilerplate change log that was just created? It looks like it goes through the whole changelog removing all descriptions along the way. > + print line.replace("Reviewed by NOBODY (OOPS!)", "No review, rolling out %s" % revision), I like to prefix revisions with an r. Consider: ..."No review, rolling out r%s"... > +class RolloutCommit(Command): ... > + def create_changelogs_for_revert(scm, revision): ... > + # This could move to prepare-ChangeLog by adding a --revert= option That might be easier. I think the current code has a bug as I noted above. > diff --git a/WebKitTools/Scripts/modules/scm.py b/WebKitTools/Scripts/modules/scm.py > + @classmethod > + def git_commit_from_svn_revision(cls, revision): > + git_log_output = cls.run_command(['git', 'svn', 'log', '-r', revision, '--oneline', '--show-commit']) > + # Example: r45396 | b4954e7 | 2009-06-30 Antonio Gomes <antonio.gomes@openbossa.org> > + matched_commit = re.search("^r(\d+) \| (\w+) \| .*$", git_log_output, re.MULTILINE) This is ok, but I always find code more readable when it uses named groups.
David Levin
Comment 7 2009-07-20 08:34:00 PDT
btw, ignore this comment: > + # This could move to prepare-ChangeLog by adding a --revert= option That might be easier. I think the current code has a bug as I noted above. as I realized I was completely wrong about it being easier and I rather like it in these files :)
Evan Martin
Comment 8 2009-07-22 11:28:31 PDT
+ git_log_output = cls.run_command(['git', 'svn', 'log', '-r', revision, '--oneline', '--show-commit']) + # Example: r45396 | b4954e7 | 2009-06-30 Antonio Gomes <antonio.gomes@openbossa.org> + matched_commit = re.search("^r(\d+) \| (\w+) \| .*$", git_log_output, re.MULTILINE) git svn find-rev r45396
Eric Seidel (no email)
Comment 9 2009-07-31 22:32:47 PDT
Created attachment 33930 [details] Updated, just needs testing with more rollouts --- 4 files changed, 186 insertions(+), 53 deletions(-)
Eric Seidel (no email)
Comment 10 2009-08-04 11:33:41 PDT
Some testing code for later: def _parse_revision_to_commit_mappings(self, stderr): revision_to_commit_mapping = {} for line in stderr.split_lines(): match = line.match("^(?P<revision>r\d+) = (?P<commit_id>\w+)$") if match: revision_to_commit_mapping[match.group('revision')] = match.group('commit_id') return revision_to_commit_mapping
Eric Seidel (no email)
Comment 11 2009-08-11 23:16:13 PDT
Created attachment 34639 [details] Updated to ToT --- 4 files changed, 123 insertions(+), 3 deletions(-)
Eric Seidel (no email)
Comment 12 2009-08-19 14:55:33 PDT
Created attachment 35147 [details] Update rollout support --- 7 files changed, 373 insertions(+), 49 deletions(-)
Eric Seidel (no email)
Comment 13 2009-09-10 14:12:02 PDT
Created attachment 39377 [details] Patch v1
Eric Seidel (no email)
Comment 14 2009-09-10 14:16:29 PDT
Created attachment 39379 [details] Works well enough for others to start to use
Eric Seidel (no email)
Comment 15 2009-09-10 14:18:09 PDT
Created attachment 39381 [details] Make ChangeLog less pessimistic
Adam Barth
Comment 16 2009-09-10 14:47:45 PDT
Comment on attachment 39381 [details] Make ChangeLog less pessimistic Would be nice to have some unittests for this change... We also discussed some alternate approaches if the ChangeLog conflicts are too problematic.
Eric Seidel (no email)
Comment 17 2009-09-10 17:29:46 PDT
Created attachment 39397 [details] Add a bunch of testing code for abarth
Eric Seidel (no email)
Comment 18 2009-09-10 17:30:54 PDT
Created attachment 39398 [details] Final patch with a bunch of extra testing
Eric Seidel (no email)
Comment 19 2009-09-10 17:31:48 PDT
Note that I uploaded a diff from the reviewed patch which just contains the new testing changes as "Add a bunch of testing code for abarth". The final patch (including all the added tests) is up for review again.
Eric Seidel (no email)
Comment 20 2009-09-10 17:40:18 PDT
Created attachment 39400 [details] Document all the extra testing in the ChangeLog
Adam Barth
Comment 21 2009-09-10 21:52:59 PDT
Comment on attachment 39400 [details] Document all the extra testing in the ChangeLog Thanks for the tests.
WebKit Commit Bot
Comment 22 2009-09-11 01:12:24 PDT
Comment on attachment 39400 [details] Document all the extra testing in the ChangeLog Rejecting patch 39400 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
Eric Seidel (no email)
Comment 23 2009-09-11 09:06:39 PDT
Comment on attachment 39400 [details] Document all the extra testing in the ChangeLog Bitten by bug 28845. :( media/video-source-error.html -> timed out
WebKit Commit Bot
Comment 24 2009-09-11 09:54:51 PDT
Comment on attachment 39400 [details] Document all the extra testing in the ChangeLog Clearing flags on attachment: 39400 Committed r48303: <http://trac.webkit.org/changeset/48303>
WebKit Commit Bot
Comment 25 2009-09-11 09:55:01 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 26 2009-09-11 10:12:13 PDT
Note You need to log in before you can comment on or make changes to this bug.