WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
181254
run-webkit-tests fails when there is a curly brace in Xcode build output
https://bugs.webkit.org/show_bug.cgi?id=181254
Summary
run-webkit-tests fails when there is a curly brace in Xcode build output
Alexey Proskuryakov
Reported
2018-01-03 12:25:27 PST
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)
Attachments
proposed fix
(1.22 KB, patch)
2018-01-03 12:27 PST
,
Alexey Proskuryakov
no flags
Details
Formatted Diff
Diff
Patch
(2.23 KB, patch)
2018-01-12 09:35 PST
,
Jonathan Bedard
no flags
Details
Formatted Diff
Diff
Patch
(1.30 KB, patch)
2018-01-12 14:50 PST
,
Jonathan Bedard
no flags
Details
Formatted Diff
Diff
Patch
(1.29 KB, patch)
2018-01-16 09:01 PST
,
Jonathan Bedard
no flags
Details
Formatted Diff
Diff
Patch
(1.36 KB, patch)
2018-01-17 11:53 PST
,
Jonathan Bedard
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2018-01-03 12:27:12 PST
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?
Daniel Bates
Comment 2
2018-01-11 10:33:59 PST
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.
Alexey Proskuryakov
Comment 3
2018-01-11 10:39:38 PST
> 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.
Alexey Proskuryakov
Comment 4
2018-01-11 15:50:48 PST
Comment on
attachment 330409
[details]
proposed fix Please feel encouraged to roll back if this breaks anything.
WebKit Commit Bot
Comment 5
2018-01-11 16:15:18 PST
Comment on
attachment 330409
[details]
proposed fix Clearing flags on attachment: 330409 Committed
r226816
: <
https://trac.webkit.org/changeset/226816
>
WebKit Commit Bot
Comment 6
2018-01-11 16:15:19 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7
2018-01-11 16:20:38 PST
<
rdar://problem/36453374
>
Ryan Haddad
Comment 8
2018-01-11 18:17:29 PST
This broke LayoutTests on the bots:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/2213
Ryan Haddad
Comment 9
2018-01-11 18:19:30 PST
Reverted
r226816
for reason: This change broke LayoutTests on the bots. Committed
r226837
: <
https://trac.webkit.org/changeset/226837
>
Jonathan Bedard
Comment 10
2018-01-12 08:33:03 PST
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.
Jonathan Bedard
Comment 11
2018-01-12 09:35:46 PST
Created
attachment 331207
[details]
Patch
Jonathan Bedard
Comment 12
2018-01-12 09:37:37 PST
(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.
Daniel Bates
Comment 13
2018-01-12 11:47:55 PST
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))
Jonathan Bedard
Comment 14
2018-01-12 14:08:23 PST
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.
Jonathan Bedard
Comment 15
2018-01-12 14:50:44 PST
Created
attachment 331238
[details]
Patch
Jonathan Bedard
Comment 16
2018-01-12 14:53:42 PST
(In reply to Jonathan Bedard from
comment #15
)
> Created
attachment 331238
[details]
> Patch
This will address Alexey's problem without the side-effects.
Daniel Bates
Comment 17
2018-01-12 15:27:24 PST
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.
Jonathan Bedard
Comment 18
2018-01-16 09:01:36 PST
Created
attachment 331390
[details]
Patch
Jonathan Bedard
Comment 19
2018-01-16 09:35:36 PST
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.
Daniel Bates
Comment 20
2018-01-17 11:40:46 PST
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).
Jonathan Bedard
Comment 21
2018-01-17 11:53:08 PST
Created
attachment 331528
[details]
Patch
Jonathan Bedard
Comment 22
2018-01-17 11:54:44 PST
(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.
Daniel Bates
Comment 23
2018-01-19 10:34:06 PST
Comment on
attachment 331528
[details]
Patch r=me
WebKit Commit Bot
Comment 24
2018-01-19 11:16:03 PST
Comment on
attachment 331528
[details]
Patch Clearing flags on attachment: 331528 Committed
r227222
: <
https://trac.webkit.org/changeset/227222
>
WebKit Commit Bot
Comment 25
2018-01-19 11:16:04 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug