Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unicode problem in protoc-gen-depends #23

Merged
merged 1 commit into from
Sep 2, 2015

Conversation

bobvanderlinden
Copy link
Contributor

When running make, I had the following error:

Traceback (most recent call last):
  File "scripts/protoc-gen-depends", line 25, in <module>
    req.MergeFromString(''.join(stdin.readlines()))
  File "/usr/lib/python3.4/codecs.py", line 319, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc6 in position 161: invalid continuation byte
--depends_out: protoc-gen-depends: Plugin failed with status code 1.

It was caused by stdin being read as a text-stream (default for sys.stdin). My machine is configured to use UTF-8 as text-encoding and in turn it seemed Python couldn't decode the stream as UTF-8.

This change uses sys.stdin.buffer, instead of sys.stdin, which should read bytes instead of encoded-text-strings.

@bobvanderlinden
Copy link
Contributor Author

Oh, I must add that I'm not 100% sure whether the output that protoc-gen-depends generates is actually correct (I have not tested the .d files), but everything still functions the same way, except that byte-strings are used instead of text-strings.

req = CodeGeneratorRequest()
req.MergeFromString(''.join(stdin.readlines()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The solution looks a bit complicated. Have you tried: .encode('utf-8')?

@bobvanderlinden
Copy link
Contributor Author

Found a nicer way to read all bytes from stdin. This will make the code a bit simpler, but still just reads bytes (without using any text-encoding).

@machinekoder
Copy link
Member

Well done, you could rebase the second commit as fixup to keep the history clean.

…treams

This solves Python being unable to decode stdin as the system-specified
encoding (UTF-8 in my case).
@bobvanderlinden
Copy link
Contributor Author

Alright, fixed up original commit with the simplified calls.

mhaberler added a commit that referenced this pull request Sep 2, 2015
Fix unicode problem in protoc-gen-depends
@mhaberler mhaberler merged commit 348d0a9 into machinekit:master Sep 2, 2015
@bobvanderlinden
Copy link
Contributor Author

Thanks!

On Wed, Sep 2, 2015 at 7:40 AM Michael Haberler notifications@github.com
wrote:

Merged #23 #23.


Reply to this email directly or view it on GitHub
#23 (comment)
.

machinekoder pushed a commit that referenced this pull request Nov 22, 2016
Added pri file to make inclusion into other Qt projects easier
machinekoder added a commit that referenced this pull request Nov 22, 2016
314b080 Merge pull request #62 from mhaberler/master
017ce0a Makefile: make template path configurable, depend on all moving parts
05de663 Merge pull request #60 from machinekit/ArcEye-patch-1
b30e223 Update asciidoc.mustache
a292fb0 Merge pull request #58 from strahlex/python-examples
31afa09 Update README.md
c123a4a add Python examples and install README
dc8c822 Merge pull request #57 from mhaberler/asciidoc-support
72d6749 scripts/asciidoc.mustache: a start - this is still markdown
522d981 Makefile: support different output formats
a2494ab Merge pull request #55 from strahlex/doc-fix
7864694 fix automatic build of documentation
e56f95d Merge pull request #51 from strahlex/preview-doc
59180cd added some documentation for preview.proto
5621140 Merge pull request #50 from strahlex/small-clean
3f5125c updated js bundle
860a798 types.proto: added generic full and incremental update
f2b21dd status.proto: fix case incosistency
8ecfe22 Merge pull request #49 from strahlex/js-bundle
e552450 added bundled JavaScript files for browser
0caef9c js: added scripts to create bundle with browserify
a52ae93 Merge pull request #46 from strahlex/doc
7efd4b5 Merge pull request #48 from bobvanderlinden/nodejs
a289f8b changed the way nodejs packages are created
fe0c77d generating single doc file with defined templated
57b2205 Merge pull request #32 from strahlex/unit-cleanup
773843b added machine unit fields
b449530 Merge pull request #36 from strahlex/python-setup
517a13e fix protojs paths and disabling of protojs
81a4623 added protobuf folder and module __init__ to git
d8e02cc added install rules for .proto and .h files
0ae22d8 create separate build directory to hold all ouputs
b3d8036 build C++, Python, Js, objects all with namespace directories
41ebbcc initial work for Python packaging
9b30785 Merge pull request #35 from strahlex/doc
9d8ad18 Merge pull request #38 from bobvanderlinden/fix-qeueue
126b476 fixed typo in INTERP_QEUEUE_WAIT to INTERP_QUEUE_WAIT
5c1a401 Merge pull request #37 from bobvanderlinden/python-environment
c5c2737 use python from environment
2de4ee6 Added documenation for status.proto
a38a012 Added doc generator
5900be6 Merge pull request #33 from strahlex/tool-table-offset
a4ca021 machinetalk-protobuf: replaced tool table offset fields
621f595 Merge pull request #31 from strahlex/python2_3
cae23c4 fix backwards compatibility with Python2
e653531 Merge pull request #29 from strahlex/origin-fix
f73e663 Merge pull request #30 from strahlex/emccmd-feedback
80dc355 added types for command feedback
f00172a OriginIndex: added ORIGIN_UNKNOWN field:
348d0a9 Merge pull request #23 from bobvanderlinden/unicode-fix
b8dda44 protoc-gen-depends: treat stdin/stdout as byte-streams, not as text-streams
c00f3a6 Merge pull request #21 from mhaberler/master
f2105a1 Update README.md
762b3ad Merge pull request #20 from mhaberler/master
10aad18 Update README.md
7724509 rtapicommand: add task creation flags

git-subtree-dir: 3rdparty/machinetalk-protobuf
git-subtree-split: 314b080
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants