Bug 89573 – webkit-patch can't handle if there are more than one "Reviewed by NOBODY (OOPS!)." line in the changelog

• Status: RESOLVED
• Resolution: INVALID
• Component: Tools / Tests

[Csaba Osztrogonác]

This bug is revealed by https://bugs.webkit.org/show_bug.cgi?id=67935

https://bugs.webkit.org/attachment.cgi?id=148323&action=review was reviewed by rniwa,
and cq couldn't land the patch. We tried to apply it with webkit-patch apply-attachment
and apply-from-bug, but webkit-patch replaced only the first occurance of "Reviewed by NOBODY ..."

We don't know why ... The following code should replace all, but it doesn't do it.
I'm sure that replacing all "NOBODY (OOPS!)" wouldn't be the correct behaviour,
but this code shouldn't work as it works now. :)

http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/checkout/changelog.py#L347
353	            for line in fileinput.FileInput(self.path, inplace=1):
354	                # Trailing comma suppresses printing newline
355	                print line.replace("NOBODY (OOPS!)", reviewer.encode("utf-8")),

[Eric Seidel (no email)]

It's actually bothered me for a long time that webkit-patch land replaces more than the first entry, so as long as it's not throwing an exception, I'd prefer if it only replaced the first entry in the ChangeLog. :)

[Eric Seidel (no email)]

This makes it easier for me to land the top patch in a series of git commits using something liek this:

webkit-patch land-safely -g head....

If webkit-patch replaces more than the first NOBODY, than the generated patch will fail to apply. :)

[Csaba Osztrogonác]

Ouch, I got it. :) webkit-patch apply-attachment command (and apply-from-bug too) uses Tools/Script/svn-apply perl script to apply the patch and add reviewer name with its --reviewer option. Which replaces the first occurence and the python code I copy/pasted never runs in this case. Fortunately :) Because it would be add one more reviewer line because of a bug, see https://bugs.webkit.org/show_bug.cgi?id=67935#c26 for details.

So this bug isn't bug, it is absolutely invalid. Sorry for the noise.