Skip to content

Bugfix: fix processing of UTF-8 files with BOM - #5506

Merged
lasote merged 2 commits into
conan-io:developfrom
SSE4:fix_bom
Jul 29, 2019
Merged

lasote merged 2 commits into
conan-io:developfrom
SSE4:fix_bom

Conversation

@SSE4

@SSE4 SSE4 commented Jul 17, 2019

Copy link
Copy Markdown
Contributor

Changelog: Bugfix: fix processing of UTF-8 files with BOM
Docs: omit
@PYVERS: Macos@py27, Windows@py36, Linux@py27, py34
@tags: svn, slow
@revisions: 1
closes: #5504

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

Signed-off-by: SSE4 <tomskside@gmail.com>
Comment thread conans/util/files.py Outdated
tmp = handle.read()
if not binary:
import codecs
encodings = {codecs.BOM_UTF8: "utf_8_sig",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wow, I can't believe this is necessary... So every python application out there that is reading text files should do something like this? I am not sure it makes sense, but most likely I am failing to understand the issue. Can't we just ask that conanfile.txt should have a standard ascii or utf8 encoding? Is this something that will be solved in Python3 and this is only for Python 2? Wdyt @lasote?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible solving this using comments with encoding like # -*- coding: utf-8 -*- ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, seems so, I have found many such recommendations on stack overflow.
some text editors, mostly on Windows, defaults to save into UTF-8 with BOM, or UTF-16 with BOM, if they contain some non-ASCII byte sequences.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

from wiki:

Microsoft compilers[9] and interpreters, and many pieces of software on Microsoft Windows such as Notepad treat the BOM as a required magic number rather than use heuristics. These tools add a BOM when saving text as UTF-8, and cannot interpret UTF-8 unless the BOM is present or the file contains only ASCII. Google Docs also adds a BOM when converting a document to a plain text file for download.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow, this is insane. Probably that code should be moved to decode_text function that it is already a bit insane.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved to decode_text

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, to confirm, please answer the question from @memsharded, Is this something that will be solved in Python3 and this is only for Python 2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no, it will not be solved by Python 3, I have exactly the same issue with Python 3 and UTF-8 with BOM files on Windows, which Notepad saves by default for me.

Signed-off-by: SSE4 <tomskside@gmail.com>
@lasote lasote added this to the 1.18 milestone Jul 19, 2019
@lasote
lasote merged commit 4e37165 into conan-io:develop Jul 29, 2019
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.

Conan can parse conanfile.txt with BOM

4 participants