WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
38870
svn.webkit.org needs a post-commit hook to fix svn:author from commit-queue patches
https://bugs.webkit.org/show_bug.cgi?id=38870
Summary
svn.webkit.org needs a post-commit hook to fix svn:author from commit-queue p...
Eric Seidel (no email)
Reported
2010-05-10 15:06:12 PDT
svn.webkit.org needs a post-commit hook to fix svn:author from commit-queue patches Proposal is detailed here:
https://lists.webkit.org/pipermail/webkit-dev/2010-April/012350.html
Should be simple to write. It needs to be tested locally (to make sure it can't cause the commit to fail in any way, even if it fails), and then handed off to Bill Siegrist for integration with svn.webkit.org
Attachments
Patch
(5.06 KB, patch)
2010-05-10 16:31 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Patch
(5.00 KB, patch)
2010-05-10 16:35 PDT
,
Adam Barth
eric
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2010-05-10 15:07:51 PDT
Bug 29274
should also be fixed before this is turned on to make it possible to distinguish patches which were landed via the commit-queue from those which weren't.
Eric Seidel (no email)
Comment 2
2010-05-10 15:11:06 PDT
Tests that this post-commit hook should pass: - correctly do nothing patches where the author is not a committer (committers.py or the svn auth file) - correctly not block the commit in case of some sort of lookup error (the fix-the-author script should be separate)
William Siegrist
Comment 3
2010-05-10 15:15:25 PDT
This part of the hook will have to be run synchronously and finish before anything else in the hook runs, so that everything else will see the correct author (emails to -changes, buildbot, etc). So we'll need a way of making this timeout if it hangs for any reason.
Adam Barth
Comment 4
2010-05-10 15:16:40 PDT
Are our existing commit hooks in SVN somewhere? If you show me an example, I might be able to do this.
William Siegrist
Comment 5
2010-05-10 15:22:11 PDT
The hook script is not in svn. The hook script is just a shell script, with two variables already defined for you (path to the repo and the revision number). I can make webkitpy available on that server as well if that helps.
Adam Barth
Comment 6
2010-05-10 16:31:17 PDT
Created
attachment 55619
[details]
Patch
Adam Barth
Comment 7
2010-05-10 16:32:55 PDT
This is more of a first draft for discussion. Open questions: 1) Should we only change the svn:author for the commit-queue? 2) How does this code handle the case where we have a users bugzilla email in committers.py but not their svn email? 3) Can post-commit call this script directly or do we need a shell script in between? 4) What's our testing plan here?
Adam Barth
Comment 8
2010-05-10 16:35:24 PDT
Created
attachment 55621
[details]
Patch
Adam Barth
Comment 9
2010-05-10 16:38:24 PDT
I should also say that I don't have any clue if this script works because I don't know how to test it.
Eric Seidel (no email)
Comment 10
2010-05-10 16:42:01 PDT
I think in the end we're going to want to use the svn auth file (whatever that looks like). We should be able to test this using various ChangeLog entries as test cases.
Eric Seidel (no email)
Comment 11
2010-05-10 16:45:33 PDT
(In reply to
comment #7
)
> This is more of a first draft for discussion. Open questions:
Thank you!
> 1) Should we only change the svn:author for the commit-queue?
Yes. Otherwise you could "commit as" darin just by putting his name in the ChangeLog. Eeek. Ideally we should create a separate commit account for the commit-queue. :)
> 2) How does this code handle the case where we have a users bugzilla email in committers.py but not their svn email?
I don't think we should use the committers.py list. It's nice, but it's not as accurate as the theoretical svn auth file on the server (which I know nothing about).
> 3) Can post-commit call this script directly or do we need a shell script in between?
I don't see why not.
> 4) What's our testing plan here?
If we're using webkitpy, we should be able to write python unit tests for testing what happens with various ChangeLog messages. scm_unittests or checkout_unittests would be appropriate places.
Eric Seidel (no email)
Comment 12
2010-05-10 16:48:47 PDT
(In reply to
comment #9
)
> I should also say that I don't have any clue if this script works because I don't know how to test it.
We would have to set up a post-commit hook on a local test repository. possibly as part of scm_unittest, or more likely as part of some new module which was this command.
William Siegrist
Comment 13
2010-05-10 19:04:05 PDT
This seems like a lot of abstraction for an svn hook. And we don't have a working copy on the server, so this should end up using svnlook instead. I'll work on an alternate approach.
Eric Seidel (no email)
Comment 14
2010-05-10 19:18:37 PDT
Really all one needs is: 1. The location and format of the svn auth file (probably grep is all one really needs) 2. A regexp with which to parse date lines. The trouble is, if the ChangeLog ever doens't start at the top of the file, then you coudl pull the wrong author for the commit! We may need a pre-commit hook for SVN to prevent such bad changelog edits before we could really use something like this dependably. See
bug 28291
.
Eric Seidel (no email)
Comment 15
2010-05-10 23:54:39 PDT
Comment on
attachment 55621
[details]
Patch wms is going to work up an alternate approach he says.
Eric Seidel (no email)
Comment 16
2010-06-25 11:35:05 PDT
wms is currently testing a solution.
William Siegrist
Comment 17
2010-12-24 11:03:55 PST
I'm pretty sure this has been fixed for a while. Eric, was there any other changes you wanted under this bug?
Eric Seidel (no email)
Comment 18
2010-12-24 11:55:50 PST
Nope. Thanks.
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