Bug 39097 – webkit-patch barfed on upload with a new image test result

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Created attachment 56040 [details]
webkit-patch output

I ran 'webkit-patch upload NNN' with a new image result added to my git index. In the diff file (attached), I see:

Exception raised during decoding git binary patch:
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:67:in `exec': No such file or directory - /opt/local/bin/git apply --directory=/var/folders/4I/4IFxjUcVHIip-GD1W3OsEE+++TM/-Tmp- (Errno::ENOENT)
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:67:in `popen3'
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:53:in `fork'
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:53:in `popen3'
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:51:in `fork'
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/open3.rb:51:in `popen3'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:452:in `extract_contents_from_git_binary_chunk'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:373:in `initialize'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:372:in `collect'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:372:in `initialize'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `new'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `parse'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `collect'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `parse'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:15:in `prettify'
	from /Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/prettify.rb:26

/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:458:in `extract_contents_from_git_binary_chunk'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:373:in `initialize'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:372:in `collect'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:372:in `initialize'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `new'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `parse'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `collect'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:424:in `parse'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/PrettyPatch.rb:15:in `prettify'
/Volumes/InternalData/Development/webkit/OpenSource/BugsSite/PrettyPatch/prettify.rb:26

[Simon Fraser (smfr)]

On SnowLeopard, WebKit r59396

[Simon Fraser (smfr)]

No such file or directory - /opt/local/bin/git apply
My git is in /usr/local/bin/git.

[Simon Fraser (smfr)]

BugsSite/PrettyPatch/PrettyPatch.rb has GIT_PATH = "/opt/local/bin/git" which is bogus.

[Eric Seidel (no email)]

PrettyPatch is thanks to senior Roben, although others (including myself) have hacked on it.

Seems like an easy fix.  We should probably use 'which git' if available.

[Eric Seidel (no email)]

Thank you for the report, btw.

[Tony Chang]

Created attachment 64610 [details]
Patch

[Tony Chang]

Comment on attachment 64610 [details]
Patch

It looks like other scripts also expect git to be in your path (e.g., prepare-ChangeLog, resolve-ChangeLog, VCSUtils.pm).

[WebKit Review Bot]

Attachment 64610 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
BugsSite/ChangeLog:1:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
Total errors found: 9 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

prepare-ChangeLog works fine for me on the same machine.

[Eric Seidel (no email)]

Comment on attachment 64610 [details]
Patch

OK.  Makes sense.

[WebKit Commit Bot]

Comment on attachment 64610 [details]
Patch

Rejecting patch 64610 from commit-queue.

Unexpected failure when processing patch!  Please file a bug against webkit-patch.
Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 64610, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 66, in run
    if self._has_valid_reviewer(changelog_entry):
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 48, in _has_valid_reviewer
    if changelog_entry.reviewer():
AttributeError: 'NoneType' object has no attribute 'reviewer'

[Eric Seidel (no email)]

Comment on attachment 64610 [details]
Patch

I'm confused by that commit-queue error.  Lets try again. :)

[WebKit Commit Bot]

Comment on attachment 64610 [details]
Patch

Rejecting patch 64610 from commit-queue.

Unexpected failure when processing patch!  Please file a bug against webkit-patch.
Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 64610, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 66, in run
    if self._has_valid_reviewer(changelog_entry):
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 48, in _has_valid_reviewer
    if changelog_entry.reviewer():
AttributeError: 'NoneType' object has no attribute 'reviewer'

[Eric Seidel (no email)]

My guess is its failing to parse the changelog. Just not sure why.

[Adam Barth]

Likely because of the \r complained about in https://bugs.webkit.org/show_bug.cgi?id=39097#c8

[Eric Seidel (no email)]

Comment on attachment 64610 [details]
Patch

Seems like we should update webkit-patch to detect \r in a ChangeLog and barf nicely.

This can't be cq+'d as is.  And ideally the \r would be removed from the ChangeLog anyway.

[Tony Chang]

*** Bug 38101 has been marked as a duplicate of this bug. ***

[Tony Chang]

Created attachment 64867 [details]
Patch

[Tony Chang]

Comment on attachment 64867 [details]
Patch

I removed the \r's in ChangeLog (the whole file had \r's) and set the svn:eol-style to native.

[Adam Barth]

Comment on attachment 64867 [details]
Patch

Okiedokes.  Thanks for fixing the ChangeLog.

[WebKit Commit Bot]

Comment on attachment 64867 [details]
Patch

Clearing flags on attachment: 64867

Committed r65686: <http://trac.webkit.org/changeset/65686>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

It turns out that whatever computer runs PrettyPatch.rb when you click "Formatted Diff" on the bots depended on the full git path.  For example, the following patch fails even though it includes the git binary information in the diff.

https://bugs.webkit.org/attachment.cgi?id=64890&action=prettypatch

Where does this run?  Can we fix the path to git on the bots?

[Eric Seidel (no email)]

wms is our man!  if he can't do it, no one can!

[William Siegrist]

Dave, looks like we need to adjust the PATH for the open2() in attachment.cgi?

http://trac.webkit.org/browser/trunk/BugsSite/attachment.cgi#L395

[Eric Seidel (no email)]

jparent is also on rather intimate terms with bugzilla...

[Tony Chang]

(In reply to comment #25)
> Dave, looks like we need to adjust the PATH for the open2() in attachment.cgi?
> 
> http://trac.webkit.org/browser/trunk/BugsSite/attachment.cgi#L395

Alternately, maybe we can just symlink /usr/bin/git to /opt/local/bin/git on bugs.webkit.org?

I could also change PrettyPatch.rb to use /usr/bin/env git.

[William Siegrist]

Bugs' PrettyPatch has been fixed in <http://trac.webkit.org/changeset/66104>.

[David Kilzer (:ddkilzer)]

(In reply to comment #28)
> Bugs' PrettyPatch has been fixed in <http://trac.webkit.org/changeset/66104>.

Inadvertent changes backed out in r66105.
<http://trac.webkit.org/changeset/66105>