WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Working, but not ready for primetime
(16.48 KB, patch)
2009-07-02 02:59 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Updated patch for rollout support
(16.90 KB, patch)
2009-07-17 15:04 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Begone ye tabs!
(16.92 KB, patch)
2009-07-17 15:06 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Updated, just needs testing with more rollouts
(16.72 KB, patch)
2009-07-31 22:32 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Updated to ToT
(9.34 KB, patch)
2009-08-11 23:16 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Update rollout support
(24.72 KB, patch)
2009-08-19 14:55 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Patch v1
(9.13 KB, patch)
2009-09-10 14:12 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
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
Details
Formatted Diff
Diff
Make ChangeLog less pessimistic
(9.12 KB, patch)
2009-09-10 14:18 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Add a bunch of testing code for abarth
(12.14 KB, patch)
2009-09-10 17:29 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Final patch with a bunch of extra testing
(18.86 KB, patch)
2009-09-10 17:30 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Document all the extra testing in the ChangeLog
(20.08 KB, patch)
2009-09-10 17:40 PDT
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(12)
View All
Add attachment
proposed patch, testcase, etc.
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
Committed
r48304
: <
http://trac.webkit.org/changeset/48304
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug