I'm currently in a state where Xcode outputs something with a curly quote when building, and that breaks run-webkit-tests (which calls build-dumprendertree). UnicodeEncodeError raised: 'ascii' codec can't encode character u'\u201c' in position 1514: ordinal not in range(128) Traceback (most recent call last): File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 85, in main run_details = run(port, options, args, stderr) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 445, in run run_details = manager.run(args) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 246, in run if not self._set_up_run(tests_to_run): File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 164, in _set_up_run if not self._port.check_build(self.needs_servers(test_names)): File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/port/base.py", line 230, in check_build if not self._root_was_set and self.get_option('build') and not self._build_driver(): File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/port/base.py", line 1438, in _build_driver self._run_script("build-dumprendertree", args=self._build_driver_flags(), env=env) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/port/base.py", line 1428, in _run_script _log.debug('Output of %s:\n%s' % (run_script_command, output)) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/__init__.py", line 1153, in debug self._log(DEBUG, msg, args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/__init__.py", line 1284, in _log self.handle(record) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/__init__.py", line 1294, in handle self.callHandlers(record) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/__init__.py", line 1334, in callHandlers hdlr.handle(record) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/logging/__init__.py", line 757, in handle self.emit(record) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py", line 135, in emit self._meter.writeln(record.getMessage(), record.created, record.process) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py", line 111, in writeln self.write(self._ensure_newline(txt), now, pid) File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py", line 108, in write self._stream.write(msg) UnicodeEncodeError: 'ascii' codec can't encode character u'\u201c' in position 1514: ordinal not in range(128)
Created attachment 330409 [details] proposed fix I don't really know what I'm doing here, just patched it enough to work on my machine. Thoughts?
Comment on attachment 330409 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=330409&action=review > Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py:108 > + self._stream.write(msg.encode('utf-8')) This makes the assumption that the output shell is using UTF-8 encoding. Is this the default shell encoding on macOS, Linux, Windows? Alternatively we could appropriately encode the output we get when running build-dumprendertree.
> Is this the default shell encoding on macOS, Linux, Windows? I don't know how to answer that, or what the code was doing by default. I only know that this fixes my local build.
Comment on attachment 330409 [details] proposed fix Please feel encouraged to roll back if this breaks anything.
Comment on attachment 330409 [details] proposed fix Clearing flags on attachment: 330409 Committed r226816: <https://trac.webkit.org/changeset/226816>
All reviewed patches have been landed. Closing bug.
<rdar://problem/36453374>
This broke LayoutTests on the bots: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/2213
Reverted r226816 for reason: This change broke LayoutTests on the bots. Committed r226837: <https://trac.webkit.org/changeset/226837>
I'm not sure that this should be fixed in webkitpy. I've seen a similar issue where LEFT and RIGHT quotations coming from xcodebuild have the same problem. Perhaps we should place a filter around calls to xcodebuild in perl.
Created attachment 331207 [details] Patch
(In reply to Jonathan Bedard from comment #11) > Created attachment 331207 [details] > Patch Talking with Alexey, it seems that we are not willing to filter in build-webkit. I think we need a larger discussion of which string encoding we would like to standardize in webkitpy, but for the time being, we need to prevent exceptions.
Comment on attachment 331207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331207&action=review > Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py:109 > + self._stream.write(msg.encode(self._encoding, 'replace')) Did you confirm that this actually fixes the issue? To be more specific, were you able to reproduce the original bug? And did you verify that this actually fixes the bug? From briefly looking at the backtrace in <https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/2213/steps/layout-test/logs/stdio> I get the impression that the bug was that we were UTF-8 encoding twice (e.g. u"\u2013".encode('utf-8').encode('utf-8')). Assuming this is the bug then I would expect the fix to be that we UTF-8 encode |output| before perform string interpolation and passing the result to _log.debug() as seen in the original backtrace: File "/Users/ap/Safari/OpenSource/Tools/Scripts/webkitpy/port/base.py", line 1428, in _run_script _log.debug('Output of %s:\n%s' % (run_script_command, output))
Comment on attachment 331207 [details] Patch I had not yet verified that this fixed the new bug. I managed to reproduce the new bug locally, the proposed patch does not address it.
Created attachment 331238 [details] Patch
(In reply to Jonathan Bedard from comment #15) > Created attachment 331238 [details] > Patch This will address Alexey's problem without the side-effects.
Comment on attachment 331238 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331238&action=review > Tools/Scripts/webkitpy/port/base.py:1429 > + if decode_output: > + output = output.decode('utf-8') This does not make sense because |output| represents the decoded output when decode_output is True. Please start reading from the last sentence of the first paragraph in comment #13 again.
Created attachment 331390 [details] Patch
Comment on attachment 331390 [details] Patch I looked closer at how I was attempting to reproduce the failure that Alexey saw, and realized that I have yet to actually reproduce it. Dan's comments were accurate, and this change essentially applies Alexey's previous patch to only decoded script runs.
Comment on attachment 331390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331390&action=review > Tools/Scripts/webkitpy/port/base.py:1430 > + if decode_output: > + output = output.encode('utf-8') > _log.debug('Output of %s:\n%s' % (run_script_command, output)) We should only encode |output| for the purpose of calling _log.debug(). That is, we should not return the encoded data because the caller expects a string of characters (not a string of bytes).
Created attachment 331528 [details] Patch
(In reply to Daniel Bates from comment #20) > Comment on attachment 331390 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331390&action=review > > > Tools/Scripts/webkitpy/port/base.py:1430 > > + if decode_output: > > + output = output.encode('utf-8') > > _log.debug('Output of %s:\n%s' % (run_script_command, output)) > > We should only encode |output| for the purpose of calling _log.debug(). That > is, we should not return the encoded data because the caller expects a > string of characters (not a string of bytes). The output is unused by the callers, but this is a fair point. Especially since this bug only occurs with an explicit request to decode the output.
Comment on attachment 331528 [details] Patch r=me
Comment on attachment 331528 [details] Patch Clearing flags on attachment: 331528 Committed r227222: <https://trac.webkit.org/changeset/227222>