Bug 63452 – webkit-patch doesn't like UTF-8 characters in reviewers names

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

https://bugs.webkit.org/show_bug.cgi?id=46714#c5
https://bugs.webkit.org/show_bug.cgi?id=61890#c12

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 1

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=97789&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=61890&ctype=xml
Processing 1 patch from 1 bug.
Processing patch 97789 from bug 61890.
Traceback (most recent call last):
  File "./Tools/Scripts/webkit-patch", line 66, in <module>
    main()
  File "./Tools/Scripts/webkit-patch", line 61, in main
    WebKitPatch(__file__).main()
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 308, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 117, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/download.py", line 152, in execute
    self._process_patch(patch, options, args, tool)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/download.py", line 171, in _process_patch
    self._main_sequence.run_and_handle_errors(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/applypatchwithlocalcommit.py", line 40, in run
    ApplyPatch.run(self, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/applypatch.py", line 43, in run
    self._tool.checkout().apply_patch(state["patch"], force=self._options.non_interactive or self._options.force_patch)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/checkout/checkout.py", line 163, in apply_patch
    run_command(args, input=patch.contents())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 102, in run_command
    return Executive().run_command(*args, **kwargs)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 382, in run_command
    close_fds=self._should_close_fds())
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 438, in popen
    return subprocess.Popen(*args, **kwargs)
  File "/usr/lib/python2.6/subprocess.py", line 623, in __init__
    errread, errwrite)
  File "/usr/lib/python2.6/subprocess.py", line 1141, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

[Csaba Osztrogonác]

Kristóf, could you check this bug, please?

[Eric Seidel (no email)]

I believe this is related to us setting LANG=ascii on the bots to work around a gcc issue.  See bug 50843.

[Csaba Osztrogonác]

I set it to P1 blocker, because webkit-patch and CQ can't land patches reviewed by myself. :(

[Csaba Osztrogonác]

Peter, could you check this bug with Kristof ASAP?

[Eric Seidel (no email)]

Sigh.  I thought this had gone away.  I can take another look.  A test case (patch) would be most helpful.

[Csaba Osztrogonác]

Guys wasn't able to reproduce it locally, but unfortunately it is still valid on the commit queue machine: https://bugs.webkit.org/show_bug.cgi?id=67646#c2

Eric or someone else who has access to this bot, could you check it, please?
It is so annoying that we can't land patches reviewed by me with the commit queue. :(

[Eric Seidel (no email)]

It's on the (growing) list of bugs I'd like to fix. :(

[Adam Barth]

@Eric: time to create a meta bug and burn through these?

[Tom Zakrajsek]

Created attachment 108853 [details]
Patch

[Tom Zakrajsek]

I'm not sure how to set up a regression test for this.  I can test that the mod (wrapping stdio with a codec) works in a standalone unit-test, but it's a lot more invasive to try to force unicode text through the output of webkit-patch itself.

[Adam Barth]

Eric is probably your best reviewer here.  Generally, it's hard to test changes to the webkit-patch script self (as opposed to the webkitpy library).  I suspec this change wi'll have somewhat wide-ranging effects that will be difficult to predict, but we shouldn't let that stop us!

[Tom Zakrajsek]

Created attachment 108876 [details]
Test case to show

Uploading an example script to demonstrate the problem/solution in a minimal case.  To see the issue, run the script and redirect/tee the output.

[Eric Seidel (no email)]

Comment on attachment 108853 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108853&action=review

> Tools/Scripts/webkit-patch:41
> +sys.stdout = codecs.lookup('utf-8')[-1](sys.stdout)

Please add a (brief) comment explaing why we do this, and linking to this bug.  You should also consider linking from this bug or your ChangeLog to places on the web where they explain why this is the right way to fix this.   Just some sort of paper trail so that someone can understand later why this change was made.

[Tom Zakrajsek]

Created attachment 108885 [details]
Patch

[Tom Zakrajsek]

For more information, see "Python Cookbook 2nd Ed., recipe 1.22" and http://docs.python.org/library/codecs.html#module-codecs.

[Eric Seidel (no email)]

Comment on attachment 108885 [details]
Patch

Thanks.

[WebKit Review Bot]

Comment on attachment 108885 [details]
Patch

Clearing flags on attachment: 108885

Committed r96161: <http://trac.webkit.org/changeset/96161>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

Unfortunately CQ still hates me. :(

[Csaba Osztrogonác]

(In reply to comment #19)
> Unfortunately CQ still hates me. :(

I missed to add the URL: https://bugs.webkit.org/show_bug.cgi?id=69004#c4

[Eric Seidel (no email)]

Another reproduction:
webkit-patch apply-attachment 113861 [details]

That doesn't repro on my local machine, but I suspect it will repro on our EWS instances.  I'll figure out how to SSH into one and try it.

[Eric Seidel (no email)]

The EC2 instances use some environment variables to work around fancy-quotes in GCC causing unicode decoding exceptions during other parts of processing.  Those environment variables are what's causing the problem here.  We may need to revisit which ones we set.

The following invocation of the command will reproduce this bug *on ec2* it still does not reproduce locally on my mac.

LANG=en_US.US-ASCII LC_ALL=C PYTHONIOENCODING=utf_8 ./Tools/Scripts/webkit-patch apply-attachment 113861 [details]

[Csaba Osztrogonác]

CQ works fine nowadays with patches reviewed by myself or Tor Arne :)