WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
36477
webkit-patch reads wrong bug url from unified diff context
https://bugs.webkit.org/show_bug.cgi?id=36477
Summary
webkit-patch reads wrong bug url from unified diff context
Hajime Morrita
Reported
2010-03-22 21:27:32 PDT
With "post-commits", webkit-patch can read wrong URL from "context" part of diff text + We expect
https://bugs.webkit.org/show_bug.cgi?id=12345
to be found + but, @@ -23,4 +25,4 @@ .... It found this
http://webkit.org/b/67890
instead. How to reproduce: 1. Checkout git.webkit.org tree 2. Ensure last changelog contains http://webkit.org/b/123 style bug URL 3. make change, with changelog which containshttps://bugs.webkit.org/show_bug.cgi=456 style bug URL 4. try post-commits --dry for that change Expected: - the patch would be post for Bug 456 Actual: - the patch is post for 123 A concrete example of such patch is
https://bug-36338-attachments.webkit.org/attachment.cgi?id=51146
Attachments
patch v0
(4.05 KB, patch)
2010-03-22 21:58 PDT
,
Hajime Morrita
no flags
Details
Formatted Diff
Diff
Patch
(6.92 KB, patch)
2010-03-31 02:22 PDT
,
Adam Barth
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Hajime Morrita
Comment 1
2010-03-22 21:58:56 PDT
Created
attachment 51393
[details]
patch v0
Eric Seidel (no email)
Comment 2
2010-03-22 22:47:49 PDT
Comment on
attachment 51393
[details]
patch v0 This is the wrong fix. In this case you want to just use the code for pulling the bug from the ChangeLogEntry instead of the diff. This proposed change would still wrongly grab bug urls added in other parts of the diff besides the ChangeLog. I recently added a ChangeLogEntry class which exposes a bug_id() method which should fix this case.
Eric Seidel (no email)
Comment 3
2010-03-22 22:48:21 PDT
Thank you for the fix attempt though! I think we have at least one dup of this bug.
Hajime Morrita
Comment 4
2010-03-22 23:09:14 PDT
Thank you for reviewing quickly! (In reply to
comment #2
)
> This is the wrong fix. In this case you want to just use the code for pulling > the bug from the ChangeLogEntry instead of the diff. This proposed change > would still wrongly grab bug urls added in other parts of the diff besides the > ChangeLog.
Agreed.
> I recently added a ChangeLogEntry class which exposes a bug_id() method which > should fix this case.
I couldn't find bug_id() on ChangeLogEntry. It looks not landed yet. Would you tell me which bug contains that? I'll watch it and retry this after its landing. BTW,
Bug 33471
and
Bug 33197
looks similar. But I'm not sure.
Adam Barth
Comment 5
2010-03-31 02:22:28 PDT
Created
attachment 52145
[details]
Patch
Eric Seidel (no email)
Comment 6
2010-03-31 11:24:06 PDT
Comment on
attachment 52145
[details]
Patch I don't think it's right that commit_message_for_this_commit throws. But that's an old mistake of mine. Would be nice to have a FIXME in your try-catch, but this is fine as is.
WebKit Commit Bot
Comment 7
2010-03-31 13:28:27 PDT
Comment on
attachment 52145
[details]
Patch Clearing flags on attachment: 52145 Committed
r56863
: <
http://trac.webkit.org/changeset/56863
>
WebKit Commit Bot
Comment 8
2010-03-31 13:28:34 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 9
2010-04-02 00:21:56 PDT
***
Bug 33471
has been marked as a duplicate of this bug. ***
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