Bug 19290

Summary:

More patches not handled by PrettyPatch.rb

Product:

WebKit

Reporter:

David Kilzer (:ddkilzer) <ddkilzer>

Component:

Tools / Tests

Assignee:

Adam Roben (:aroben) <aroben>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aroben

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
print exceptions from PrettyPatch to stdout	none
print exceptions from PrettyPatch to stdou	none
Fix most of the patches listed in this bug	none
Fix most of the patches listed in this bug	none
Fix remaining patches	none
Fix remaining patches v2	none

David Kilzer (:ddkilzer)

Reported 2008-05-28 07:14:12 PDT

More patches that aren't handled by PrettyPatch.rb: Bug 19041 https://bugs.webkit.org/attachment.cgi?id=21119&action=prettypatch Bug 18465 https://bugs.webkit.org/attachment.cgi?id=20510&action=prettypatch https://bugs.webkit.org/attachment.cgi?id=20528&action=prettypatch

Attachments
print exceptions from PrettyPatch to stdout (3.20 KB, patch) 2008-05-28 08:40 PDT, Adam Roben (:aroben)	no flags	Details Formatted Diff Diff
print exceptions from PrettyPatch to stdou (3.21 KB, patch) 2008-05-28 08:41 PDT, Adam Roben (:aroben)	no flags	Details Formatted Diff Diff
Fix most of the patches listed in this bug (3.41 KB, patch) 2008-05-28 10:19 PDT, Adam Roben (:aroben)	no flags	Details Formatted Diff Diff
Fix most of the patches listed in this bug (3.21 KB, patch) 2008-05-28 10:20 PDT, Adam Roben (:aroben)	no flags	Details Formatted Diff Diff
Fix remaining patches (2.63 KB, patch) 2008-06-02 15:10 PDT, David Kilzer (:ddkilzer)	no flags	Details Formatted Diff Diff
Fix remaining patches v2 (2.84 KB, patch) 2008-06-24 17:35 PDT, David Kilzer (:ddkilzer)	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

David Kilzer (:ddkilzer)

Comment 1 2008-05-28 07:15:26 PDT

Bug 19043 https://bugs.webkit.org/attachment.cgi?id=21120&action=prettypatch https://bugs.webkit.org/attachment.cgi?id=21121&action=prettypatch

David Kilzer (:ddkilzer)

Comment 2 2008-05-28 07:17:20 PDT

Bug 19067 https://bugs.webkit.org/attachment.cgi?id=21151&action=prettypatch

David Kilzer (:ddkilzer)

Comment 3 2008-05-28 07:18:41 PDT

Bug 19068 https://bugs.webkit.org/attachment.cgi?id=21152&action=prettypatch

David Kilzer (:ddkilzer)

Comment 4 2008-05-28 07:19:46 PDT

Bug 19069 https://bugs.webkit.org/attachment.cgi?id=21153&action=prettypatch

David Kilzer (:ddkilzer)

Comment 5 2008-05-28 07:58:06 PDT

Note that all patches listed in Comment #0 through Comment #4 (except Bug 18465) are from the same contributor.

Adam Roben (:aroben)

Comment 6 2008-05-28 08:40:22 PDT

Created attachment 21388 [details] print exceptions from PrettyPatch to stdout

Adam Roben (:aroben)

Comment 7 2008-05-28 08:41:56 PDT

Created attachment 21389 [details] print exceptions from PrettyPatch to stdou Fixed a typo

Adam Roben (:aroben)

Comment 8 2008-05-28 08:52:42 PDT

Comment on attachment 21389 [details] print exceptions from PrettyPatch to stdou Committed in r34173

Adam Roben (:aroben)

Comment 9 2008-05-28 08:55:52 PDT

It looks like attachment 20510 [details] and attachment 20528 [details] are missing the "Index: " line that PrettyPatch uses to find the start of the diff. All the other attachments are causing PrettyPatch trouble because they have a leading / in the path of the files. This should be easy to fix.

Adam Roben (:aroben)

Comment 10 2008-05-28 10:19:05 PDT

Created attachment 21392 [details] Fix most of the patches listed in this bug

Adam Roben (:aroben)

Comment 11 2008-05-28 10:20:53 PDT

Created attachment 21393 [details] Fix most of the patches listed in this bug Fixed a typo

David Kilzer (:ddkilzer)

Comment 12 2008-05-28 10:28:10 PDT

Comment on attachment 21393 [details] Fix most of the patches listed in this bug r=me!

Adam Roben (:aroben)

Comment 13 2008-05-28 10:33:04 PDT

Comment on attachment 21393 [details] Fix most of the patches listed in this bug Committed in r34175

Adam Roben (:aroben)

Comment 14 2008-05-28 10:33:48 PDT

The only remaining broken patches are: Attachment 20510 [details] Attachment 20528 [details]

David Kilzer (:ddkilzer)

Comment 15 2008-06-02 15:10:17 PDT

Created attachment 21469 [details] Fix remaining patches Fix the remaining issue for patches with no "Index" or "diff" headers.

David Kilzer (:ddkilzer)

Comment 16 2008-06-02 15:12:05 PDT

(In reply to comment #15) > Created an attachment (id=21469) [edit] > Fix remaining patches > > Fix the remaining issue for patches with no "Index" or "diff" headers. I should note that this change will NOT handle a patch with a mix of diffs that do and do not contain headers. I don't expect that case to ever happen, though.

Adam Roben (:aroben)

Comment 17 2008-06-03 09:20:44 PDT

Comment on attachment 21469 [details] Fix remaining patches I wonder if we should just always look for the "---" and "+++" lines instead of looking for "Index:" or "diff --git" lines? I think that's what the patch utility does anyway. It seems like it would simplify this code quite a bit. Care to take a stab at that, Dave?

David Kilzer (:ddkilzer)

Comment 18 2008-06-03 10:59:06 PDT

(In reply to comment #17) > (From update of attachment 21469 [details] [edit]) > I wonder if we should just always look for the "---" and "+++" lines instead of > looking for "Index:" or "diff --git" lines? I think that's what the patch > utility does anyway. It seems like it would simplify this code quite a bit. > > Care to take a stab at that, Dave? Yes, maybe today. Bug mail is awfully slow (or Y!Mail delivery has slowed way down).

Darin Adler

Comment 19 2008-06-08 14:03:57 PDT

Comment on attachment 21469 [details] Fix remaining patches r=me

David Kilzer (:ddkilzer)

Comment 20 2008-06-08 14:05:43 PDT

Comment on attachment 21469 [details] Fix remaining patches Sorry, Darin, I should have cleared the review? flag per Comment #17 and Comment #18.

David Kilzer (:ddkilzer)

Comment 21 2008-06-24 17:35:32 PDT

Created attachment 21918 [details] Fix remaining patches v2 (In reply to comment #17) > (From update of attachment 21469 [details] [edit]) > I wonder if we should just always look for the "---" and "+++" lines instead of > looking for "Index:" or "diff --git" lines? I think that's what the patch > utility does anyway. It seems like it would simplify this code quite a bit. > > Care to take a stab at that, Dave? I don't think ignoring the "Index:" and "diff --git" lines will work. There are some patches with an "Index:" line that don't have "---" or "+++" lines (like binary patches), so this approach won't work with those cases. I did manage to simplify the patch from the previous attempt, at least for the first two changes.

Adam Roben (:aroben)

Comment 22 2008-06-25 07:49:29 PDT

Comment on attachment 21918 [details] Fix remaining patches v2 r=me

David Kilzer (:ddkilzer)

Comment 23 2008-06-25 12:47:05 PDT

Remaining patches should have been fixed by: Committed revision 34797. But it doesn't appear that they were. :(

David Kilzer (:ddkilzer)

Comment 24 2008-06-25 12:47:26 PDT

Comment on attachment 21918 [details] Fix remaining patches v2 Clearing review flag per Comment #23.

David Kilzer (:ddkilzer)

Comment 25 2008-06-25 13:58:09 PDT

(In reply to comment #23) > Remaining patches should have been fixed by: > > Committed revision 34797. > > But it doesn't appear that they were. :( This was a server update issue. The issue is now resolved!

Note You need to log in before you can comment on or make changes to this bug.