Bug 161606

Summary: "Redundant argument in sprintf" warning spam from prepare-ChangeLog
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: Tools / TestsAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, commit-queue, lforschler, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
Bug Depends on: 161628    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch darin: review+, commit-queue: commit-queue-

Description Michael Catanzaro 2016-09-05 09:10:29 PDT
I've been seeing this warning spam from prepare-ChangeLog for a year or so now:

$ prepare-ChangeLog
  Running status to find changed, added, or removed files.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 2.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 3.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 4.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 5.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 6.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 7.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 8.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 9.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 10.
Redundant argument in sprintf at /home/mcatanzaro/src/WebKit/Tools/Scripts/prepare-ChangeLog line 2342, <STAT> line 11.
Comment 1 Michael Catanzaro 2016-09-05 09:21:22 PDT
Created attachment 287966 [details]
Patch
Comment 2 Darin Adler 2016-09-05 09:50:13 PDT
Comment on attachment 287966 [details]
Patch

This change looks wrong. The argument is extraneous for some cases, and in that case it seems perl warns, but it is needed for others: $svn{A}, $svn{R}, $git{C} and $git{R}.
Comment 3 Michael Catanzaro 2016-09-05 11:45:50 PDT
You're right. I tested $git{"A"} because I was confused by the Perl and wanted to make sure I wasn't breaking anything, but that was dumb as it was the only git case that didn't use the extra format argument.

I tried moving the sprintf instead and it seems to work, though maybe there's a better Perl way to write this.
Comment 4 Michael Catanzaro 2016-09-05 11:48:02 PDT
Created attachment 287971 [details]
Patch
Comment 5 Darin Adler 2016-09-05 12:57:57 PDT
Comment on attachment 287971 [details]
Patch

OK.
Comment 6 WebKit Commit Bot 2016-09-05 17:05:05 PDT
Comment on attachment 287971 [details]
Patch

Clearing flags on attachment: 287971

Committed r205461: <http://trac.webkit.org/changeset/205461>
Comment 7 WebKit Commit Bot 2016-09-05 17:05:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 WebKit Commit Bot 2016-09-06 07:30:29 PDT
Re-opened since this is blocked by bug 161628
Comment 9 Michael Catanzaro 2016-09-06 07:45:03 PDT
The problem with the patch I landed: I failed to test the simple case of modifying an existing file. In that case, $original is unset. That's why the existing code checks if $original is defined when creating the lookup table. In the case of git, we know if there's no $original file then it wasn't copied or renamed, so we don't need to add it to the lookup table.
Comment 10 Michael Catanzaro 2016-09-06 07:47:30 PDT
Created attachment 288023 [details]
Patch
Comment 11 Michael Catanzaro 2016-09-06 07:49:50 PDT
(In reply to comment #9)
> The problem with the patch I landed: I failed to test the simple case of
> modifying an existing file.

It raises the question of how I managed to generate the changelog for that commit. I think I just copy/pasted it from the original patch instead of running prepare-ChangeLog to do it. Dumb!
Comment 12 WebKit Commit Bot 2016-09-06 09:03:54 PDT
Comment on attachment 288023 [details]
Patch

Rejecting attachment 288023 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 288023, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/git.webkit.org/WebKit
   43eeed4..0e0ce3b  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 205478 = 43eeed4fd35fabe64d8a06905563b2842a8fcfb6
r205479 = 0e0ce3b1bc1b1d4bf8aeef66fbc3e592ccd9aa43
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/2018313
Comment 13 Michael Catanzaro 2016-09-06 09:26:03 PDT
Committed r205483: <http://trac.webkit.org/changeset/205483>