xcp.pci: Fix xcp.pci.PCIIds.read() to support UTF-8 in hwdata/pci.ids#35
Closed
bernhardkaindl wants to merge 1 commit intoxenserver:masterfrom
Closed
xcp.pci: Fix xcp.pci.PCIIds.read() to support UTF-8 in hwdata/pci.ids#35bernhardkaindl wants to merge 1 commit intoxenserver:masterfrom
bernhardkaindl wants to merge 1 commit intoxenserver:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #35 +/- ##
==========================================
+ Coverage 69.04% 69.15% +0.11%
==========================================
Files 20 21 +1
Lines 3424 3430 +6
==========================================
+ Hits 2364 2372 +8
+ Misses 1060 1058 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
29cf33e to
78a3185
Compare
bernhardkaindl
commented
May 11, 2023
Contributor
Author
bernhardkaindl
left a comment
There was a problem hiding this comment.
Ready for review!
psafont
approved these changes
May 12, 2023
78a3185 to
09c60a0
Compare
See README-Unicode.md Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
09c60a0 to
9643ab3
Compare
Contributor
Author
bernhardkaindl
added a commit
to rosslagerwall/python-libs
that referenced
this pull request
May 8, 2024
…t-covered-branches CP-41819: Py3: Collect, check and report code coverage from Python3 unit test runs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR:
Fixes and obsoletes #21:
The return types of
Popen()differ between Python2 and Python3, on Python3 and mode/Unicode conversion can be forced using new Python3-specific keyword arguments.Python3
open()is affected as well, but tries todecode()to returnstrby default.This decoding and encoding can fail when open is setup to use the 'ascii' codec:
running python from a user's shell)
ascii, which raisesUnicodeDecodeError()when an the codec encounters anordinal not in range(128).The
tests/test_pci.pynow checks that the fixed code also works in this case.CP-41972: update xcp to support ack py3 #21.
For parsing text (like
/usr/share/hwdata/pci.idswhich has UTF-8 sequences for some graphics cards) from XAPI plugins which do not have a locale set, we need to ensure text mode with encoding="utf-8" is selected for both,open()andPopen().open()andPopen()are used many times in thexcppackage, andpylintwarns about many cases of missingencoding=. Other spots for upcoming work in this topic are:xcp/cmd.pyxcp/net/biosdevname.pyxcp/net/ip.pyWhere
xcp/net/ip.pydoes not have a test yet (to be added) and the tests for the cmd and biosdevname() usemockforopen/Popenand therefore their tests are not neccessarily reflecting/modelling the real behaviour ofopen/Popen()for Python3 in all cases.CI is updated do cover this fix with support for UTF-8 data added at three locations:
Description
We'll need to address all of these places, so this adds a central place to define a way to conditionally enable UTF-8 text most for open() and Popen().
To keep it brief, this specifically only fixes
xcp.pci.The test suite is extended to completely test the fix.
openandPopenas part of the tests is of course upgraded to support both text and binary modes.Ensured by GitHub CI (can also be verfied locally by running tox - used for development):
Details
ACK uses xcp.pci.PCIIds.read() from an XAPI plugin which does not set a locale, so it was an early example for code which is affected by Python3's Popen behavior:
Popen():
Py2 returns Python2 strings (like bytes, we just read/write any data)
Py3 defaults to returning bytes, not str (In Python3: str is Unicode), but it can be switched into text mode to do .decode() and .encode() for us.
Action: