Summary: | VCSUtils.pm complains about uninitialized value $newLine | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dumitru Daniliuc <dumi> | ||||||||
Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, cjerdonek, eric, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Dumitru Daniliuc
2010-06-28 22:33:05 PDT
I was able to get past this problem by changing my $newLine = pop(@overlappingLines); if ($text ne substr($newLine, 1)) { to my $newLine = (scalar @overlappingLines > 0 ? pop(@overlappingLines) : ""); if ((length($newLine) == 0) || ($text ne substr($newLine, 1))) { The patch file I got seems to be OK. Created attachment 59990 [details]
ChangeLogs files + patch
Created attachment 60679 [details]
[Patch] Work in progress
Created attachment 60832 [details]
Patch with unit test
We run off the end of an array when processing a change log entry that was inserted earlier in the file, but after an entry with the same author and date (because of the blank line of context at the top of the change log entry).
Currently, when we detect that the new change log entry is earlier in the change log file we do not move this entry to the top of the file as the entry may have been explicitly placed earlier by the author of the patch. This situation seems rare and we may want to re-consider this decision and/or provide some kind of optional boolean flag to forcefully move a new change log entry to the top of the change log to prevent inadvertently landing a change with a change log entry in the wrong place. Alternatively, we may want to consider adding a warning message of the form "WARNING: The new change log entry is not at the top of the change log. Make sure this is what you intended" when we detect such a change log entry.
For the purpose of this patch, I have decided to only fix the array issue as I think it is best that we resolve the placement issue/print a warning in a separate patch.
Comment on attachment 60832 [details]
Patch with unit test
r=me.
FWIW, I think we we should always push new ChangeLogs entries to the top.
Comment on attachment 60832 [details] Patch with unit test Clearing flags on attachment: 60832 Committed r62755: <http://trac.webkit.org/changeset/62755> All reviewed patches have been landed. Closing bug. (In reply to comment #5) > (From update of attachment 60832 [details]) > r=me. > > FWIW, I think we we should always push new ChangeLogs entries to the top. I filed bug #41833 to track this issue. http://trac.webkit.org/changeset/62755 might have broken GTK Linux 64-bit Debug (In reply to comment #9) > http://trac.webkit.org/changeset/62755 might have broken GTK Linux 64-bit Debug The error message appear unrelated to this change. On another note, this bot has run out of disk space as per the error messages in the Perl and Python unit test stdio, <http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/9397/steps/webkitperl-test/logs/stdio> and <http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug/builds/9397/steps/webkitpy-test/logs/stdio>, respectively. |